| Message ID | 20240708041056.54504-1-dongli.zhang@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/1] pci: don't skip function 0 occupancy verification for devfn auto assign | expand |
On Sun, Jul 07, 2024 at 09:10:56PM -0700, Dongli Zhang wrote: > When the devfn is already assigned in the command line, the > do_pci_register_device() may verify if the function 0 is already occupied. > > However, when devfn < 0, the verification is skipped because it is part of > the last "else if". > > For instance, suppose there is already a device at addr=00.00 of a port. > > -device pcie-root-port,bus=pcie.0,chassis=115,id=port01,addr=0e.00 \ > -device virtio-net-pci,bus=port01,id=vnet01,addr=00.00 \ > > When 'addr' is specified for the 2nd device, the hotplug is denied. > > (qemu) device_add virtio-net-pci,bus=port01,id=vnet02,addr=01.00 > Error: PCI: slot 0 function 0 already occupied by virtio-net-pci, new func virtio-net-pci cannot be exposed to guest. > > When 'addr' is automatically assigned, the hotplug is not denied. This is > because the verification is skipped. > > (qemu) device_add virtio-net-pci,bus=port01,id=vnet02 > warning: PCI: slot 1 is not valid for virtio-net-pci, parent device only allows plugging into slot 0. > > Fix the issue by moving the verification into an independent 'if' > statement. > > Fixes: 3f1e1478db2d ("enable multi-function hot-add") > Reported-by: Aswin Unnikrishnan <aswin.u.unnikrishnan@oracle.com> > Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com> Well, the user did nothing wrong here. Either we forbid a mix of manual and auto-assigned functions, or we make it work correctly. Also note such change in behaviour will need a compat knob and only affect new machine types. > --- > hw/pci/pci.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 4c7be52951..82ebd243d0 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -1186,14 +1186,15 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, > PCI_SLOT(devfn), PCI_FUNC(devfn), name, > bus->devices[devfn]->name, bus->devices[devfn]->qdev.id); > return NULL; > - } /* > - * Populating function 0 triggers a scan from the guest that > - * exposes other non-zero functions. Hence we need to ensure that > - * function 0 wasn't added yet. > - */ > - else if (dev->hotplugged && > - !pci_is_vf(pci_dev) && > - pci_get_function_0(pci_dev)) { > + } > + > + /* > + * Populating function 0 triggers a scan from the guest that > + * exposes other non-zero functions. Hence we need to ensure that > + * function 0 wasn't added yet. > + */ > + if (dev->hotplugged && !pci_is_vf(pci_dev) && > + pci_get_function_0(pci_dev)) { > error_setg(errp, "PCI: slot %d function 0 already occupied by %s," > " new func %s cannot be exposed to guest.", > PCI_SLOT(pci_get_function_0(pci_dev)->devfn), > -- > 2.34.1
diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 4c7be52951..82ebd243d0 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -1186,14 +1186,15 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCI_SLOT(devfn), PCI_FUNC(devfn), name, bus->devices[devfn]->name, bus->devices[devfn]->qdev.id); return NULL; - } /* - * Populating function 0 triggers a scan from the guest that - * exposes other non-zero functions. Hence we need to ensure that - * function 0 wasn't added yet. - */ - else if (dev->hotplugged && - !pci_is_vf(pci_dev) && - pci_get_function_0(pci_dev)) { + } + + /* + * Populating function 0 triggers a scan from the guest that + * exposes other non-zero functions. Hence we need to ensure that + * function 0 wasn't added yet. + */ + if (dev->hotplugged && !pci_is_vf(pci_dev) && + pci_get_function_0(pci_dev)) { error_setg(errp, "PCI: slot %d function 0 already occupied by %s," " new func %s cannot be exposed to guest.", PCI_SLOT(pci_get_function_0(pci_dev)->devfn),
When the devfn is already assigned in the command line, the do_pci_register_device() may verify if the function 0 is already occupied. However, when devfn < 0, the verification is skipped because it is part of the last "else if". For instance, suppose there is already a device at addr=00.00 of a port. -device pcie-root-port,bus=pcie.0,chassis=115,id=port01,addr=0e.00 \ -device virtio-net-pci,bus=port01,id=vnet01,addr=00.00 \ When 'addr' is specified for the 2nd device, the hotplug is denied. (qemu) device_add virtio-net-pci,bus=port01,id=vnet02,addr=01.00 Error: PCI: slot 0 function 0 already occupied by virtio-net-pci, new func virtio-net-pci cannot be exposed to guest. When 'addr' is automatically assigned, the hotplug is not denied. This is because the verification is skipped. (qemu) device_add virtio-net-pci,bus=port01,id=vnet02 warning: PCI: slot 1 is not valid for virtio-net-pci, parent device only allows plugging into slot 0. Fix the issue by moving the verification into an independent 'if' statement. Fixes: 3f1e1478db2d ("enable multi-function hot-add") Reported-by: Aswin Unnikrishnan <aswin.u.unnikrishnan@oracle.com> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com> --- hw/pci/pci.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)