Page MenuHomeFreeBSD

bhyve: Add legacy debug/test interfaces for kvm unit tests.
ClosedPublic

Authored by adam_fenn.io on Nov 6 2020, 10:32 PM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 10 2024, 1:38 PM
Unknown Object (File)
Feb 10 2024, 1:38 PM
Unknown Object (File)
Feb 10 2024, 1:34 PM
Unknown Object (File)
Feb 6 2024, 4:32 AM
Unknown Object (File)
Jan 27 2024, 3:15 PM
Unknown Object (File)
Dec 23 2023, 2:27 AM
Unknown Object (File)
Dec 5 2023, 5:58 AM
Unknown Object (File)
Dec 1 2023, 2:23 AM

Details

Summary

Implement the legacy debug/test interfaces expected by KVM-unit-tests'
realmode, emulator, and ioapic tests.

Test Plan

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 34775
Build 31825: arc lint + arc unit

Event Timeline

adam_fenn.io held this revision as a draft.

Fix style mistake in Makefile.

grehan retitled this revision from bhyve: Add legacy debug/test interfaces to bhyve: Add legacy debug/test interfaces for kvm unit tests..Nov 12 2020, 8:59 AM

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

markj added inline comments.
usr.sbin/bhyve/pci_lpc.c
119

I think this could be simplified a bit:

if (pctestdev_present) {
    ...
} else if (pctestdev_parse(str) == 0) {
    ...
}
usr.sbin/bhyve/pctestdev.c
223

Do we need to handle overflow here?

268

Perhaps group this with pctestdev_getname() above.

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

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.

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).

I was a bit confused because pctestdev_iomem_io() asserts that addr >= IOMEM_BASE but this function doesn't have a corresponding assertion.

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?

markj added inline comments.
usr.sbin/bhyve/pctestdev.c
223

That seems reasonable to me. Overall the diff looks right to me, thanks.

This revision is now accepted and ready to land.Nov 16 2020, 4:43 PM

Remove unnecessary assertion.

This revision now requires review to proceed.Nov 16 2020, 5:40 PM
adam_fenn.io added inline comments.
usr.sbin/bhyve/pctestdev.c
223

Diff updated with assertion removed. Thanks again for the review!

This revision is now accepted and ready to land.Nov 17 2020, 12:22 PM