All of lore.kernel.org
 help / color / mirror / Atom feed
From: Igor Mammedov <imammedo@redhat.com>
To: qemu-devel@nongnu.org
Cc: Laurent Vivier <lvivier@redhat.com>,
	peter.maydell@linaro.org, jusual@redhat.com, mst@redhat.com
Subject: [PATCH 2/2] acpi: x86: pcihp: add support hotplug on multifunction bridges
Date: Thu, 22 Jul 2021 06:59:45 -0400	[thread overview]
Message-ID: <20210722105945.2080428-3-imammedo@redhat.com> (raw)
In-Reply-To: <20210722105945.2080428-1-imammedo@redhat.com>

Commit 17858a1695 (hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35)
switched PCI hotplug from native to ACPI one by default.

That however breaks ihotplug on following CLI that used to work:
   -nodefaults -machine q35 \
   -device pcie-root-port,id=pcie-root-port-0,multifunction=on,bus=pcie.0,addr=0x1,chassis=1 \
   -device pcie-root-port,id=pcie-root-port-1,port=0x1,addr=0x1.0x1,bus=pcie.0,chassis=2

where PCI device is hotplugged to pcie-root-port-1 with error on guest side:

  ACPI BIOS Error (bug): Could not resolve symbol [^S0B.PCNT], AE_NOT_FOUND (20201113/psargs-330)
  ACPI Error: Aborting method \_SB.PCI0.PCNT due to previous error (AE_NOT_FOUND) (20201113/psparse-531)
  ACPI Error: Aborting method \_GPE._E01 due to previous error (AE_NOT_FOUND) (20201113/psparse-531)
  ACPI Error: AE_NOT_FOUND, while evaluating GPE method [_E01] (20201113/evgpe-515)

cause is that QEMU's ACPI hotplug never supported functions other then 0
and due to bug it was generating notification entries for not described
functions.

Technically there is no reason not to describe cold-plugged bridges
(root ports) on functions other then 0, as they similaraly to bridge
on function 0 are unpluggable.

Fix consists of describing cold-plugged bridges[root ports] on functions
other than 0.

Fixes: 17858a169508609ca9063c544833e5a1adeb7b52
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reported-by: Laurent Vivier <lvivier@redhat.com>
---
 hw/i386/acpi-build.c | 31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b40e284b72..77cb7a338a 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -364,7 +364,7 @@ static void build_append_pcihp_notify_entry(Aml *method, int slot)
     int32_t devfn = PCI_DEVFN(slot, 0);
 
     if_ctx = aml_if(aml_and(aml_arg(0), aml_int(0x1U << slot), NULL));
-    aml_append(if_ctx, aml_notify(aml_name("S%.02X", devfn), aml_arg(1)));
+    aml_append(if_ctx, aml_notify(aml_name("^S%.03X", devfn), aml_arg(1)));
     aml_append(method, if_ctx);
 }
 
@@ -389,18 +389,26 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
         PCIDeviceClass *pc;
         PCIDevice *pdev = bus->devices[devfn];
         int slot = PCI_SLOT(devfn);
+        int func = PCI_FUNC(devfn);
+        /* ACPI spec: 1.0b: Table 6-2 _ADR Object Bus Types, PCI type */
+        int adr = slot << 16 | func;
         bool hotplug_enabled_dev;
         bool bridge_in_acpi;
         bool cold_plugged_bridge;
 
         if (!pdev) {
-            if (bsel) { /* add hotplug slots for non present devices */
+            /*
+             * add hotplug slots for non present devices.
+             * hotplug is supported only for non-multifunction device
+             * so generate device description only for function 0
+             */
+            if (bsel && !PCI_FUNC(devfn)) {
                 if (pci_bus_is_express(bus) && slot > 0) {
                     break;
                 }
-                dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
+                dev = aml_device("S%.03X", devfn);
                 aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
-                aml_append(dev, aml_name_decl("_ADR", aml_int(slot << 16)));
+                aml_append(dev, aml_name_decl("_ADR", aml_int(adr)));
                 method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
                 aml_append(method,
                     aml_call2("PCEJ", aml_name("BSEL"), aml_name("_SUN"))
@@ -436,9 +444,18 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
             continue;
         }
 
+        /*
+         * allow describing coldplugged bridges in ACPI even if they are not
+         * on function 0, as they are not unpluggale, for all other devices
+         * generate description only for function 0 per slot
+         */
+        if (PCI_FUNC(devfn) && !bridge_in_acpi) {
+            continue;
+        }
+
         /* start to compose PCI slot descriptor */
-        dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
-        aml_append(dev, aml_name_decl("_ADR", aml_int(slot << 16)));
+        dev = aml_device("S%.03X", devfn);
+        aml_append(dev, aml_name_decl("_ADR", aml_int(adr)));
 
         if (bsel) {
             /*
@@ -529,7 +546,7 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
                     continue;
                 }
 
-                aml_append(method, aml_name("^S%.02X.PCNT",
+                aml_append(method, aml_name("^S%.03X.PCNT",
                                             sec->parent_dev->devfn));
             }
         }
-- 
2.27.0



  parent reply	other threads:[~2021-07-22 11:04 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-22 10:59 [PATCH 0/2] acpi: pcihp: fix hotplug when bridge is wired to function > 0 Igor Mammedov
2021-07-22 10:59 ` [PATCH 1/2] acpi: x86: pcihp: cleanup devfn usage in build_append_pci_bus_devices() Igor Mammedov
2021-07-22 17:56   ` Michael S. Tsirkin
2021-07-23  7:34     ` Igor Mammedov
2021-07-22 10:59 ` Igor Mammedov [this message]
2021-07-22 12:38   ` [PATCH 2/2] acpi: x86: pcihp: add support hotplug on multifunction bridges Laurent Vivier
2021-07-22 17:49   ` Michael S. Tsirkin
2021-07-22 18:13     ` Laurent Vivier
2021-07-23  7:47     ` Igor Mammedov
2021-07-23  8:05       ` Michael S. Tsirkin
2021-07-23  8:33         ` Igor Mammedov
2021-07-23  8:48     ` Daniel P. Berrangé
2021-07-23  9:34       ` Michael S. Tsirkin
2021-07-23  8:50   ` Daniel P. Berrangé
2021-07-22 17:56 ` [PATCH 0/2] acpi: pcihp: fix hotplug when bridge is wired to function > 0 Michael S. Tsirkin

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=20210722105945.2080428-3-imammedo@redhat.com \
    --to=imammedo@redhat.com \
    --cc=jusual@redhat.com \
    --cc=lvivier@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.