On Fri, Aug 21, 2026 at 12:14:26PM +0100, Mark Cave-Ayland wrote:
> Move the ACPI_PCIHP_PROP_BSEL property to the PCIBus object and update all
> callers accordingly.
>
> Since the existing logic checks for the existence of the ACPI_PCIHP_PROP_BSEL
> property to enable the relevant ACPI changes, set the type of the underlying
> variable to uint32_t with a default value of UINT32_MAX indicating that the
> property has not been set.
>
> Signed-off-by: Mark Cave-Ayland <[email protected]>
> Reviewed-by: Marc-André Lureau <[email protected]>
> ---
> include/hw/pci/pci_bus.h | 2 ++
> hw/acpi/pci-bridge.c | 9 ++++++++-
> hw/acpi/pcihp.c | 30 ++++++++++++++----------------
> hw/arm/virt-acpi-build.c | 7 ++++++-
> hw/i386/acpi-build.c | 7 ++++++-
> hw/pci/pci.c | 7 +++++++
> 6 files changed, 43 insertions(+), 19 deletions(-)
>
> diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
> index c738446788..7ecb23d13c 100644
> --- a/include/hw/pci/pci_bus.h
> +++ b/include/hw/pci/pci_bus.h
> @@ -56,6 +56,8 @@ struct PCIBus {
> int *irq_count;
>
> Notifier machine_done;
> +
> + uint32_t acpi_pcihp_bsel_val;
> };
>
> static inline bool pci_bus_is_cxl(PCIBus *bus)
> diff --git a/hw/acpi/pci-bridge.c b/hw/acpi/pci-bridge.c
> index 394a919479..1bfc5431f2 100644
> --- a/hw/acpi/pci-bridge.c
> +++ b/hw/acpi/pci-bridge.c
> @@ -23,6 +23,8 @@ void build_pci_bridge_aml(AcpiDevAmlIf *adev, Aml *scope)
>
> if (!DEVICE(br)->hotplugged) {
> PCIBus *sec_bus = pci_bridge_get_sec_bus(br);
> + Error *local_err = NULL;
> + uint32_t bsel;
>
> build_append_pci_bus_devices(scope, sec_bus);
>
> @@ -30,9 +32,14 @@ void build_pci_bridge_aml(AcpiDevAmlIf *adev, Aml *scope)
> * generate hotplug slots descriptors if
> * bridge has ACPI PCI hotplug attached,
> */
> - if (object_property_find(OBJECT(sec_bus), ACPI_PCIHP_PROP_BSEL)) {
> + bsel = object_property_get_uint(OBJECT(sec_bus),
> ACPI_PCIHP_PROP_BSEL,
> + &local_err);
> +
> + if (local_err == NULL && bsel != UINT32_MAX) {
> build_append_pcihp_slots(scope, sec_bus);
> }
> +
> + error_free(local_err);
IIUC, in this new approach the property should always exist and so I'd
expect object_property_get_uint to always succeed.
IOW, is something stopping us using &error_abort and merely checking
if (bsel != UINT32_MAX)
?
> }
> }
>
> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> index a91f523c93..b26ddf0a26 100644
> --- a/hw/acpi/pcihp.c
> +++ b/hw/acpi/pcihp.c
> @@ -58,7 +58,7 @@ typedef struct AcpiPciHpFind {
> static int acpi_pcihp_get_bsel(PCIBus *bus)
> {
> Error *local_err = NULL;
> - uint64_t bsel = object_property_get_uint(OBJECT(bus),
> ACPI_PCIHP_PROP_BSEL,
> + uint32_t bsel = object_property_get_uint(OBJECT(bus),
> ACPI_PCIHP_PROP_BSEL,
> &local_err);
>
> if (local_err || bsel >= ACPI_PCIHP_MAX_HOTPLUG_BUS) {
> @@ -78,18 +78,14 @@ typedef struct {
> static void *acpi_set_bsel(PCIBus *bus, void *opaque)
> {
> BSELInfo *info = opaque;
> - unsigned *bus_bsel;
> DeviceState *br = bus->qbus.parent;
> bool is_bridge = IS_PCI_BRIDGE(br);
>
> /* hotplugged bridges can't be described in ACPI ignore them */
> if (qbus_is_hotpluggable(BUS(bus))) {
> if (!is_bridge || (!br->hotplugged && info->has_bridge_hotplug)) {
> - bus_bsel = g_malloc(sizeof *bus_bsel);
> -
> - *bus_bsel = info->bsel_alloc++;
> - object_property_add_uint32_ptr(OBJECT(bus), ACPI_PCIHP_PROP_BSEL,
> - bus_bsel, OBJ_PROP_FLAG_READ);
> + object_property_set_uint(OBJECT(bus), ACPI_PCIHP_PROP_BSEL,
> + info->bsel_alloc++, NULL);
> }
> }
>
> @@ -730,14 +726,16 @@ bool build_append_notification_callback(Aml
> *parent_scope, const PCIBus *bus)
> /* 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++;
> + uint32_t bsel_val = qnum_get_uint(qobject_to(QNum, bsel));
> +
> + if (bsel_val != UINT32_MAX) {
> + 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 */
> @@ -848,7 +846,7 @@ void build_append_pcihp_slots(Aml *parent_scope, PCIBus
> *bus)
> 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));
> + uint32_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)));
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index f6386088b6..7ba7b38773 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -1207,6 +1207,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms)
> AcpiTable table = { .sig = "DSDT", .rev = 2, .oem_id = vms->oem_id,
> .oem_table_id = vms->oem_table_id };
> Aml *pci0_scope;
> + Error *local_err = NULL;
> + uint32_t bsel;
>
> acpi_table_begin(&table, table_data);
> dsdt = init_aml_allocator();
> @@ -1269,9 +1271,12 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms)
>
> aml_append(pci0_scope, build_pci_bridge_edsm());
> build_append_pci_bus_devices(pci0_scope, vms->bus);
> - if (object_property_find(OBJECT(vms->bus), ACPI_PCIHP_PROP_BSEL)) {
> + bsel = object_property_get_uint(OBJECT(vms->bus), ACPI_PCIHP_PROP_BSEL,
> + &local_err);
> + if (local_err == NULL && bsel != UINT32_MAX) {
> build_append_pcihp_slots(pci0_scope, vms->bus);
> }
> + error_free(local_err);
>
> if (vms->acpi_dev) {
> bool acpi_pcihp;
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 8837b69687..cc0b6e5633 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1198,15 +1198,20 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> sb_scope = aml_scope("\\_SB");
> {
> Object *pci_host = acpi_get_i386_pci_host();
> + Error *local_err = NULL;
> + uint32_t bsel;
>
> if (pci_host) {
> PCIBus *pbus = PCI_HOST_BRIDGE(pci_host)->bus;
> Aml *ascope = aml_scope("PCI0");
> /* Scan all PCI buses. Generate tables to support hotplug. */
> build_append_pci_bus_devices(ascope, pbus);
> - if (object_property_find(OBJECT(pbus), ACPI_PCIHP_PROP_BSEL)) {
> + bsel = object_property_get_uint(OBJECT(pbus),
> ACPI_PCIHP_PROP_BSEL,
> + &local_err);
> + if (local_err == NULL && bsel != UINT32_MAX) {
> build_append_pcihp_slots(ascope, pbus);
> }
> + error_free(local_err);
> aml_append(sb_scope, ascope);
> }
> }
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index d3191609e2..e8e8a3b767 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -25,6 +25,7 @@
> #include "qemu/osdep.h"
> #include "qemu/datadir.h"
> #include "qemu/units.h"
> +#include "hw/acpi/pcihp.h"
> #include "hw/core/irq.h"
> #include "hw/pci/pci.h"
> #include "hw/pci/pci_bridge.h"
> @@ -187,6 +188,8 @@ static void pci_bus_realize(BusState *qbus, Error **errp)
> bus->machine_done.notify = pcibus_machine_done;
> qemu_add_machine_init_done_notifier(&bus->machine_done);
>
> + bus->acpi_pcihp_bsel_val = UINT32_MAX;
> +
> vmstate_register_any(NULL, &vmstate_pcibus, bus);
> }
>
> @@ -302,6 +305,10 @@ static void pci_bus_class_init(ObjectClass *klass, const
> void *data)
> pbc->numa_node = pcibus_numa_node;
>
> fwgc->get_data = pci_bus_fw_cfg_gen_data;
> +
> + object_class_property_add_uint32_ptr(klass, ACPI_PCIHP_PROP_BSEL,
> + offsetof(PCIBus,
> acpi_pcihp_bsel_val),
> + OBJ_PROP_FLAG_READWRITE);
> }
>
> static const TypeInfo pci_bus_info = {
> --
> 2.43.0
>
With regards,
Daniel
--
|: https://berrange.com ~~ https://hachyderm.io/@berrange :|
|: https://libvirt.org ~~ https://entangle-photo.org :|
|: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
