[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v6 5/5] hw/pci: ensure PCIE devices are plugged into only slo
From: |
Ani Sinha |
Subject: |
Re: [PATCH v6 5/5] hw/pci: ensure PCIE devices are plugged into only slot 0 of PCIE port |
Date: |
Thu, 29 Jun 2023 21:27:00 +0530 |
> On 29-Jun-2023, at 9:02 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Thu, Jun 29, 2023 at 08:07:57PM +0530, Ani Sinha wrote:
>>
>>
>>> On 29-Jun-2023, at 7:54 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>>>
>>> On Thu, Jun 29, 2023 at 09:37:07AM +0530, Ani Sinha wrote:
>>>> PCI Express ports only have one slot, so PCI Express devices can only be
>>>> plugged into slot 0 on a PCIE port. Enforce it.
>>>>
>>>> The change has been tested to not break ARI by instantiating seven vfs on
>>>> an
>>>> emulated igb device (the maximum number of vfs the linux igb driver
>>>> supports).
>>>
>>> I guess we need to test with some other device then? 7 VFs is same
>>> slot so hardly a good test.
>>
>> No its not the same slot. Its using different slots/device numbers. I
>> checked that.
>> The same patch was failing without the vf check.
>
> Ah, playing with VF stride? Could you show the command line please?
Akhido mentioned this in the other thread. Basically For QEMU:
-device pcie-root-port,id=p -device igb,bus=p
Then from within the guest (in my case RHEL 9.2):
$ echo 7 > /sys/bus/pci/devices/0000\:01\:00.0/sriov_numvfs
You’ll find that if you use something more than 7 there will be ERANGE from the
guest kernel because the driver can create maximum 7 vfs.
This above command line will fail if we do not check for !vfs in the patch with
the following error from QEMU:
(qemu) qemu-system-x86_64: PCI: slot 16 is not valid for igbvf, parent device
only allows plugging into slot 0.
and an IO error on the write from the guest kernel.
In the current version of the patch with the vf check, you will find the vfs
created with the addresses:
01:10.{2,4,6,8} and 01.11.{2,4,6} , that is bus 1 for the root port, devices 10
and 11, functions 2,4,6,8 etc.
There would be no error from QEMU.
>
>>>
>>>> The vfs are seen to have non-zero device/slot numbers in the conventional
>>>> PCI BDF representation.
>>>>
>>>> CC: jusual@redhat.com
>>>> CC: imammedo@redhat.com
>>>> CC: akihiko.odaki@daynix.com
>>>>
>>>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2128929
>>>> Signed-off-by: Ani Sinha <anisinha@redhat.com>
>>>> Reviewed-by: Julia Suvorova <jusual@redhat.com>
>>>> ---
>>>> hw/pci/pci.c | 15 +++++++++++++++
>>>> 1 file changed, 15 insertions(+)
>>>>
>>>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
>>>> index e2eb4c3b4a..0320ac2bb3 100644
>>>> --- a/hw/pci/pci.c
>>>> +++ b/hw/pci/pci.c
>>>> @@ -65,6 +65,7 @@ bool pci_available = true;
>>>> static char *pcibus_get_dev_path(DeviceState *dev);
>>>> static char *pcibus_get_fw_dev_path(DeviceState *dev);
>>>> static void pcibus_reset(BusState *qbus);
>>>> +static bool pcie_has_upstream_port(PCIDevice *dev);
>>>>
>>>> static Property pci_props[] = {
>>>> DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
>>>> @@ -1190,6 +1191,20 @@ static PCIDevice *do_pci_register_device(PCIDevice
>>>> *pci_dev,
>>>> name);
>>>>
>>>> return NULL;
>>>> + } /*
>>>> + * With SRIOV and ARI, vfs can have non-zero slot in the
>>>> conventional
>>>> + * PCI interpretation as all five bits reserved for slot addresses
>>>> are
>>>> + * also used for function bits for the various vfs. Ignore that
>>>> case.
>>>> + * It is too early here to check for ARI capabilities in the PCI
>>>> config
>>>> + * space. Hence, we check for a vf device instead.
>>>> + */
>>>> + else if (!pci_is_vf(pci_dev) &&
>>>> + pcie_has_upstream_port(pci_dev) &&
>>>> + PCI_SLOT(devfn)) {
>>>> + error_setg(errp, "PCI: slot %d is not valid for %s,"
>>>> + " parent device only allows plugging into slot 0.",
>>>> + PCI_SLOT(devfn), name);
>>>> + return NULL;
>>>> }
>>>>
>>>> pci_dev->devfn = devfn;
>>>> --
>>>> 2.39.1
- Re: [PATCH v6 5/5] hw/pci: ensure PCIE devices are plugged into only slot 0 of PCIE port, (continued)
- Re: [PATCH v6 5/5] hw/pci: ensure PCIE devices are plugged into only slot 0 of PCIE port, Ani Sinha, 2023/06/29
- Re: [PATCH v6 5/5] hw/pci: ensure PCIE devices are plugged into only slot 0 of PCIE port, Akihiko Odaki, 2023/06/29
- Re: [PATCH v6 5/5] hw/pci: ensure PCIE devices are plugged into only slot 0 of PCIE port, Michael S. Tsirkin, 2023/06/30
- Re: [PATCH v6 5/5] hw/pci: ensure PCIE devices are plugged into only slot 0 of PCIE port, Ani Sinha, 2023/06/30
- Re: [PATCH v6 5/5] hw/pci: ensure PCIE devices are plugged into only slot 0 of PCIE port, Michael S. Tsirkin, 2023/06/30
- Re: [PATCH v6 5/5] hw/pci: ensure PCIE devices are plugged into only slot 0 of PCIE port, Ani Sinha, 2023/06/30
- Re: [PATCH v6 5/5] hw/pci: ensure PCIE devices are plugged into only slot 0 of PCIE port, Michael S. Tsirkin, 2023/06/30
Re: [PATCH v6 5/5] hw/pci: ensure PCIE devices are plugged into only slot 0 of PCIE port, Michael S. Tsirkin, 2023/06/29
[PATCH v6 2/5] tests/acpi/bios-tables-test: use the correct slot on the pcie-root-port, Ani Sinha, 2023/06/29
[PATCH v6 4/5] tests/qtest/hd-geo-test: fix incorrect pcie-root-port usage and simplify test, Ani Sinha, 2023/06/29
[PATCH v6 3/5] tests/acpi/bios-tables-test: update acpi blob q35/DSDT.noacpihp, Ani Sinha, 2023/06/29