Details
- Reviewers
corvink - Group Reviewers
bhyve - Commits
- rG2feea221b248: bhyve: don't crash when guest writes TPM int_enable register
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.
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. |
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. |
usr.sbin/bhyve/tpm_intf_crb.c | ||
---|---|---|
341 | I don't have a strong preference, so just keep it as is. |
Do I need a more elaborate commit message here, too?
Also, and in general, where do testing notes go, assuming they are needed?
You should always explain in the commit message why you want to change something. IMO, testing notes should be added to the commit message too. The commit message will never get lost because it's part of the project itself.