From: Gustavo Romero <gustavo.romero@linaro.org>
To: Eric Auger <eric.auger@redhat.com>,
eric.auger.pro@gmail.com, qemu-devel@nongnu.org,
qemu-arm@nongnu.org, peter.maydell@linaro.org,
imammedo@redhat.com, anisinha@redhat.com, mst@redhat.com,
shannon.zhaosl@gmail.com
Cc: pbonzini@redhat.com, Jonathan.Cameron@huawei.com, philmd@linaro.org
Subject: Re: [PATCH 12/22] hw/i386/acpi-build: Move build_append_pci_bus_devices/pcihp_slots to pcihp
Date: Tue, 20 May 2025 16:59:42 -0300 [thread overview]
Message-ID: <fdf853d2-35ae-40d7-942e-e01c7331ba34@linaro.org> (raw)
In-Reply-To: <20250514170431.2786231-13-eric.auger@redhat.com>
Hi Eric,
On 5/14/25 14:00, Eric Auger wrote:
> We intend to reuse build_append_pci_bus_devices and
> build_append_pcihp_slots on ARM. So Let's move them to
nit: lowercase ---^
> hw/acpi/pcihp.c as well as all static helpers they
> use.
>
> No functional change intended.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
> include/hw/acpi/pci.h | 1 -
> include/hw/acpi/pcihp.h | 2 +
> hw/acpi/pcihp.c | 173 ++++++++++++++++++++++++++++++++++++++++
> hw/i386/acpi-build.c | 172 ---------------------------------------
> 4 files changed, 175 insertions(+), 173 deletions(-)
Reviewed-by: Gustavo Romero <gustavo.romero@linaro.org>
Cheers,
Gustavo
> diff --git a/include/hw/acpi/pci.h b/include/hw/acpi/pci.h
> index ab0187a894..4dca22c0e2 100644
> --- a/include/hw/acpi/pci.h
> +++ b/include/hw/acpi/pci.h
> @@ -37,7 +37,6 @@ typedef struct AcpiMcfgInfo {
> void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info,
> const char *oem_id, const char *oem_table_id);
>
> -void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus);
> void build_pci_bridge_aml(AcpiDevAmlIf *adev, Aml *scope);
>
> void build_srat_generic_affinity_structures(GArray *table_data);
> diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
> index 7c5d59243f..4d820b4baf 100644
> --- a/include/hw/acpi/pcihp.h
> +++ b/include/hw/acpi/pcihp.h
> @@ -80,6 +80,8 @@ void build_append_pcihp_resources(Aml *table,
> uint64_t io_addr, uint64_t io_len);
> bool build_append_notification_callback(Aml *parent_scope, const PCIBus *bus);
>
> +void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus);
> +
> /* Called on reset */
> void acpi_pcihp_reset(AcpiPciHpState *s);
>
> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> index 907a08ac7f..942669ea89 100644
> --- a/hw/acpi/pcihp.c
> +++ b/hw/acpi/pcihp.c
> @@ -27,6 +27,7 @@
> #include "qemu/osdep.h"
> #include "hw/acpi/pcihp.h"
> #include "hw/acpi/aml-build.h"
> +#include "hw/acpi/acpi_aml_interface.h"
> #include "hw/pci-host/i440fx.h"
> #include "hw/pci/pci.h"
> #include "hw/pci/pci_bridge.h"
> @@ -763,6 +764,178 @@ bool build_append_notification_callback(Aml *parent_scope, const PCIBus *bus)
> return !!nr_notifiers;
> }
>
> +static void build_append_pcihp_notify_entry(Aml *method, int slot)
> +{
> + Aml *if_ctx;
> + 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(method, if_ctx);
> +}
> +
> +static bool is_devfn_ignored_generic(const int devfn, const PCIBus *bus)
> +{
> + const PCIDevice *pdev = bus->devices[devfn];
> +
> + 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;
> + }
> + }
> + }
> + return false;
> +}
> +
> +static bool is_devfn_ignored_hotplug(const int devfn, const PCIBus *bus)
> +{
> + PCIDevice *pdev = bus->devices[devfn];
> + if (pdev) {
> + return is_devfn_ignored_generic(devfn, bus) ||
> + !DEVICE_GET_CLASS(pdev)->hotpluggable ||
> + /* Cold plugged bridges aren't themselves hot-pluggable */
> + (IS_PCI_BRIDGE(pdev) && !DEVICE(pdev)->hotplugged);
> + } else { /* non populated slots */
> + /*
> + * hotplug is supported only for non-multifunction device
> + * so generate device description only for function 0
> + */
> + if (PCI_FUNC(devfn) ||
> + (pci_bus_is_express(bus) && PCI_SLOT(devfn) > 0)) {
> + return true;
> + }
> + }
> + return false;
> +}
> +
> +static Aml *aml_pci_static_endpoint_dsm(PCIDevice *pdev)
> +{
> + Aml *method;
> +
> + g_assert(pdev->acpi_index != 0);
> + method = aml_method("_DSM", 4, AML_SERIALIZED);
> + {
> + Aml *params = aml_local(0);
> + Aml *pkg = aml_package(1);
> + aml_append(pkg, aml_int(pdev->acpi_index));
> + aml_append(method, aml_store(pkg, params));
> + aml_append(method,
> + aml_return(aml_call5("EDSM", aml_arg(0), aml_arg(1),
> + aml_arg(2), aml_arg(3), params))
> + );
> + }
> + return method;
> +}
> +
> +static Aml *aml_pci_device_dsm(void)
> +{
> + Aml *method;
> +
> + method = aml_method("_DSM", 4, AML_SERIALIZED);
> + {
> + Aml *params = aml_local(0);
> + Aml *pkg = aml_package(2);
> + aml_append(pkg, aml_int(0));
> + aml_append(pkg, aml_int(0));
> + aml_append(method, aml_store(pkg, params));
> + aml_append(method,
> + aml_store(aml_name("BSEL"), aml_index(params, aml_int(0))));
> + aml_append(method,
> + aml_store(aml_name("ASUN"), aml_index(params, aml_int(1))));
> + aml_append(method,
> + aml_return(aml_call5("PDSM", aml_arg(0), aml_arg(1),
> + aml_arg(2), aml_arg(3), params))
> + );
> + }
> + return method;
> +}
> +
> +void build_append_pcihp_slots(Aml *parent_scope, PCIBus *bus)
> +{
> + int devfn;
> + Aml *dev, *notify_method = NULL, *method;
> + QObject *bsel = object_property_get_qobject(OBJECT(bus),
> + ACPI_PCIHP_PROP_BSEL, NULL);
> + uint64_t bsel_val = qnum_get_uint(qobject_to(QNum, bsel));
> + qobject_unref(bsel);
> +
> + 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++) {
> + int slot = PCI_SLOT(devfn);
> + int adr = slot << 16 | PCI_FUNC(devfn);
> +
> + if (is_devfn_ignored_hotplug(devfn, bus)) {
> + continue;
> + }
> +
> + if (bus->devices[devfn]) {
> + dev = aml_scope("S%.02X", devfn);
> + } else {
> + dev = aml_device("S%.02X", devfn);
> + aml_append(dev, aml_name_decl("_ADR", aml_int(adr)));
> + }
> +
> + /*
> + * 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());
> +
> + aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
> + /* add _EJ0 to make slot hotpluggable */
> + method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
> + aml_append(method,
> + aml_call2("PCEJ", aml_name("BSEL"), aml_name("_SUN"))
> + );
> + aml_append(dev, method);
> +
> + build_append_pcihp_notify_entry(notify_method, slot);
> +
> + /* 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)
> +{
> + int devfn;
> + Aml *dev;
> +
> + 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);
> + PCIDevice *pdev = bus->devices[devfn];
> +
> + if (!pdev || 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);
> + /* add _DSM if device has acpi-index set */
> + if (pdev->acpi_index &&
> + !object_property_get_bool(OBJECT(pdev), "hotpluggable",
> + &error_abort)) {
> + aml_append(dev, aml_pci_static_endpoint_dsm(pdev));
> + }
> +
> + /* device descriptor has been composed, add it into parent context */
> + aml_append(parent_scope, dev);
> + }
> +}
> +
> const VMStateDescription vmstate_acpi_pcihp_pci_status = {
> .name = "acpi_pcihp_pci_status",
> .version_id = 1,
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index f08ce407c8..06b4b8eed4 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -338,29 +338,6 @@ build_facs(GArray *table_data)
> g_array_append_vals(table_data, reserved, 40); /* Reserved */
> }
>
> -static Aml *aml_pci_device_dsm(void)
> -{
> - Aml *method;
> -
> - method = aml_method("_DSM", 4, AML_SERIALIZED);
> - {
> - Aml *params = aml_local(0);
> - Aml *pkg = aml_package(2);
> - aml_append(pkg, aml_int(0));
> - aml_append(pkg, aml_int(0));
> - aml_append(method, aml_store(pkg, params));
> - aml_append(method,
> - aml_store(aml_name("BSEL"), aml_index(params, aml_int(0))));
> - aml_append(method,
> - aml_store(aml_name("ASUN"), aml_index(params, aml_int(1))));
> - aml_append(method,
> - aml_return(aml_call5("PDSM", aml_arg(0), aml_arg(1),
> - aml_arg(2), aml_arg(3), params))
> - );
> - }
> - return method;
> -}
> -
> static Aml *aml_pci_edsm(void)
> {
> Aml *method, *ifctx;
> @@ -414,155 +391,6 @@ static Aml *aml_pci_edsm(void)
> return method;
> }
>
> -static Aml *aml_pci_static_endpoint_dsm(PCIDevice *pdev)
> -{
> - Aml *method;
> -
> - g_assert(pdev->acpi_index != 0);
> - method = aml_method("_DSM", 4, AML_SERIALIZED);
> - {
> - Aml *params = aml_local(0);
> - Aml *pkg = aml_package(1);
> - aml_append(pkg, aml_int(pdev->acpi_index));
> - aml_append(method, aml_store(pkg, params));
> - aml_append(method,
> - aml_return(aml_call5("EDSM", aml_arg(0), aml_arg(1),
> - aml_arg(2), aml_arg(3), params))
> - );
> - }
> - return method;
> -}
> -
> -static void build_append_pcihp_notify_entry(Aml *method, int slot)
> -{
> - Aml *if_ctx;
> - 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(method, if_ctx);
> -}
> -
> -static bool is_devfn_ignored_generic(const int devfn, const PCIBus *bus)
> -{
> - const PCIDevice *pdev = bus->devices[devfn];
> -
> - 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;
> - }
> - }
> - }
> - return false;
> -}
> -
> -static bool is_devfn_ignored_hotplug(const int devfn, const PCIBus *bus)
> -{
> - PCIDevice *pdev = bus->devices[devfn];
> - if (pdev) {
> - return is_devfn_ignored_generic(devfn, bus) ||
> - !DEVICE_GET_CLASS(pdev)->hotpluggable ||
> - /* Cold plugged bridges aren't themselves hot-pluggable */
> - (IS_PCI_BRIDGE(pdev) && !DEVICE(pdev)->hotplugged);
> - } else { /* non populated slots */
> - /*
> - * hotplug is supported only for non-multifunction device
> - * so generate device description only for function 0
> - */
> - if (PCI_FUNC(devfn) ||
> - (pci_bus_is_express(bus) && PCI_SLOT(devfn) > 0)) {
> - return true;
> - }
> - }
> - return false;
> -}
> -
> -void build_append_pcihp_slots(Aml *parent_scope, PCIBus *bus)
> -{
> - int devfn;
> - Aml *dev, *notify_method = NULL, *method;
> - QObject *bsel = object_property_get_qobject(OBJECT(bus),
> - ACPI_PCIHP_PROP_BSEL, NULL);
> - uint64_t bsel_val = qnum_get_uint(qobject_to(QNum, bsel));
> - qobject_unref(bsel);
> -
> - 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++) {
> - int slot = PCI_SLOT(devfn);
> - int adr = slot << 16 | PCI_FUNC(devfn);
> -
> - if (is_devfn_ignored_hotplug(devfn, bus)) {
> - continue;
> - }
> -
> - if (bus->devices[devfn]) {
> - dev = aml_scope("S%.02X", devfn);
> - } else {
> - dev = aml_device("S%.02X", devfn);
> - aml_append(dev, aml_name_decl("_ADR", aml_int(adr)));
> - }
> -
> - /*
> - * 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());
> -
> - aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
> - /* add _EJ0 to make slot hotpluggable */
> - method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
> - aml_append(method,
> - aml_call2("PCEJ", aml_name("BSEL"), aml_name("_SUN"))
> - );
> - aml_append(dev, method);
> -
> - build_append_pcihp_notify_entry(notify_method, slot);
> -
> - /* 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)
> -{
> - int devfn;
> - Aml *dev;
> -
> - 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);
> - PCIDevice *pdev = bus->devices[devfn];
> -
> - if (!pdev || 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);
> - /* add _DSM if device has acpi-index set */
> - if (pdev->acpi_index &&
> - !object_property_get_bool(OBJECT(pdev), "hotpluggable",
> - &error_abort)) {
> - aml_append(dev, aml_pci_static_endpoint_dsm(pdev));
> - }
> -
> - /* device descriptor has been composed, add it into parent context */
> - aml_append(parent_scope, dev);
> - }
> -}
> -
> /*
> * build_prt - Define interrupt routing rules
> *
next prev parent reply other threads:[~2025-05-20 20:00 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-14 17:00 [PATCH 00/22] ACPI PCI Hotplug support on ARM Eric Auger
2025-05-14 17:00 ` [PATCH 01/22] hw/i386/acpi-build: Make aml_pci_device_dsm() static Eric Auger
2025-05-20 12:30 ` Gustavo Romero
2025-05-25 15:29 ` Philippe Mathieu-Daudé
2025-05-14 17:00 ` [PATCH 02/22] hw/arm/virt: Introduce machine state acpi pcihp flags and props Eric Auger
2025-05-14 17:00 ` [PATCH 03/22] hw/acpi: Rename and move build_x86_acpi_pci_hotplug to pcihp Eric Auger
2025-05-20 12:31 ` Gustavo Romero
2025-05-14 17:00 ` [PATCH 04/22] hw/pci-host/gpex-acpi: Add native_pci_hotplug arg to acpi_dsdt_add_pci_osc Eric Auger
2025-05-14 17:00 ` [PATCH 05/22] hw/pci-host/gpex-acpi: Split host bridge OSC and DSM generation Eric Auger
2025-05-14 17:00 ` [PATCH 06/22] hw/pci-host/gpex-acpi: Propagate hotplug type info from virt machine downto gpex Eric Auger
2025-05-14 17:00 ` [PATCH 07/22] hw/i386/acpi-build: Turn build_q35_osc_method into a generic method Eric Auger
2025-05-20 12:59 ` Gustavo Romero
2025-05-14 17:00 ` [PATCH 08/22] hw/pci-host/gpex-acpi: Use build_pci_host_bridge_osc_method Eric Auger
2025-05-20 14:09 ` Gustavo Romero
2025-05-21 16:12 ` Eric Auger
2025-05-24 10:54 ` Michael S. Tsirkin
2025-05-26 9:16 ` Eric Auger
2025-05-14 17:00 ` [PATCH 09/22] hw/i386/acpi-build: Introduce build_append_pcihp_resources() helper Eric Auger
2025-05-20 18:57 ` Gustavo Romero
2025-05-14 17:00 ` [PATCH 10/22] hw/acpi/pcihp: Add an AmlRegionSpace arg to build_acpi_pci_hotplug Eric Auger
2025-05-20 18:58 ` Gustavo Romero
2025-05-14 17:00 ` [PATCH 11/22] hw/i386/acpi-build: Move build_append_notification_callback to pcihp Eric Auger
2025-05-20 18:58 ` Gustavo Romero
2025-05-14 17:00 ` [PATCH 12/22] hw/i386/acpi-build: Move build_append_pci_bus_devices/pcihp_slots " Eric Auger
2025-05-20 19:59 ` Gustavo Romero [this message]
2025-05-14 17:01 ` [PATCH 13/22] hw/i386/acpi-build: Introduce and use acpi_get_pci_host Eric Auger
2025-05-20 20:00 ` Gustavo Romero
2025-05-26 9:16 ` Eric Auger
2025-05-14 17:01 ` [PATCH 14/22] hw/i386/acpi-build: Move aml_pci_edsm to a generic place Eric Auger
2025-05-21 15:26 ` Gustavo Romero
2025-05-21 15:56 ` Eric Auger
2025-05-21 16:24 ` Gustavo Romero
2025-05-21 20:19 ` Gustavo Romero
2025-05-27 7:18 ` Eric Auger
2025-05-27 19:34 ` Gustavo Romero
2025-05-14 17:01 ` [PATCH 15/22] hw/arm/virt-acpi-build: Modify the DSDT ACPI table to enable ACPI PCI hotplug Eric Auger
2025-05-14 17:01 ` [PATCH 16/22] hw/acpi/ged: Prepare the device to react to PCI hotplug events Eric Auger
2025-05-25 15:34 ` Philippe Mathieu-Daudé
2025-05-14 17:01 ` [PATCH 17/22] hw/acpi/ged: Call pcihp plug callbacks in hotplug handler implementation Eric Auger
2025-05-14 17:01 ` [PATCH 18/22] hw/acpi/ged: Support migration of AcpiPciHpState Eric Auger
2025-05-14 17:01 ` [PATCH 19/22] hw/core/sysbus: Introduce sysbus_mmio_map_name() helper Eric Auger
2025-05-14 17:01 ` [PATCH 20/22] hw/arm/virt: Let virt support pci hotplug/unplug GED event Eric Auger
2025-05-14 17:01 ` [PATCH 21/22] hw/arm/virt: Plug pcihp hotplug/hotunplug callbacks Eric Auger
2025-05-14 17:01 ` [PATCH 22/22] hw/arm/virt: Use ACPI PCI hotplug by default Eric Auger
2025-05-26 5:55 ` [PATCH 00/22] ACPI PCI Hotplug support on ARM Gustavo Romero
2025-05-26 10:21 ` Eric Auger
2025-05-26 13:56 ` Gustavo Romero
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=fdf853d2-35ae-40d7-942e-e01c7331ba34@linaro.org \
--to=gustavo.romero@linaro.org \
--cc=Jonathan.Cameron@huawei.com \
--cc=anisinha@redhat.com \
--cc=eric.auger.pro@gmail.com \
--cc=eric.auger@redhat.com \
--cc=imammedo@redhat.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=philmd@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=shannon.zhaosl@gmail.com \
/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).