[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: |
Daniel P . Berrangé |
Subject: |
Re: [PULL v2 11/19] pci: acpi: ensure that acpi-index is unique |
Date: |
Tue, 6 Apr 2021 15:54:24 +0100 |
User-agent: |
Mutt/2.0.5 (2021-01-21) |
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....
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|