Hi Jonathan
On 6/30/25 2:53 PM, Jonathan Cameron wrote:
> On Fri, 27 Jun 2025 11:55:18 +0200
> Eric Auger <eric.auger@redhat.com> wrote:
>
>> Set up the IO registers used to communicate between QEMU
>> and ACPI.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Follow on comment inline. Otherwise LGTM
>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
thanks!
>
>> ---
>> v2 -> v3:
>> - remove acpi_ged_state->pcihp_state.use_acpi_hotplug_bridge = true;
>> - use sysbus_mmio_map_name for all regs (Igor)
>> - create_pcie left at its original place
>>
>> v1 -> v2:
>> - use ACPI_PCIHP_REGION_NAME
>> ---
>> hw/arm/virt.c | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 878c567354..d8706ef9c8 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -686,6 +686,7 @@ static inline DeviceState *create_acpi_ged(VirtMachineState *vms)
>> SysBusDevice *sbdev;
>> int irq = vms->irqmap[VIRT_ACPI_GED];
>> uint32_t event = ACPI_GED_PWR_DOWN_EVT;
>> + bool acpi_pcihp;
>>
>> if (ms->ram_slots) {
>> event |= ACPI_GED_MEM_HOTPLUG_EVT;
>> @@ -704,6 +705,18 @@ static inline DeviceState *create_acpi_ged(VirtMachineState *vms)
>> sysbus_mmio_map_name(sbdev, TYPE_ACPI_GED, vms->memmap[VIRT_ACPI_GED].base);
>> sysbus_mmio_map_name(sbdev, ACPI_MEMHP_REGION_NAME,
>> vms->memmap[VIRT_PCDIMM_ACPI].base);
>> +
>> + acpi_pcihp = object_property_get_bool(OBJECT(dev),
>> + ACPI_PM_PROP_ACPI_PCIHP_BRIDGE, NULL);
>> +
>> + if (acpi_pcihp) {
> As mentioned in earlier patch review, with this code you now have means to set the event
> bitmap as done in other cases. Maybe just do that here as well?
see my previous reply
Hope this clarifies
Eric
>
>> + int pcihp_region_index;
>> +
>> + pcihp_region_index = sysbus_mmio_map_name(sbdev, ACPI_PCIHP_REGION_NAME,
>> + vms->memmap[VIRT_ACPI_PCIHP].base);
>> + assert(pcihp_region_index >= 0);
>> + }
>> +
>> sysbus_connect_irq(sbdev, 0, qdev_get_gpio_in(vms->gic, irq));
>>
>> return dev;