Re: [PATCH v2 13/25] hw/i386/acpi-build: Move build_append_notification_callback to pcihp

[Igor Mammedov <imammedo@redhat.com>]

On Tue, 27 May 2025 09:40:15 +0200
Eric Auger <eric.auger@redhat.com> wrote:

> We plan to reuse build_append_notification_callback() on ARM
> so let's move it to pcihp.c.
> 
> No functional change intended.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Reviewed-by: Gustavo Romero <gustavo.romero@linaro.org>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  include/hw/acpi/pcihp.h |  1 +
>  hw/acpi/pcihp.c         | 58 +++++++++++++++++++++++++++++++++++++++++
>  hw/i386/acpi-build.c    | 58 -----------------------------------------
>  3 files changed, 59 insertions(+), 58 deletions(-)
> 
> diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
> index 253ac6e483..f4fd44cb32 100644
> --- a/include/hw/acpi/pcihp.h
> +++ b/include/hw/acpi/pcihp.h
> @@ -78,6 +78,7 @@ void build_acpi_pci_hotplug(Aml *table, AmlRegionSpace rs, uint64_t pcihp_addr);
>  void build_append_pci_dsm_func0_common(Aml *ctx, Aml *retvar);
>  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);
>  
>  /* Called on reset */
>  void acpi_pcihp_reset(AcpiPciHpState *s);
> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> index 310a5c54bd..907a08ac7f 100644
> --- a/hw/acpi/pcihp.c
> +++ b/hw/acpi/pcihp.c
> @@ -39,6 +39,7 @@
>  #include "migration/vmstate.h"
>  #include "qapi/error.h"
>  #include "qom/qom-qobject.h"
> +#include "qobject/qnum.h"
>  #include "trace.h"
>  
>  #define ACPI_PCIHP_SIZE 0x0018
> @@ -705,6 +706,63 @@ void build_append_pcihp_resources(Aml *scope /* \\_SB.PCI0 */,
>      aml_append(scope, dev);
>  }
>  
> +bool build_append_notification_callback(Aml *parent_scope, const PCIBus *bus)
> +{
> +    Aml *method;
> +    PCIBus *sec;
> +    QObject *bsel;
> +    int nr_notifiers = 0;
> +    GQueue *pcnt_bus_list = g_queue_new();
> +
> +    QLIST_FOREACH(sec, &bus->child, sibling) {
> +        Aml *br_scope = aml_scope("S%.02X", sec->parent_dev->devfn);
> +        if (pci_bus_is_root(sec)) {
> +            continue;
> +        }
> +        nr_notifiers = nr_notifiers +
> +                       build_append_notification_callback(br_scope, sec);
> +        /*
> +         * add new child scope to parent
> +         * and keep track of bus that have PCNT,
> +         * bus list is used later to call children PCNTs from this level PCNT
> +         */
> +        if (nr_notifiers) {
> +            g_queue_push_tail(pcnt_bus_list, sec);
> +            aml_append(parent_scope, br_scope);
> +        }
> +    }
> +
> +    /*
> +     * Append PCNT method to notify about events on local and child buses.
> +     * ps: hostbridge might not have hotplug (bsel) enabled but might have
> +     * child bridges that do have bsel.
> +     */
> +    method = aml_method("PCNT", 0, AML_NOTSERIALIZED);
> +
> +    /* If bus supports hotplug select it and notify about local events */
> +    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(method, aml_store(aml_int(bsel_val), aml_name("BNUM")));
> +        aml_append(method, aml_call2("DVNT", aml_name("PCIU"),
> +                                     aml_int(1))); /* Device Check */
> +        aml_append(method, aml_call2("DVNT", aml_name("PCID"),
> +                                     aml_int(3))); /* Eject Request */
> +        nr_notifiers++;
> +    }
> +
> +    /* Notify about child bus events in any case */
> +    while ((sec = g_queue_pop_head(pcnt_bus_list))) {
> +        aml_append(method, aml_name("^S%.02X.PCNT", sec->parent_dev->devfn));
> +    }
> +
> +    aml_append(parent_scope, method);
> +    qobject_unref(bsel);
> +    g_queue_free(pcnt_bus_list);
> +    return !!nr_notifiers;
> +}
> +
>  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 6ca2b34ef8..3275675e60 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -563,64 +563,6 @@ void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus)
>      }
>  }
>  
> -static bool build_append_notification_callback(Aml *parent_scope,
> -                                               const PCIBus *bus)
> -{
> -    Aml *method;
> -    PCIBus *sec;
> -    QObject *bsel;
> -    int nr_notifiers = 0;
> -    GQueue *pcnt_bus_list = g_queue_new();
> -
> -    QLIST_FOREACH(sec, &bus->child, sibling) {
> -        Aml *br_scope = aml_scope("S%.02X", sec->parent_dev->devfn);
> -        if (pci_bus_is_root(sec)) {
> -            continue;
> -        }
> -        nr_notifiers = nr_notifiers +
> -                       build_append_notification_callback(br_scope, sec);
> -        /*
> -         * add new child scope to parent
> -         * and keep track of bus that have PCNT,
> -         * bus list is used later to call children PCNTs from this level PCNT
> -         */
> -        if (nr_notifiers) {
> -            g_queue_push_tail(pcnt_bus_list, sec);
> -            aml_append(parent_scope, br_scope);
> -        }
> -    }
> -
> -    /*
> -     * Append PCNT method to notify about events on local and child buses.
> -     * ps: hostbridge might not have hotplug (bsel) enabled but might have
> -     * child bridges that do have bsel.
> -     */
> -    method = aml_method("PCNT", 0, AML_NOTSERIALIZED);
> -
> -    /* If bus supports hotplug select it and notify about local events */
> -    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(method, aml_store(aml_int(bsel_val), aml_name("BNUM")));
> -        aml_append(method, aml_call2("DVNT", aml_name("PCIU"),
> -                                     aml_int(1))); /* Device Check */
> -        aml_append(method, aml_call2("DVNT", aml_name("PCID"),
> -                                     aml_int(3))); /* Eject Request */
> -        nr_notifiers++;
> -    }
> -
> -    /* Notify about child bus events in any case */
> -    while ((sec = g_queue_pop_head(pcnt_bus_list))) {
> -        aml_append(method, aml_name("^S%.02X.PCNT", sec->parent_dev->devfn));
> -    }
> -
> -    aml_append(parent_scope, method);
> -    qobject_unref(bsel);
> -    g_queue_free(pcnt_bus_list);
> -    return !!nr_notifiers;
> -}
> -
>  /*
>   * build_prt - Define interrupt routing rules
>   *