[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PULL v2 11/19] pci: acpi: ensure that acpi-index is unique
From: |
Igor Mammedov |
Subject: |
Re: [PULL v2 11/19] pci: acpi: ensure that acpi-index is unique |
Date: |
Tue, 6 Apr 2021 20:15:46 +0200 |
On Tue, 6 Apr 2021 16:07:25 +0100
Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Tue, Apr 06, 2021 at 03:54:24PM +0100, Daniel P. Berrangé wrote:
> > On Mon, Mar 22, 2021 at 07:00:18PM -0400, Michael S. Tsirkin wrote:
> > > From: Igor Mammedov <imammedo@redhat.com>
> > >
> > > it helps to avoid device naming conflicts when guest OS is
> > > configured to use acpi-index for naming.
> > > Spec ialso says so:
> > >
> > > PCI Firmware Specification Revision 3.2
> > > 4.6.7. _DSM for Naming a PCI or PCI Express Device Under Operating
> > > Systems
> > > "
> > > Instance number must be unique under \_SB scope. This instance number
> > > does not have to
> > > be sequential in a given system configuration.
> > > "
> > >
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > Message-Id: <20210315180102.3008391-4-imammedo@redhat.com>
> > > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > ---
> > > hw/acpi/pcihp.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 46 insertions(+)
> > >
> > > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> > > index ceab287bd3..f4cb3c979d 100644
> > > --- a/hw/acpi/pcihp.c
> > > +++ b/hw/acpi/pcihp.c
> > > @@ -52,6 +52,21 @@ typedef struct AcpiPciHpFind {
> > > PCIBus *bus;
> > > } AcpiPciHpFind;
> > >
> > > +static gint g_cmp_uint32(gconstpointer a, gconstpointer b, gpointer
> > > user_data)
> > > +{
> > > + return a - b;
> > > +}
> > > +
> > > +static GSequence *pci_acpi_index_list(void)
> > > +{
> > > + static GSequence *used_acpi_index_list;
> > > +
> > > + if (!used_acpi_index_list) {
> > > + used_acpi_index_list = g_sequence_new(NULL);
> > > + }
> > > + return used_acpi_index_list;
> > > +}
> > > +
> > > static int acpi_pcihp_get_bsel(PCIBus *bus)
> > > {
> > > Error *local_err = NULL;
> > > @@ -277,6 +292,23 @@ void acpi_pcihp_device_pre_plug_cb(HotplugHandler
> > > *hotplug_dev,
> > > ONBOARD_INDEX_MAX);
> > > return;
> > > }
> > > +
> > > + /*
> > > + * make sure that acpi-index is unique across all present PCI devices
> > > + */
> > > + if (pdev->acpi_index) {
> > > + GSequence *used_indexes = pci_acpi_index_list();
> > > +
> > > + if (g_sequence_lookup(used_indexes,
> > > GINT_TO_POINTER(pdev->acpi_index),
> > > + g_cmp_uint32, NULL)) {
> > > + error_setg(errp, "a PCI device with acpi-index = %" PRIu32
> > > + " already exist", pdev->acpi_index);
> > > + return;
> > > + }
> > > + g_sequence_insert_sorted(used_indexes,
> > > + GINT_TO_POINTER(pdev->acpi_index),
> > > + g_cmp_uint32, NULL);
> > > + }
> >
> > This doesn't appear to ensure uniqueness when using PCIe topologies:
> >
> > $ ./build/x86_64-softmmu/qemu-system-x86_64 \
> > -device virtio-net,acpi-index=100 \
> > -device virtio-net,acpi-index=100
> > qemu-system-x86_64: -device virtio-net,acpi-index=100: a PCI device with
> > acpi-index = 100 already exist
> >
> > $ ./build/x86_64-softmmu/qemu-system-x86_64 \
> > -M q35 \
> > -device virtio-net,acpi-index=100
> > -device virtio-net,acpi-index=100
> > ....happily running....
>
> In fact the entire concept doesn't appear to work with Q35 at all as
> implemented.
>
> The 'acpi_index' file in the guest OS never gets created and the NICs
> are still called 'eth0', 'eth1'
>
> Only with i440fx can I can the "enoNNN" based naming to work with
> acpi-index set from QEMU
It is not supported on Q35 yet as it depends on ACPI PCI hotplug infrastructure.
Once Julia is done with porting it to Q35, acpi-index will be pulled along with
it.
> Regards,
> Daniel