Hi Eric,
On 5/26/25 07:21, Eric Auger wrote:
> Hi Gustavo,
>
> On 5/26/25 7:55 AM, Gustavo Romero wrote:
>> Hi Eric,
>>
>> On 5/14/25 14:00, Eric Auger wrote:
>>> This series enables ACPI PCI hotplug/hotunplug on ARM
>>> and makes it default for 10.1 machine type. This aligns with
>>> x86 q35 machine. Expected benefits should be similar to
>>> those listed in [1].
>>>
>>> It is still possible to disable it using a virt machine
>>> option: virt,acpi-pcihp=off and restore the legacy behavior.
>>>
>>> The infrastructure used in x86 is heavily reused and a
>>> huge part of the series consists in moving code from
>>> hw/i386/acpi-build.c to a generic place and slightly
>>> adapting it to make it usable on ARM. The DSDT table is
>>> augmented to support ACPI PCI hotplug elements.
>>>
>>> On ARM we use use a GED event to notify the OS about
>>> hotplug events.
>>>
>>> I have not noticed any tests/qtest/bios-tables-test failures
>>> neither on x86 nor ARM. x86 DSDT table has not changed.
>>> ARM DSDT table definitively has but there are no tests
>>> impacted. ARM misses hotplug tests that do exist on x86. This
>>> most probably should be considered in the future.
>>
>> I've just posted a v1 for a acpi-pcihp=on test:
>>
>> https://mail.gnu.org/archive/html/qemu-devel/2025-05/msg05824.html
>>
>> The odd thing is that I noticed failures in the existing aarch64
>> acpi tests in tests/qtest/bios-tables-test. At least the rename
>> of the variable 'CTRL' to 'Local0' in the _OSC method should cause
>> a mismatch against the current base DSDT? So I had to handle it
>> via patches 1/5, 2/5, and 3/5. The diff is in the 3/5 commit body.
>
> Thank you for contributing those tests.
>
> Hum I think my mistake was to have executed the existing tests in KVM
> acceleration mode only using
> QTEST_QEMU_BINARY=qemu-system-aarch64 tests/qtest/bios-tables-test
> direct execution.
>
> The failing tests belong to the TCG category I think.
>
> I will review your tests and if you allow me I may need to include
> patches 1 and 3 in my series to prevent existing tests from failing, of
> course with all your credits.
Yeah, absolutely!
I left some comments on you comments in 3/5 in [0] about the organization
and sequence of the patches, but I think we're in sync :)
[0] https://mail.gnu.org/archive/html/qemu-devel/2025-05/msg05824.html
> Thank you again for reporting the issue!
Np! Glad to help :)
Cheers,
Gustavo
> Eric
>
>
>
>
>>
>> Cheers,
>> Gustavo
>>
>>> Best Regards
>>>
>>> Eric
>>>
>>> This series can be found at:
>>> https://github.com/eauger/qemu/tree/arm-acpi-pcihp-v1
>>>
>>> previous RFC:
>>> https://github.com/eauger/qemu/tree/arm-acpi-pcihp-rfc
>>>
>>> History:
>>> RFC -> v1:
>>> - First 3 trivial patches were pulled separately
>>> - Fix of the register region size (0x18), ie. ACPI_PCIHP_SIZE
>>> - addition of aml_pci_edsm which was not called in RFC
>>> - acpi-index feature is now fixed. vms->bus was not set on
>>> acpi_pcihp_init. The init sequence is still hacky though. Suggestions
>>> appreciated.
>>>
>>> [1] [PATCH v6 0/6] Use ACPI PCI hot-plug for Q35
>>> https://lore.kernel.org/all/20210713004205.775386-1-jusual@redhat.com/
>>>
>>>
>>> Eric Auger (22):
>>> hw/i386/acpi-build: Make aml_pci_device_dsm() static
>>> hw/arm/virt: Introduce machine state acpi pcihp flags and props
>>> hw/acpi: Rename and move build_x86_acpi_pci_hotplug to pcihp
>>> hw/pci-host/gpex-acpi: Add native_pci_hotplug arg to
>>> acpi_dsdt_add_pci_osc
>>> hw/pci-host/gpex-acpi: Split host bridge OSC and DSM generation
>>> hw/pci-host/gpex-acpi: Propagate hotplug type info from virt machine
>>> downto gpex
>>> hw/i386/acpi-build: Turn build_q35_osc_method into a generic method
>>> hw/pci-host/gpex-acpi: Use build_pci_host_bridge_osc_method
>>> hw/i386/acpi-build: Introduce build_append_pcihp_resources() helper
>>> hw/acpi/pcihp: Add an AmlRegionSpace arg to build_acpi_pci_hotplug
>>> hw/i386/acpi-build: Move build_append_notification_callback to pcihp
>>> hw/i386/acpi-build: Move build_append_pci_bus_devices/pcihp_slots to
>>> pcihp
>>> hw/i386/acpi-build: Introduce and use acpi_get_pci_host
>>> hw/i386/acpi-build: Move aml_pci_edsm to a generic place
>>> hw/arm/virt-acpi-build: Modify the DSDT ACPI table to enable ACPI PCI
>>> hotplug
>>> hw/acpi/ged: Prepare the device to react to PCI hotplug events
>>> hw/acpi/ged: Call pcihp plug callbacks in hotplug handler
>>> implementation
>>> hw/acpi/ged: Support migration of AcpiPciHpState
>>> hw/core/sysbus: Introduce sysbus_mmio_map_name() helper
>>> hw/arm/virt: Let virt support pci hotplug/unplug GED event
>>> hw/arm/virt: Plug pcihp hotplug/hotunplug callbacks
>>> hw/arm/virt: Use ACPI PCI hotplug by default
>>>
>>> hw/i386/acpi-build.h | 4 -
>>> include/hw/acpi/aml-build.h | 2 +
>>> include/hw/acpi/generic_event_device.h | 5 +
>>> include/hw/acpi/pci.h | 4 +-
>>> include/hw/acpi/pcihp.h | 19 +-
>>> include/hw/arm/virt.h | 4 +
>>> include/hw/pci-host/gpex.h | 1 +
>>> include/hw/sysbus.h | 1 +
>>> hw/acpi/aml-build.c | 50 +++
>>> hw/acpi/generic_event_device.c | 55 +++
>>> hw/acpi/pci.c | 20 +
>>> hw/acpi/pcihp.c | 482 ++++++++++++++++++++-
>>> hw/arm/virt-acpi-build.c | 27 ++
>>> hw/arm/virt.c | 107 ++++-
>>> hw/core/sysbus.c | 11 +
>>> hw/i386/acpi-build.c | 552 +------------------------
>>> hw/pci-host/gpex-acpi.c | 75 +---
>>> hw/arm/Kconfig | 2 +
>>> 18 files changed, 806 insertions(+), 615 deletions(-)
>>>
>>
>