| Message ID | 20240627-reuse-v10-11-7ca0b8ed3d9f@daynix.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw/pci: SR-IOV related fixes and improvements | expand |
On Thu, Jun 27, 2024 at 03:08:00PM +0900, Akihiko Odaki wrote: > rom_bar is tristate but was defined as uint32_t so convert it into > OnOffAuto. > > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> Commit log should explain why this is an improvement, not just what's done. > diff --git a/docs/igd-assign.txt b/docs/igd-assign.txt > index e17bb50789ad..35c6c8e28493 100644 > --- a/docs/igd-assign.txt > +++ b/docs/igd-assign.txt > @@ -35,7 +35,7 @@ IGD has two different modes for assignment using vfio-pci: > ISA/LPC bridge device (vfio-pci-igd-lpc-bridge) on the root bus at > PCI address 1f.0. > * The IGD device must have a VGA ROM, either provided via the romfile > - option or loaded automatically through vfio (standard). rombar=0 > + option or loaded automatically through vfio (standard). rombar=off > will disable legacy mode support. > * Hotplug of the IGD device is not supported. > * The IGD device must be a SandyBridge or newer model device. ... > diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c > index 39dae72497e0..0e920ed0691a 100644 > --- a/hw/vfio/pci-quirks.c > +++ b/hw/vfio/pci-quirks.c > @@ -33,7 +33,7 @@ > * execution as noticed with the BCM 57810 card for lack of a > * more better way to handle such issues. > * The user can still override by specifying a romfile or > - * rombar=1. > + * rombar=on. > * Please see https://bugs.launchpad.net/qemu/+bug/1284874 > * for an analysis of the 57810 card hang. When adding > * a new vendor id/device id combination below, please also add So we are apparently breaking a bunch of users who followed documentation to the dot. Why is this a good idea?
On Tue, 2 Jul 2024, Michael S. Tsirkin wrote: > On Thu, Jun 27, 2024 at 03:08:00PM +0900, Akihiko Odaki wrote: >> rom_bar is tristate but was defined as uint32_t so convert it into >> OnOffAuto. >> >> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> > > Commit log should explain why this is an improvement, > not just what's done. > > >> diff --git a/docs/igd-assign.txt b/docs/igd-assign.txt >> index e17bb50789ad..35c6c8e28493 100644 >> --- a/docs/igd-assign.txt >> +++ b/docs/igd-assign.txt >> @@ -35,7 +35,7 @@ IGD has two different modes for assignment using vfio-pci: >> ISA/LPC bridge device (vfio-pci-igd-lpc-bridge) on the root bus at >> PCI address 1f.0. >> * The IGD device must have a VGA ROM, either provided via the romfile >> - option or loaded automatically through vfio (standard). rombar=0 >> + option or loaded automatically through vfio (standard). rombar=off >> will disable legacy mode support. >> * Hotplug of the IGD device is not supported. >> * The IGD device must be a SandyBridge or newer model device. > > ... > >> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c >> index 39dae72497e0..0e920ed0691a 100644 >> --- a/hw/vfio/pci-quirks.c >> +++ b/hw/vfio/pci-quirks.c >> @@ -33,7 +33,7 @@ >> * execution as noticed with the BCM 57810 card for lack of a >> * more better way to handle such issues. >> * The user can still override by specifying a romfile or >> - * rombar=1. >> + * rombar=on. >> * Please see https://bugs.launchpad.net/qemu/+bug/1284874 >> * for an analysis of the 57810 card hang. When adding >> * a new vendor id/device id combination below, please also add > > > So we are apparently breaking a bunch of users who followed > documentation to the dot. Why is this a good idea? On/off is clearer than 1/0. But isn't 1/0 a synonym for on/off so previous command lines would still work? Regards, BALATON Zoltan
On Wed, Jul 03, 2024 at 04:15:23AM +0200, BALATON Zoltan wrote: > On Tue, 2 Jul 2024, Michael S. Tsirkin wrote: > > On Thu, Jun 27, 2024 at 03:08:00PM +0900, Akihiko Odaki wrote: > > > rom_bar is tristate but was defined as uint32_t so convert it into > > > OnOffAuto. > > > > > > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> > > > > Commit log should explain why this is an improvement, > > not just what's done. > > > > > > > diff --git a/docs/igd-assign.txt b/docs/igd-assign.txt > > > index e17bb50789ad..35c6c8e28493 100644 > > > --- a/docs/igd-assign.txt > > > +++ b/docs/igd-assign.txt > > > @@ -35,7 +35,7 @@ IGD has two different modes for assignment using vfio-pci: > > > ISA/LPC bridge device (vfio-pci-igd-lpc-bridge) on the root bus at > > > PCI address 1f.0. > > > * The IGD device must have a VGA ROM, either provided via the romfile > > > - option or loaded automatically through vfio (standard). rombar=0 > > > + option or loaded automatically through vfio (standard). rombar=off > > > will disable legacy mode support. > > > * Hotplug of the IGD device is not supported. > > > * The IGD device must be a SandyBridge or newer model device. > > > > ... > > > > > diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c > > > index 39dae72497e0..0e920ed0691a 100644 > > > --- a/hw/vfio/pci-quirks.c > > > +++ b/hw/vfio/pci-quirks.c > > > @@ -33,7 +33,7 @@ > > > * execution as noticed with the BCM 57810 card for lack of a > > > * more better way to handle such issues. > > > * The user can still override by specifying a romfile or > > > - * rombar=1. > > > + * rombar=on. > > > * Please see https://bugs.launchpad.net/qemu/+bug/1284874 > > > * for an analysis of the 57810 card hang. When adding > > > * a new vendor id/device id combination below, please also add > > > > > > So we are apparently breaking a bunch of users who followed > > documentation to the dot. Why is this a good idea? > > On/off is clearer than 1/0. But isn't 1/0 a synonym for on/off so previous > command lines would still work? > > Regards, > BALATON Zoltan I see nothing in code that would make it so: const QEnumLookup OnOffAuto_lookup = { .array = (const char *const[]) { [ON_OFF_AUTO_AUTO] = "auto", [ON_OFF_AUTO_ON] = "on", [ON_OFF_AUTO_OFF] = "off", }, .size = ON_OFF_AUTO__MAX }; I also tried with an existing property: $ ./qemu-system-x86_64 -device intel-hda,msi=0 qemu-system-x86_64: -device intel-hda,msi=0: Parameter 'msi' does not accept value '0'
On Wed, 3 Jul 2024, Michael S. Tsirkin wrote: > On Wed, Jul 03, 2024 at 04:15:23AM +0200, BALATON Zoltan wrote: >> On Tue, 2 Jul 2024, Michael S. Tsirkin wrote: >>> On Thu, Jun 27, 2024 at 03:08:00PM +0900, Akihiko Odaki wrote: >>>> rom_bar is tristate but was defined as uint32_t so convert it into >>>> OnOffAuto. >>>> >>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> >>> >>> Commit log should explain why this is an improvement, >>> not just what's done. >>> >>> >>>> diff --git a/docs/igd-assign.txt b/docs/igd-assign.txt >>>> index e17bb50789ad..35c6c8e28493 100644 >>>> --- a/docs/igd-assign.txt >>>> +++ b/docs/igd-assign.txt >>>> @@ -35,7 +35,7 @@ IGD has two different modes for assignment using vfio-pci: >>>> ISA/LPC bridge device (vfio-pci-igd-lpc-bridge) on the root bus at >>>> PCI address 1f.0. >>>> * The IGD device must have a VGA ROM, either provided via the romfile >>>> - option or loaded automatically through vfio (standard). rombar=0 >>>> + option or loaded automatically through vfio (standard). rombar=off >>>> will disable legacy mode support. >>>> * Hotplug of the IGD device is not supported. >>>> * The IGD device must be a SandyBridge or newer model device. >>> >>> ... >>> >>>> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c >>>> index 39dae72497e0..0e920ed0691a 100644 >>>> --- a/hw/vfio/pci-quirks.c >>>> +++ b/hw/vfio/pci-quirks.c >>>> @@ -33,7 +33,7 @@ >>>> * execution as noticed with the BCM 57810 card for lack of a >>>> * more better way to handle such issues. >>>> * The user can still override by specifying a romfile or >>>> - * rombar=1. >>>> + * rombar=on. >>>> * Please see https://bugs.launchpad.net/qemu/+bug/1284874 >>>> * for an analysis of the 57810 card hang. When adding >>>> * a new vendor id/device id combination below, please also add >>> >>> >>> So we are apparently breaking a bunch of users who followed >>> documentation to the dot. Why is this a good idea? >> >> On/off is clearer than 1/0. But isn't 1/0 a synonym for on/off so previous >> command lines would still work? >> >> Regards, >> BALATON Zoltan > > I see nothing in code that would make it so: > > > const QEnumLookup OnOffAuto_lookup = { > .array = (const char *const[]) { > [ON_OFF_AUTO_AUTO] = "auto", > [ON_OFF_AUTO_ON] = "on", > [ON_OFF_AUTO_OFF] = "off", > }, > .size = ON_OFF_AUTO__MAX > }; > > I also tried with an existing property: > > $ ./qemu-system-x86_64 -device intel-hda,msi=0 > qemu-system-x86_64: -device intel-hda,msi=0: Parameter 'msi' does not accept value '0' Then it was probably bit properties that also accept 0/1, on/off, true/false. Maybe similar aliases could be added to on/off/auto? In any case when I first saw rombar I thought it would set the BAR of the ROM so wondered why it's 1 and not 5 or 6 or an offset. So on/off is clearer in this case. Regards, BALATON Zoltan
On Wed, Jul 03, 2024 at 01:00:21PM +0200, BALATON Zoltan wrote: > On Wed, 3 Jul 2024, Michael S. Tsirkin wrote: > > On Wed, Jul 03, 2024 at 04:15:23AM +0200, BALATON Zoltan wrote: > > > On Tue, 2 Jul 2024, Michael S. Tsirkin wrote: > > > > On Thu, Jun 27, 2024 at 03:08:00PM +0900, Akihiko Odaki wrote: > > > > > rom_bar is tristate but was defined as uint32_t so convert it into > > > > > OnOffAuto. > > > > > > > > > > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> > > > > > > > > Commit log should explain why this is an improvement, > > > > not just what's done. > > > > > > > > > > > > > diff --git a/docs/igd-assign.txt b/docs/igd-assign.txt > > > > > index e17bb50789ad..35c6c8e28493 100644 > > > > > --- a/docs/igd-assign.txt > > > > > +++ b/docs/igd-assign.txt > > > > > @@ -35,7 +35,7 @@ IGD has two different modes for assignment using vfio-pci: > > > > > ISA/LPC bridge device (vfio-pci-igd-lpc-bridge) on the root bus at > > > > > PCI address 1f.0. > > > > > * The IGD device must have a VGA ROM, either provided via the romfile > > > > > - option or loaded automatically through vfio (standard). rombar=0 > > > > > + option or loaded automatically through vfio (standard). rombar=off > > > > > will disable legacy mode support. > > > > > * Hotplug of the IGD device is not supported. > > > > > * The IGD device must be a SandyBridge or newer model device. > > > > > > > > ... > > > > > > > > > diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c > > > > > index 39dae72497e0..0e920ed0691a 100644 > > > > > --- a/hw/vfio/pci-quirks.c > > > > > +++ b/hw/vfio/pci-quirks.c > > > > > @@ -33,7 +33,7 @@ > > > > > * execution as noticed with the BCM 57810 card for lack of a > > > > > * more better way to handle such issues. > > > > > * The user can still override by specifying a romfile or > > > > > - * rombar=1. > > > > > + * rombar=on. > > > > > * Please see https://bugs.launchpad.net/qemu/+bug/1284874 > > > > > * for an analysis of the 57810 card hang. When adding > > > > > * a new vendor id/device id combination below, please also add > > > > > > > > > > > > So we are apparently breaking a bunch of users who followed > > > > documentation to the dot. Why is this a good idea? > > > > > > On/off is clearer than 1/0. But isn't 1/0 a synonym for on/off so previous > > > command lines would still work? > > > > > > Regards, > > > BALATON Zoltan > > > > I see nothing in code that would make it so: > > > > > > const QEnumLookup OnOffAuto_lookup = { > > .array = (const char *const[]) { > > [ON_OFF_AUTO_AUTO] = "auto", > > [ON_OFF_AUTO_ON] = "on", > > [ON_OFF_AUTO_OFF] = "off", > > }, > > .size = ON_OFF_AUTO__MAX > > }; > > > > I also tried with an existing property: > > > > $ ./qemu-system-x86_64 -device intel-hda,msi=0 > > qemu-system-x86_64: -device intel-hda,msi=0: Parameter 'msi' does not accept value '0' > > Then it was probably bit properties that also accept 0/1, on/off, > true/false. I mean, the code is open, why do you keep guessing? No, these reuse the bool parsing logic: static void prop_get_bit(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) { Property *prop = opaque; uint32_t *p = object_field_prop_ptr(obj, prop); bool value = (*p & qdev_get_prop_mask(prop)) != 0; visit_type_bool(v, name, &value, errp); } and that never accepted 0 or 1: bool qapi_bool_parse(const char *name, const char *value, bool *obj, Error **errp) { if (g_str_equal(value, "on") || g_str_equal(value, "yes") || g_str_equal(value, "true") || g_str_equal(value, "y")) { *obj = true; return true; } if (g_str_equal(value, "off") || g_str_equal(value, "no") || g_str_equal(value, "false") || g_str_equal(value, "n")) { *obj = false; return true; } error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name, "'on' or 'off'"); return false; } > Maybe similar aliases could be added to on/off/auto? Could be, but even then switching to that would mean that user sets 1 but query returns "on". Might or might not surprise some users. Adding true/false yes/no y/n aliases to on/off/auto might make sense though, for consistency. Donnu if QAPI guys will agree, though, and not directly related to this patchset. One other idea is to add a generic way to detect that a property is set by user. This requirement comes up, once in a while. > In any case when I first saw rombar I thought it would set the BAR of the > ROM so wondered why it's 1 and not 5 or 6 or an offset. So on/off is clearer > in this case. > > Regards, > BALATON Zoltan I agree here, but it's been here for a long time.
On Wed, 3 Jul 2024 13:00:21 +0200 (CEST) BALATON Zoltan <balaton@eik.bme.hu> wrote: > On Wed, 3 Jul 2024, Michael S. Tsirkin wrote: > > On Wed, Jul 03, 2024 at 04:15:23AM +0200, BALATON Zoltan wrote: > >> On Tue, 2 Jul 2024, Michael S. Tsirkin wrote: > >>> On Thu, Jun 27, 2024 at 03:08:00PM +0900, Akihiko Odaki wrote: > >>>> rom_bar is tristate but was defined as uint32_t so convert it into > >>>> OnOffAuto. > >>>> > >>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> > >>> > >>> Commit log should explain why this is an improvement, > >>> not just what's done. > >>> > >>> > >>>> diff --git a/docs/igd-assign.txt b/docs/igd-assign.txt > >>>> index e17bb50789ad..35c6c8e28493 100644 > >>>> --- a/docs/igd-assign.txt > >>>> +++ b/docs/igd-assign.txt > >>>> @@ -35,7 +35,7 @@ IGD has two different modes for assignment using vfio-pci: > >>>> ISA/LPC bridge device (vfio-pci-igd-lpc-bridge) on the root bus at > >>>> PCI address 1f.0. > >>>> * The IGD device must have a VGA ROM, either provided via the romfile > >>>> - option or loaded automatically through vfio (standard). rombar=0 > >>>> + option or loaded automatically through vfio (standard). rombar=off > >>>> will disable legacy mode support. > >>>> * Hotplug of the IGD device is not supported. > >>>> * The IGD device must be a SandyBridge or newer model device. > >>> > >>> ... > >>> > >>>> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c > >>>> index 39dae72497e0..0e920ed0691a 100644 > >>>> --- a/hw/vfio/pci-quirks.c > >>>> +++ b/hw/vfio/pci-quirks.c > >>>> @@ -33,7 +33,7 @@ > >>>> * execution as noticed with the BCM 57810 card for lack of a > >>>> * more better way to handle such issues. > >>>> * The user can still override by specifying a romfile or > >>>> - * rombar=1. > >>>> + * rombar=on. > >>>> * Please see https://bugs.launchpad.net/qemu/+bug/1284874 > >>>> * for an analysis of the 57810 card hang. When adding > >>>> * a new vendor id/device id combination below, please also add > >>> > >>> > >>> So we are apparently breaking a bunch of users who followed > >>> documentation to the dot. Why is this a good idea? > >> > >> On/off is clearer than 1/0. But isn't 1/0 a synonym for on/off so previous > >> command lines would still work? > >> > >> Regards, > >> BALATON Zoltan > > > > I see nothing in code that would make it so: > > > > > > const QEnumLookup OnOffAuto_lookup = { > > .array = (const char *const[]) { > > [ON_OFF_AUTO_AUTO] = "auto", > > [ON_OFF_AUTO_ON] = "on", > > [ON_OFF_AUTO_OFF] = "off", > > }, > > .size = ON_OFF_AUTO__MAX > > }; > > > > I also tried with an existing property: > > > > $ ./qemu-system-x86_64 -device intel-hda,msi=0 > > qemu-system-x86_64: -device intel-hda,msi=0: Parameter 'msi' does not accept value '0' > > Then it was probably bit properties that also accept 0/1, on/off, > true/false. Maybe similar aliases could be added to on/off/auto? > > In any case when I first saw rombar I thought it would set the BAR of the > ROM so wondered why it's 1 and not 5 or 6 or an offset. So on/off is > clearer in this case. There's only one PCI spec defined offset for the ROM BAR. Yes, the option could be more clear but relocating the ROM to a different regular BAR offset is invalid. Thanks, Alex
diff --git a/docs/igd-assign.txt b/docs/igd-assign.txt index e17bb50789ad..35c6c8e28493 100644 --- a/docs/igd-assign.txt +++ b/docs/igd-assign.txt @@ -35,7 +35,7 @@ IGD has two different modes for assignment using vfio-pci: ISA/LPC bridge device (vfio-pci-igd-lpc-bridge) on the root bus at PCI address 1f.0. * The IGD device must have a VGA ROM, either provided via the romfile - option or loaded automatically through vfio (standard). rombar=0 + option or loaded automatically through vfio (standard). rombar=off will disable legacy mode support. * Hotplug of the IGD device is not supported. * The IGD device must be a SandyBridge or newer model device. diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h index ca151325085d..49b341ce2e27 100644 --- a/include/hw/pci/pci_device.h +++ b/include/hw/pci/pci_device.h @@ -147,7 +147,7 @@ struct PCIDevice { uint32_t romsize; bool has_rom; MemoryRegion rom; - uint32_t rom_bar; + OnOffAuto rom_bar; /* INTx routing notifier */ PCIINTxRoutingNotifier intx_routing_notifier; diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 1eb6abf534ca..901f5460d774 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -71,7 +71,7 @@ static Property pci_props[] = { DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1), DEFINE_PROP_STRING("romfile", PCIDevice, romfile), DEFINE_PROP_UINT32("romsize", PCIDevice, romsize, UINT32_MAX), - DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, 1), + DEFINE_PROP_ON_OFF_AUTO("rombar", PCIDevice, rom_bar, ON_OFF_AUTO_AUTO), DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present, QEMU_PCI_CAP_MULTIFUNCTION_BITNR, false), DEFINE_PROP_BIT("x-pcie-lnksta-dllla", PCIDevice, cap_present, @@ -2334,7 +2334,7 @@ static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom, return; } - if (!pdev->rom_bar) { + if (pdev->rom_bar == ON_OFF_AUTO_OFF) { /* * Load rom via fw_cfg instead of creating a rom bar, * for 0.11 compatibility. diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c index 39dae72497e0..0e920ed0691a 100644 --- a/hw/vfio/pci-quirks.c +++ b/hw/vfio/pci-quirks.c @@ -33,7 +33,7 @@ * execution as noticed with the BCM 57810 card for lack of a * more better way to handle such issues. * The user can still override by specifying a romfile or - * rombar=1. + * rombar=on. * Please see https://bugs.launchpad.net/qemu/+bug/1284874 * for an analysis of the 57810 card hang. When adding * a new vendor id/device id combination below, please also add diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index 74a79bdf61f9..4c4d9dc81efb 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -902,7 +902,7 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev) error_report("vfio-pci: Cannot read device rom at " "%s", vdev->vbasedev.name); error_printf("Device option ROM contents are probably invalid " - "(check dmesg).\nSkip option ROM probe with rombar=0, " + "(check dmesg).\nSkip option ROM probe with rombar=off, " "or load from file with romfile=\n"); return; } @@ -1012,11 +1012,10 @@ static void vfio_pci_size_rom(VFIOPCIDevice *vdev) { uint32_t orig, size = cpu_to_le32((uint32_t)PCI_ROM_ADDRESS_MASK); off_t offset = vdev->config_offset + PCI_ROM_ADDRESS; - DeviceState *dev = DEVICE(vdev); char *name; int fd = vdev->vbasedev.fd; - if (vdev->pdev.romfile || !vdev->pdev.rom_bar) { + if (vdev->pdev.romfile || vdev->pdev.rom_bar == ON_OFF_AUTO_OFF) { /* Since pci handles romfile, just print a message and return */ if (vfio_opt_rom_in_denylist(vdev) && vdev->pdev.romfile) { warn_report("Device at %s is known to cause system instability" @@ -1046,17 +1045,17 @@ static void vfio_pci_size_rom(VFIOPCIDevice *vdev) } if (vfio_opt_rom_in_denylist(vdev)) { - if (dev->opts && qdict_haskey(dev->opts, "rombar")) { + if (vdev->pdev.rom_bar == ON_OFF_AUTO_ON) { warn_report("Device at %s is known to cause system instability" " issues during option rom execution", vdev->vbasedev.name); error_printf("Proceeding anyway since user specified" - " non zero value for rombar\n"); + " on for rombar\n"); } else { warn_report("Rom loading for device at %s has been disabled" " due to system instability issues", vdev->vbasedev.name); - error_printf("Specify rombar=1 or romfile to force\n"); + error_printf("Specify rombar=on or romfile to force\n"); return; } } diff --git a/hw/xen/xen_pt_load_rom.c b/hw/xen/xen_pt_load_rom.c index 6bc64acd3352..025a6b25a916 100644 --- a/hw/xen/xen_pt_load_rom.c +++ b/hw/xen/xen_pt_load_rom.c @@ -26,7 +26,7 @@ void *pci_assign_dev_load_option_rom(PCIDevice *dev, Object *owner = OBJECT(dev); /* If loading ROM from file, pci handles it */ - if (dev->romfile || !dev->rom_bar) { + if (dev->romfile || dev->rom_bar == ON_OFF_AUTO_OFF) { return NULL; } @@ -71,7 +71,7 @@ void *pci_assign_dev_load_option_rom(PCIDevice *dev, if (!fread(ptr, 1, st.st_size, fp)) { error_report("pci-assign: Cannot read from host %s", rom_file); error_printf("Device option ROM contents are probably invalid " - "(check dmesg).\nSkip option ROM probe with rombar=0, " + "(check dmesg).\nSkip option ROM probe with rombar=off, " "or load from file with romfile=\n"); goto close_rom; } diff --git a/tests/qtest/virtio-net-failover.c b/tests/qtest/virtio-net-failover.c index 73dfabc2728b..f65b97683fb6 100644 --- a/tests/qtest/virtio-net-failover.c +++ b/tests/qtest/virtio-net-failover.c @@ -568,7 +568,7 @@ static void test_hotplug_2_reverse(void) "{'bus': 'root0'," "'failover': true," "'netdev': 'hs0'," - "'rombar': 0," + "'rombar': 'off'," "'romfile': ''," "'mac': '"MAC_STANDBY0"'}"); @@ -655,7 +655,7 @@ static void test_migrate_out(gconstpointer opaque) "{'bus': 'root1'," "'failover_pair_id': 'standby0'," "'netdev': 'hs1'," - "'rombar': 0," + "'rombar': 'off'," "'romfile': ''," "'mac': '"MAC_PRIMARY0"'}"); @@ -765,7 +765,7 @@ static void test_migrate_in(gconstpointer opaque) "{'bus': 'root1'," "'failover_pair_id': 'standby0'," "'netdev': 'hs1'," - "'rombar': 0," + "'rombar': 'off'," "'romfile': ''," "'mac': '"MAC_PRIMARY0"'}"); @@ -819,7 +819,7 @@ static void test_off_migrate_out(gconstpointer opaque) "{'bus': 'root1'," "'failover_pair_id': 'standby0'," "'netdev': 'hs1'," - "'rombar': 0," + "'rombar': 'off'," "'romfile': ''," "'mac': '"MAC_PRIMARY0"'}"); @@ -887,7 +887,7 @@ static void test_off_migrate_in(gconstpointer opaque) "{'bus': 'root1'," "'failover_pair_id': 'standby0'," "'netdev': 'hs1'," - "'rombar': 0," + "'rombar': 'off'," "'romfile': ''," "'mac': '"MAC_PRIMARY0"'}"); @@ -938,7 +938,7 @@ static void test_guest_off_migrate_out(gconstpointer opaque) "{'bus': 'root1'," "'failover_pair_id': 'standby0'," "'netdev': 'hs1'," - "'rombar': 0," + "'rombar': 'off'," "'romfile': ''," "'mac': '"MAC_PRIMARY0"'}"); @@ -1014,7 +1014,7 @@ static void test_guest_off_migrate_in(gconstpointer opaque) "{'bus': 'root1'," "'failover_pair_id': 'standby0'," "'netdev': 'hs1'," - "'rombar': 0," + "'rombar': 'off'," "'romfile': ''," "'mac': '"MAC_PRIMARY0"'}"); @@ -1065,7 +1065,7 @@ static void test_migrate_guest_off_abort(gconstpointer opaque) "{'bus': 'root1'," "'failover_pair_id': 'standby0'," "'netdev': 'hs1'," - "'rombar': 0," + "'rombar': 'off'," "'romfile': ''," "'mac': '"MAC_PRIMARY0"'}"); @@ -1170,7 +1170,7 @@ static void test_migrate_abort_wait_unplug(gconstpointer opaque) "{'bus': 'root1'," "'failover_pair_id': 'standby0'," "'netdev': 'hs1'," - "'rombar': 0," + "'rombar': 'off'," "'romfile': ''," "'mac': '"MAC_PRIMARY0"'}"); @@ -1259,7 +1259,7 @@ static void test_migrate_abort_active(gconstpointer opaque) "{'bus': 'root1'," "'failover_pair_id': 'standby0'," "'netdev': 'hs1'," - "'rombar': 0," + "'rombar': 'off'," "'romfile': ''," "'mac': '"MAC_PRIMARY0"'}"); @@ -1358,7 +1358,7 @@ static void test_migrate_off_abort(gconstpointer opaque) "{'bus': 'root1'," "'failover_pair_id': 'standby0'," "'netdev': 'hs1'," - "'rombar': 0," + "'rombar': 'off'," "'romfile': ''," "'mac': '"MAC_PRIMARY0"'}"); @@ -1450,7 +1450,7 @@ static void test_migrate_abort_timeout(gconstpointer opaque) "{'bus': 'root1'," "'failover_pair_id': 'standby0'," "'netdev': 'hs1'," - "'rombar': 0," + "'rombar': 'off'," "'romfile': ''," "'mac': '"MAC_PRIMARY0"'}"); @@ -1543,7 +1543,7 @@ static void test_multi_out(gconstpointer opaque) "{'bus': 'root1'," "'failover_pair_id': 'standby0'," "'netdev': 'hs1'," - "'rombar': 0," + "'rombar': 'off'," "'romfile': ''," "'mac': '"MAC_PRIMARY0"'}"); @@ -1574,7 +1574,7 @@ static void test_multi_out(gconstpointer opaque) "{'bus': 'root3'," "'failover_pair_id': 'standby1'," "'netdev': 'hs3'," - "'rombar': 0," + "'rombar': 'off'," "'romfile': ''," "'mac': '"MAC_PRIMARY1"'}"); @@ -1713,7 +1713,7 @@ static void test_multi_in(gconstpointer opaque) "{'bus': 'root1'," "'failover_pair_id': 'standby0'," "'netdev': 'hs1'," - "'rombar': 0," + "'rombar': 'off'," "'romfile': ''," "'mac': '"MAC_PRIMARY0"'}"); @@ -1737,7 +1737,7 @@ static void test_multi_in(gconstpointer opaque) "{'bus': 'root3'," "'failover_pair_id': 'standby1'," "'netdev': 'hs3'," - "'rombar': 0," + "'rombar': 'off'," "'romfile': ''," "'mac': '"MAC_PRIMARY1"'}");
rom_bar is tristate but was defined as uint32_t so convert it into OnOffAuto. Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> --- docs/igd-assign.txt | 2 +- include/hw/pci/pci_device.h | 2 +- hw/pci/pci.c | 4 ++-- hw/vfio/pci-quirks.c | 2 +- hw/vfio/pci.c | 11 +++++------ hw/xen/xen_pt_load_rom.c | 4 ++-- tests/qtest/virtio-net-failover.c | 32 ++++++++++++++++---------------- 7 files changed, 28 insertions(+), 29 deletions(-)