Page MenuHomeFreeBSD

bhyve: don't crash when guest writes TPM int_enable register
AcceptedPublic

Authored by rosenfeld_grumpf.hope-2000.org on Fri, Sep 6, 1:03 PM.
Tags
None
Referenced Files
F94821818: D46562.diff
Wed, Sep 18, 10:07 AM
F94796391: D46562.diff
Wed, Sep 18, 8:24 AM
Unknown Object (File)
Sun, Sep 15, 11:39 PM
Unknown Object (File)
Thu, Sep 12, 12:12 AM
Unknown Object (File)
Wed, Sep 11, 12:41 PM
Unknown Object (File)
Wed, Sep 11, 6:29 AM
Unknown Object (File)
Sun, Sep 8, 5:56 PM
Unknown Object (File)
Sat, Sep 7, 10:56 AM

Details

Reviewers
corvink
Group Reviewers
bhyve

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 59339
Build 56226: arc lint + arc unit

Event Timeline

Even FreeBSD's own TPM driver writes the INT_ENABLE register during attach, writing 0 to make sure interrupts are off. bhyve really shouldn't crash the VM when that happens.

corvink added inline comments.
usr.sbin/bhyve/tpm_intf_crb.c
341

I slightly prefer adding a warning at least. Ignoring all writes will be unexpected if someone ever tries to get a TPM driver using interrupts working in a bhyve vm.
We could also accept 0 here and exit on other values.

usr.sbin/bhyve/tpm_intf_crb.c
341

I don't mind adding a warning, but I really don't see much value in it, neither for a user nor from a developers perspective. The best case scenario is that it'll end up in some logfile and is going to be ignored, worst case the log fills up with pointless messages.

While it's probably trivial to accept only 0 writes and exit on other values, I'm not convinced it's a sensible idea to rig the hypervisor with booby-traps that will just kill a VM on a unsupported register access. Real hardware usually doesn't halt or catch fire when that happens, so why should the hypervisor do this? I think it makes more sense to carry on and not work against the guest OS. On the other hand, I suppose that's probably what most device emulations in bhyve do already, so maybe it's a feature and not a bug.

Of course, if you insist, I'll happily do either or both. Please let me know.

corvink added inline comments.
usr.sbin/bhyve/tpm_intf_crb.c
341

I don't have a strong preference, so just keep it as is.

This revision is now accepted and ready to land.Wed, Sep 18, 12:40 PM