From: Igor Mammedov <imammedo@redhat.com>
To: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
Julia Suvorova <jusual@redhat.com>,
qemu-devel@nongnu.org, "Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [PULL v2 11/19] pci: acpi: ensure that acpi-index is unique
Date: Tue, 6 Apr 2021 20:15:46 +0200 [thread overview]
Message-ID: <20210406201546.2377830e@redhat.com> (raw)
In-Reply-To: <YGx5LRiqkKRmO4aJ@redhat.com>
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
next prev parent reply other threads:[~2021-04-06 18:17 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-22 22:59 [PULL v2 00/19] pc,virtio,pci: fixes, features Michael S. Tsirkin
2021-03-22 22:59 ` [PULL v2 01/19] virtio: Fix virtio_mmio_read()/virtio_mmio_write() Michael S. Tsirkin
2021-03-22 22:59 ` [PULL v2 02/19] vhost-user: Drop misleading EAGAIN checks in slave_read() Michael S. Tsirkin
2021-03-22 22:59 ` [PULL v2 03/19] vhost-user: Fix double-close on slave_read() error path Michael S. Tsirkin
2021-03-22 23:00 ` [PULL v2 04/19] vhost-user: Factor out duplicated slave_fd teardown code Michael S. Tsirkin
2021-03-22 23:00 ` [PULL v2 05/19] vhost-user: Convert slave channel to QIOChannelSocket Michael S. Tsirkin
2021-03-22 23:00 ` [PULL v2 06/19] vhost-user: Introduce nested event loop in vhost_user_read() Michael S. Tsirkin
2021-03-22 23:00 ` [PULL v2 07/19] vhost-user: Monitor slave channel " Michael S. Tsirkin
2021-03-22 23:00 ` [PULL v2 08/19] virtio-pmem: fix virtio_pmem_resp assign problem Michael S. Tsirkin
2021-03-22 23:00 ` [PULL v2 09/19] tests: acpi: temporary whitelist DSDT changes Michael S. Tsirkin
2021-03-22 23:00 ` [PULL v2 10/19] pci: introduce acpi-index property for PCI device Michael S. Tsirkin
2021-03-22 23:00 ` [PULL v2 11/19] pci: acpi: ensure that acpi-index is unique Michael S. Tsirkin
2021-04-06 14:54 ` Daniel P. Berrangé
2021-04-06 15:07 ` Daniel P. Berrangé
2021-04-06 18:15 ` Igor Mammedov [this message]
2021-04-07 8:29 ` Daniel P. Berrangé
2021-04-07 21:25 ` Igor Mammedov
2021-04-07 13:29 ` Michael S. Tsirkin
2021-04-07 21:23 ` Igor Mammedov
2021-04-07 13:27 ` Michael S. Tsirkin
2021-03-22 23:00 ` [PULL v2 12/19] acpi: add aml_to_decimalstring() and aml_call6() helpers Michael S. Tsirkin
2021-03-22 23:00 ` [PULL v2 13/19] pci: acpi: add _DSM method to PCI devices Michael S. Tsirkin
2021-03-22 23:00 ` [PULL v2 14/19] tests: acpi: update expected blobs Michael S. Tsirkin
2021-03-23 14:12 ` Michael S. Tsirkin
2021-03-22 23:00 ` [PULL v2 15/19] acpi: Set proper maximum size for "etc/table-loader" blob Michael S. Tsirkin
2021-03-22 23:00 ` [PULL v2 17/19] acpi: Move maximum size logic into acpi_add_rom_blob() Michael S. Tsirkin
2021-03-22 23:00 ` [PULL v2 18/19] acpi: Set proper maximum size for "etc/acpi/rsdp" blob Michael S. Tsirkin
2021-03-22 23:00 ` [PULL v2 19/19] acpi: Move setters/getters of oem fields to X86MachineState Michael S. Tsirkin
2021-03-23 15:30 ` [PULL v2 00/19] pc,virtio,pci: fixes, features Peter Maydell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210406201546.2377830e@redhat.com \
--to=imammedo@redhat.com \
--cc=berrange@redhat.com \
--cc=jusual@redhat.com \
--cc=mst@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).