Page MenuHomeFreeBSD

bhyve: Move legacy PCI interrupt handling under amd64/
ClosedPublic

Authored by markj on Jun 23 2023, 10:03 PM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 27 2023, 9:47 PM
Unknown Object (File)
Dec 23 2023, 3:05 AM
Unknown Object (File)
Dec 11 2023, 1:02 AM
Unknown Object (File)
Dec 1 2023, 4:18 PM
Unknown Object (File)
Nov 15 2023, 3:17 PM
Unknown Object (File)
Nov 5 2023, 5:35 AM
Unknown Object (File)
Oct 10 2023, 8:33 PM
Unknown Object (File)
Oct 4 2023, 10:18 PM
Subscribers

Details

Summary

Specifically, move IO-APIC, LPC and PIRQ routing code under amd64/.

Use ifdefs to conditionally compile related code in other files. In
particular, legacy PCI interrupt handling is now compiled only on amd64.
This is not too invasive, but suggestions for a more modular approach
would be appreciated.

I am not sure why qemu fwcfg handling is tied to LPC, and I suspect it
should be decoupled. In this commit I just apply an ifdef hammer, but
we will eventually want fwcfg on arm64 as well.

No functional change intended.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

usr.sbin/bhyve/pci_emul.h
127

This ifdef isn't required. Especially, as legacy interrupts could be implemented by arm64 too.

In general, I'd like to avoid ifdefs as much as possible. However, I don't have a strong preference here.

usr.sbin/bhyve/qemu_fwcfg.c
431

fwcfg is mainly used for hypervisor <-> guest firmware communication. That's why it's tied to the bootrom option which is tied to the lpc. Btw: I was told to do it that way. See https://reviews.freebsd.org/D31578#712764

This revision is now accepted and ready to land.Jun 26 2023, 6:25 AM
usr.sbin/bhyve/qemu_fwcfg.c
433

I do think that we want to have fwcfg for arm64 too. Maybe add a comment which explains that we want to have fwcfg for arm64 but that we can't enable it yet because we do not implement the mmio interface yet.

For fwcfg, it's an odd quirk of x86 that the bootrom functionality is hung off of the lpc layer. We may want to adjust the configuration of this such that fwcfg and bootroom are configured at the top-level instead of under lpc at least on arm64, but that can wait until we are a bit further along I think.

usr.sbin/bhyve/pci_emul.c
1767–1768
usr.sbin/bhyve/pci_emul.h
127

I think it's fine to #ifdef this. I would be tempted perhaps to have a helper macro that is something like 'PCI_LINTR` that is only defined on platforms that support legacy interrupts, but I don't think we are likely to support them on either arm64 or RISC-V, so making it all conditional on only amd64 is fine (unless PCI_LINTR would be more readable).

markj marked 3 inline comments as done.Jul 5 2023, 2:25 PM
markj added inline comments.
usr.sbin/bhyve/pci_emul.h
127

A more abstract PCI_LINTR symbol would be more readable, but this code is also tied to amd64-specific interfaces (e.g., from ioapic.h), so I'm on the fence here. I think I'll just leave it as is.

In general, I'd like to avoid ifdefs as much as possible.

Me too, at least most of the ifdefs in bhyverun.c will go away once the file is split into MD components.

usr.sbin/bhyve/qemu_fwcfg.c
431

We could also repurpose the -l option on arm64 to mean something other than "LPC devices". It can be used for generic platform configuration variables, e.g., bootrom and fwcfg.

433

Indeed, I'll add a comment.

This revision was automatically updated to reflect the committed changes.
markj marked 2 inline comments as done.