Implement the legacy debug/test interfaces expected by KVM-unit-tests'
realmode, emulator, and ioapic tests.
Details
Verified via KVM-unit-tests' realmode, emulator, and ioapic tests.
(It should be noted that emulator and ioapic still fail, but, now, for
reasons other than needing these test interfaces).
Diff Detail
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 34832 Build 31863: arc lint + arc unit
Event Timeline
A snapshot of the KVM unit tests, slightly modified to work with bhyve and Adam's change, is at https://people.freebsd.org/~grehan/kvm_unit_tests/kut.tgz
Incorporate @markj's review feedback and style(9)-ize pctestdev_init()'s local variable declarations.
Thanks a lot for the review and feedback, @markj! Diff updated and question/response added accordingly.
usr.sbin/bhyve/pctestdev.c | ||
---|---|---|
223 | So my read was that instructions like outw %eax, 0xe3 would still end up here and, in light of this being a test interface, I thought it made sense to make any misuses immediately apparent. Have I misunderstood something? |
usr.sbin/bhyve/pctestdev.c | ||
---|---|---|
223 | I was really asking whether the existing check is sufficient, but I see now that bhyve ensures that IOPORT_BASE <= port < IOPORT_BASE + IOPORT_LEN, so no other checks are required. I was a bit confused because pctestdev_iomem_io() asserts that addr >= IOMEM_BASE but this function doesn't have a corresponding assertion. |
usr.sbin/bhyve/pctestdev.c | ||
---|---|---|
223 |
Oh ok. Right, as per inout's handler registration implementation, this handler can only be called if port is in [IOPORT_BASE, IOPORT_BASE + IOPORT_LEN).
Ah, thanks! I think I meant to remove this assertion per analogous reasoning (mem handler registration appears to have the same behavior). Does that sound good? |
usr.sbin/bhyve/pctestdev.c | ||
---|---|---|
223 | That seems reasonable to me. Overall the diff looks right to me, thanks. |
usr.sbin/bhyve/pctestdev.c | ||
---|---|---|
223 | Diff updated with assertion removed. Thanks again for the review! |