On 8/8/23 17:40, Alex Williamson wrote: > On Tue, 8 Aug 2023 16:59:16 +0200 > Laszlo Ersek <ler...@redhat.com> wrote: > >> The Solarflare Communications SFC9220 NIC's physical function (PF) appears >> to expose an expansion ROM with the following characteristics: >> >> (1) Single-image ROM, with only a legacy BIOS image (no UEFI driver). >> Alex's rom-parser utility dumps it like this: >> >>> Valid ROM signature found @0h, PCIR offset 20h >>> PCIR: type 0 (x86 PC-AT), vendor: 1924, device: 0a03, class: 000002 >>> PCIR: revision 3, vendor revision: 1 >>> Last image >> >> (2) The BIOS image crashes when booted on i440fx. >> >> (3) The BIOS image prints the following messages on q35: >> >>> Solarflare Boot Manager (v5.2.2.1006) >>> Solarflare Communications 2008-2019 >>> gPXE (http://etherboot.org) - [...] PCI[...] PnP PMM[...] >> >> So it appears like a modified derivative of old gPXE. >> >> Alex surmised in advance that the BIOS image could be accessing >> host-physical addresses rather than guest-phys ones, leading to the crash >> on i440fx. > > ROMs sometimes take shortcuts around the standard interfaces to the > device and can therefore hit gaps in the virtualization, which is why > that's suspect to me. However if it works on q35 but not 440fx it > might be more that we're not matching a PCI topology expectation of the > ROM. Was it only tested on 440fx attached to the root bus or does it > also fail if the PF is attached downstream of a PCI-to-PCI bridge?
I don't know; I'll need to test both of these setups then. > >> Don't expose the option ROM BAR to the VM by default. While this prevents >> netbooting the VM off the PF on q35/SeaBIOS (a relatively rare scenario), >> it does not make any difference for UEFI, and at least the VM doesn't >> crash during boot on i440fx/SeaBIOS (a relatively frequent scenario). >> Users can restore the original behavior via the QEMU cmdline and the >> libvirt domain XML. >> >> (In two years, we've not seen any customer interest in this bug, hence >> there's no incentive to investigate (2).) >> >> Cc: Alex Williamson <alex.william...@redhat.com> (supporter:VFIO) >> Cc: "Cédric Le Goater" <c...@redhat.com> (supporter:VFIO) >> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1975776 >> Signed-off-by: Laszlo Ersek <ler...@redhat.com> >> --- >> hw/vfio/pci-quirks.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c >> index f4ff83680572..270eb16b91fa 100644 >> --- a/hw/vfio/pci-quirks.c >> +++ b/hw/vfio/pci-quirks.c >> @@ -45,6 +45,10 @@ static const struct { >> uint32_t device; >> } rom_denylist[] = { >> { 0x14e4, 0x168e }, /* Broadcom BCM 57810 */ >> + { 0x1924, 0x0a03 }, /* Solarflare Communications >> + * SFC9220 10/40G Ethernet Controller >> + * >> https://bugzilla.redhat.com/show_bug.cgi?id=1975776 > > Unfortunately this is not a public bz so there's not much point in > referencing it in public code or commit log :-\ Thanks, The BZ is not public because it was originally (mis-)filed for the RH kernel, and those BZs are private by default. I'd corrected the BZ component yesterday, but didn't realize I should relax the BZ's visibility. I'll do that now. (That's the right thing to do regardless of whether this patch gets in or not.) Thanks! Laszlo > > Alex > >> + */ >> }; >> >> bool vfio_opt_rom_in_denylist(VFIOPCIDevice *vdev) >
