qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: qemu-devel@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>,
	Igor Mammedov <imammedo@redhat.com>, Ani Sinha <ani@anisinha.ca>,
	Marcel Apfelbaum <marcel.apfelbaum@gmail.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Richard Henderson <richard.henderson@linaro.org>,
	Eduardo Habkost <eduardo@habkost.net>
Subject: [PULL 43/56] pcihp: acpi: decouple hotplug and generic slots description
Date: Mon, 30 Jan 2023 15:21:05 -0500	[thread overview]
Message-ID: <20230130201810.11518-44-mst@redhat.com> (raw)
In-Reply-To: <20230130201810.11518-1-mst@redhat.com>

From: Igor Mammedov <imammedo@redhat.com>

Split build_append_pci_bus_devices() onto generic part that builds
AML descriptions only for populated slots which is applicable to
both hotplug disabled and enabled bridges. And a hotplug only
part that complements generic AML with hotplug depended bits
(that depend on BSEL), like _SUN/_EJ0 entries, dynamic _DSM.

Hotplug part, will generate full 'Device' descriptors for
non-populated slots (like it used to be) and complementary
'Scope' descriptors for populated slots that are hotplug capable.
i.e. something like this:
  - ...
  +                Name (BSEL, 0x03)
  +                Scope (S00)
  +                {
  +                    Name (ASUN, Zero)
  +                    Method (_DSM, 4, Serialized)  // _DSM: Device-Specific Method
  +                    {
  +                        Local0 = Package (0x02)
  +                            {
  +                                BSEL,
  +                                ASUN
  +                            }
  +                        Return (PDSM (Arg0, Arg1, Arg2, Arg3, Local0))
  +                    }
  +  [ ... other hotplug depended bits ]
  +                }

While generic build_append_pci_bus_devices() still calls hotplug part at
its end it doesn't really depend on any hotplug bits anymore and later
both could be completely separated when it's necessary.

Main benefit though is that both build_append_pci_bus_devices() and
build_append_pcihp_slots() become more readable and it makes easier
to modify them with less risk of affecting another part. Also it opens
possibility to re-use generic part elsewhere (microvm, arm/virt).

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <20230112140312.3096331-34-imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/acpi-build.c | 121 +++++++++++++++++++++++++------------------
 1 file changed, 72 insertions(+), 49 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b4c9ff4794..2077efbee4 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -383,35 +383,40 @@ static void build_append_pcihp_notify_entry(Aml *method, int slot)
     aml_append(method, if_ctx);
 }
 
-static bool is_devfn_ignored(const int devfn, const PCIBus *bus,
-                             bool bus_has_hotplug)
+static bool is_devfn_ignored_generic(const int devfn, const PCIBus *bus)
 {
     const PCIDevice *pdev = bus->devices[devfn];
 
-    if (pdev) {
-        if (PCI_FUNC(devfn)) {
-            if (IS_PCI_BRIDGE(pdev)) {
-                /*
-                 * Ignore only hotplugged PCI bridges on !0 functions, but
-                 * allow describing cold plugged bridges on all functions
-                 */
-                if (DEVICE(pdev)->hotplugged) {
-                    return true;
-                }
-            } else if (!get_dev_aml_func(DEVICE(pdev))) {
-                /*
-                 * Ignore all other devices on !0 functions unless they
-                 * have AML description (i.e have get_dev_aml_func() != 0)
-                 */
+    if (PCI_FUNC(devfn)) {
+        if (IS_PCI_BRIDGE(pdev)) {
+            /*
+             * Ignore only hotplugged PCI bridges on !0 functions, but
+             * allow describing cold plugged bridges on all functions
+             */
+            if (DEVICE(pdev)->hotplugged) {
                 return true;
             }
+        } else if (!get_dev_aml_func(DEVICE(pdev))) {
+            /*
+             * Ignore all other devices on !0 functions unless they
+             * have AML description (i.e have get_dev_aml_func() != 0)
+             */
+            return true;
         }
+    }
+    return false;
+}
+
+static bool is_devfn_ignored_hotplug(const int devfn, const PCIBus *bus)
+{
+    if (bus->devices[devfn]) {
+        return is_devfn_ignored_generic(devfn, bus);
     } else { /* non populated slots */
-        /*
+         /*
          * hotplug is supported only for non-multifunction device
          * so generate device description only for function 0
          */
-        if (!bus_has_hotplug || PCI_FUNC(devfn) ||
+        if (PCI_FUNC(devfn) ||
             (pci_bus_is_express(bus) && PCI_SLOT(devfn) > 0)) {
             return true;
         }
@@ -419,29 +424,23 @@ static bool is_devfn_ignored(const int devfn, const PCIBus *bus,
     return false;
 }
 
-void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus)
+static void build_append_pcihp_slots(Aml *parent_scope, PCIBus *bus,
+                                     QObject *bsel)
 {
-    Aml *dev, *notify_method = NULL, *method;
-    QObject *bsel;
     int devfn;
+    Aml *dev, *notify_method = NULL, *method;
+    uint64_t bsel_val = qnum_get_uint(qobject_to(QNum, bsel));
 
-    bsel = object_property_get_qobject(OBJECT(bus), ACPI_PCIHP_PROP_BSEL, NULL);
-    if (bsel) {
-        uint64_t bsel_val = qnum_get_uint(qobject_to(QNum, bsel));
-
-        aml_append(parent_scope, aml_name_decl("BSEL", aml_int(bsel_val)));
-        notify_method = aml_method("DVNT", 2, AML_NOTSERIALIZED);
-    }
+    aml_append(parent_scope, aml_name_decl("BSEL", aml_int(bsel_val)));
+    notify_method = aml_method("DVNT", 2, AML_NOTSERIALIZED);
 
     for (devfn = 0; devfn < ARRAY_SIZE(bus->devices); devfn++) {
         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;
+        int adr = slot << 16 | PCI_FUNC(devfn);
         bool hotpluggbale_slot = true;
 
-        if (is_devfn_ignored(devfn, bus, !!bsel)) {
+        if (is_devfn_ignored_hotplug(devfn, bus)) {
             continue;
         }
 
@@ -452,24 +451,20 @@ void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus)
              */
             bool cold_plugged_bridge = IS_PCI_BRIDGE(pdev) &&
                                   !DEVICE(pdev)->hotplugged;
-            hotpluggbale_slot = bsel && DEVICE_GET_CLASS(pdev)->hotpluggable &&
+            hotpluggbale_slot = DEVICE_GET_CLASS(pdev)->hotpluggable &&
                                 !cold_plugged_bridge;
+            dev = aml_scope("S%.02X", devfn);
+        } else {
+            dev = aml_device("S%.02X", devfn);
+            aml_append(dev, aml_name_decl("_ADR", aml_int(adr)));
         }
 
-        /* start to compose PCI device descriptor */
-        dev = aml_device("S%.02X", devfn);
-        aml_append(dev, aml_name_decl("_ADR", aml_int(adr)));
-
-        if (bsel) {
-            /*
-             * Can't declare _SUN here for every device as it changes 'slot'
-             * enumeration order in linux kernel, so use another variable for it
-             */
-            aml_append(dev, aml_name_decl("ASUN", aml_int(slot)));
-            aml_append(dev, aml_pci_device_dsm());
-        }
-
-        call_dev_aml_func(DEVICE(pdev), dev);
+        /*
+         * Can't declare _SUN here for every device as it changes 'slot'
+         * enumeration order in linux kernel, so use another variable for it
+         */
+        aml_append(dev, aml_name_decl("ASUN", aml_int(slot)));
+        aml_append(dev, aml_pci_device_dsm());
 
         if (hotpluggbale_slot) {
             aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
@@ -486,9 +481,37 @@ void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus)
         /* device descriptor has been composed, add it into parent context */
         aml_append(parent_scope, dev);
     }
+    aml_append(parent_scope, notify_method);
+}
+
+void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus)
+{
+    QObject *bsel;
+    int devfn;
+    Aml *dev;
+
+    bsel = object_property_get_qobject(OBJECT(bus), ACPI_PCIHP_PROP_BSEL, NULL);
+
+    for (devfn = 0; devfn < ARRAY_SIZE(bus->devices); devfn++) {
+        /* ACPI spec: 1.0b: Table 6-2 _ADR Object Bus Types, PCI type */
+        int adr = PCI_SLOT(devfn) << 16 | PCI_FUNC(devfn);
+
+        if (!bus->devices[devfn] || is_devfn_ignored_generic(devfn, bus)) {
+            continue;
+        }
+
+        /* start to compose PCI device descriptor */
+        dev = aml_device("S%.02X", devfn);
+        aml_append(dev, aml_name_decl("_ADR", aml_int(adr)));
+
+        call_dev_aml_func(DEVICE(bus->devices[devfn]), dev);
+
+        /* device descriptor has been composed, add it into parent context */
+        aml_append(parent_scope, dev);
+    }
 
     if (bsel) {
-        aml_append(parent_scope, notify_method);
+        build_append_pcihp_slots(parent_scope, bus, bsel);
     }
 
     qobject_unref(bsel);
-- 
MST



  parent reply	other threads:[~2023-01-30 20:32 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-30 20:18 [PULL 00/56] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
2023-01-30 20:18 ` [PULL 01/56] shpc: disallow unplug when power indicator is blinking Michael S. Tsirkin
2023-01-30 20:18 ` [PULL 02/56] hw/i386/acpi-build: Remove unused attributes Michael S. Tsirkin
2023-01-30 20:18 ` [PULL 03/56] hw/isa/isa-bus: Turn isa_build_aml() into qbus_build_aml() Michael S. Tsirkin
2023-01-30 20:18 ` [PULL 04/56] hw/acpi/piix4: No need to #include "hw/southbridge/piix.h" Michael S. Tsirkin
2023-01-30 20:18 ` [PULL 05/56] hw/acpi/acpi_dev_interface: Remove unused parameter from AcpiDeviceIfClass::madt_cpu Michael S. Tsirkin
2023-01-30 20:19 ` [PULL 06/56] vhost-user: Correct a reference of TARGET_AARCH64 Michael S. Tsirkin
2023-01-30 20:19 ` [PULL 07/56] hw/pci-host: Use register definitions from PCI standard Michael S. Tsirkin
2023-01-30 20:19 ` [PULL 08/56] virtio-rng-pci: fix migration compat for vectors Michael S. Tsirkin
2023-01-30 20:19 ` [PULL 09/56] intel-iommu: Document iova_tree Michael S. Tsirkin
2023-01-30 20:19 ` [PULL 12/56] tests: acpi: cleanup arguments to make them more readable Michael S. Tsirkin
2023-01-30 20:19 ` [PULL 13/56] tests: acpi: whitelist DSDT blobs for tests that use pci-bridges Michael S. Tsirkin
2023-01-30 20:19 ` [PULL 14/56] tests: acpi: extend pcihp with nested bridges Michael S. Tsirkin
2023-01-30 20:19 ` [PULL 15/56] tests: acpi: update expected blobs Michael S. Tsirkin
2023-01-30 20:19 ` [PULL 16/56] tests: acpi: cleanup use_uefi argument usage Michael S. Tsirkin
2023-01-30 20:19 ` [PULL 17/56] pci_bridge: remove whitespace Michael S. Tsirkin
2023-01-30 20:19 ` [PULL 18/56] x86: acpi: pcihp: clean up duplicate bridge_in_acpi assignment Michael S. Tsirkin
2023-01-30 20:19 ` [PULL 19/56] pci: acpi hotplug: rename x-native-hotplug to x-do-not-expose-native-hotplug-cap Michael S. Tsirkin
2023-01-30 20:19 ` [PULL 20/56] pcihp: piix4: do not call acpi_pcihp_reset() when ACPI PCI hotplug is disabled Michael S. Tsirkin
2023-01-30 20:19 ` [PULL 21/56] pci: acpihp: assign BSEL only to coldplugged bridges Michael S. Tsirkin
2023-01-30 20:19 ` [PULL 22/56] x86: pcihp: fix invalid AML PCNT calls to hotplugged bridges Michael S. Tsirkin
2023-01-30 20:19 ` [PULL 23/56] tests: boot_sector_test: avoid crashing if status is not available yet Michael S. Tsirkin
2023-01-30 20:19 ` [PULL 10/56] x86: don't let decompressed kernel image clobber setup_data Michael S. Tsirkin
2023-01-30 20:19   ` Michael S. Tsirkin
2023-01-31 19:39   ` Jason A. Donenfeld
2023-01-31 21:27     ` Michael S. Tsirkin
2023-01-31 20:54   ` H. Peter Anvin
2023-01-31 21:22     ` Jason A. Donenfeld
2023-02-01  5:40       ` H. Peter Anvin
2023-01-31 23:32   ` Jason A. Donenfeld
2023-01-30 20:20 ` [PULL 11/56] tests: qtest: print device_add error before failing test Michael S. Tsirkin
2023-01-30 20:20 ` [PULL 26/56] tests: acpi: add reboot cycle to bridge test Michael S. Tsirkin
2023-01-30 20:20 ` [PULL 27/56] tests: acpi: whitelist DSDT before refactoring acpi based PCI hotplug machinery Michael S. Tsirkin
2023-01-30 20:20 ` [PULL 28/56] pcihp: drop pcihp_bridge_en dependency when composing PCNT method Michael S. Tsirkin
2023-01-30 20:20 ` [PULL 29/56] tests: acpi: update expected blobs Michael S. Tsirkin
2023-01-30 20:20 ` [PULL 30/56] tests: acpi: whitelist DSDT before refactoring acpi based PCI hotplug machinery Michael S. Tsirkin
2023-01-30 20:20 ` [PULL 24/56] tests: acpi: extend bridge tests with hotplugged bridges Michael S. Tsirkin
2023-01-30 20:20   ` Michael S. Tsirkin
2023-01-30 20:20 ` [PULL 31/56] pcihp: compose PCNT callchain right before its user _GPE._E01 Michael S. Tsirkin
2023-01-30 20:20 ` [PULL 25/56] tests: boot_sector_test(): make it multi-shot Michael S. Tsirkin
2023-01-30 20:20 ` [PULL 32/56] pcihp: do not put empty PCNT in DSDT Michael S. Tsirkin
2023-01-30 20:20 ` [PULL 33/56] tests: acpi: update expected blobs Michael S. Tsirkin
2023-01-30 20:20 ` [PULL 34/56] whitelist DSDT before adding endpoint devices to bridge testcases Michael S. Tsirkin
2023-01-30 20:20 ` [PULL 35/56] tests: acpi: add endpoint devices to bridges Michael S. Tsirkin
2023-01-30 20:20 ` [PULL 36/56] tests: acpi: update expected blobs Michael S. Tsirkin
2023-01-30 20:20 ` [PULL 37/56] x86: pcihp: acpi: prepare slot ignore rule to work with self describing bridges Michael S. Tsirkin
2023-01-30 20:20 ` [PULL 38/56] pci: acpi: wire up AcpiDevAmlIf interface to generic bridge Michael S. Tsirkin
2023-01-30 20:20 ` [PULL 39/56] pcihp: make bridge describe itself using AcpiDevAmlIfClass:build_dev_aml Michael S. Tsirkin
2023-01-30 20:20 ` [PULL 40/56] pci: make sure pci_bus_is_express() won't error out with "discards ‘const’ qualifier" Michael S. Tsirkin
2023-01-30 20:20 ` [PULL 41/56] pcihp: isolate rule whether slot should be described in DSDT Michael S. Tsirkin
2023-01-30 20:21 ` [PULL 42/56] tests: acpi: whitelist DSDT before decoupling PCI hotplug code from basic slots description Michael S. Tsirkin
2023-01-30 20:21 ` Michael S. Tsirkin [this message]
2023-01-30 20:21 ` [PULL 44/56] tests: acpi: update expected blobs Michael S. Tsirkin
2023-01-30 20:21 ` [PULL 45/56] tests: acpi: whitelist DSDT blobs before removing dynamic _DSM on coldplugged bridges Michael S. Tsirkin
2023-01-30 20:21 ` [PULL 46/56] pcihp: acpi: ignore coldplugged bridges when composing hotpluggable slots Michael S. Tsirkin
2023-01-30 20:21 ` [PULL 47/56] tests: acpi: update expected blobs Michael S. Tsirkin
2023-01-30 20:21 ` [PULL 48/56] tests: acpi: whitelist DSDT before moving non-hotpluggble slots description from hotplug path Michael S. Tsirkin
2023-01-30 20:21 ` [PULL 49/56] pcihp: generate populated non-hotpluggble slot descriptions on non-hotplug path Michael S. Tsirkin
2023-01-30 20:21 ` [PULL 50/56] tests: acpi: update expected blobs Michael S. Tsirkin
2023-01-30 20:21 ` [PULL 51/56] vhost-user: Skip unnecessary duplicated VHOST_USER_ADD/REM_MEM_REG requests Michael S. Tsirkin
2023-01-30 20:21 ` [PULL 52/56] hw: Use TYPE_PCI_BUS definition where appropriate Michael S. Tsirkin
2023-01-30 20:21 ` [PULL 53/56] tests/qtest/bios-tables-test: Make the test less verbose by default Michael S. Tsirkin
2023-01-30 20:21 ` [PULL 54/56] Revert "vhost-user: Monitor slave channel in vhost_user_read()" Michael S. Tsirkin
2023-01-30 20:21 ` [PULL 55/56] Revert "vhost-user: Introduce nested event loop " Michael S. Tsirkin
2023-01-30 20:21 ` [PULL 56/56] docs/pcie.txt: Replace ioh3420 with pcie-root-port Michael S. Tsirkin
2023-02-02 13:42 ` [PULL 00/56] virtio,pc,pci: features, cleanups, fixes 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=20230130201810.11518-44-mst@redhat.com \
    --to=mst@redhat.com \
    --cc=ani@anisinha.ca \
    --cc=eduardo@habkost.net \
    --cc=imammedo@redhat.com \
    --cc=marcel.apfelbaum@gmail.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.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).