Re: [PATCH 2/3] hw/i386/acpi: fix conflicting IO address range for acpi pci hotplug in q35

[Igor Mammedov]

On Tue, 14 Sep 2021 10:24:09 +0530
Ani Sinha <ani@anisinha.ca> wrote:

> Change caf108bc58790 ("hw/i386/acpi-build: Add ACPI PCI hot-plug methods to 
> Q35")
> selects an IO address range for acpi based PCI hotplug for q35 arbitrarily. It
> starts at address 0x0cc4 and ends at 0x0cdb. At the time when the patch was
> written but the final version of the patch was not yet pushed upstream, this
> address range was free and did not conflict with any other IO address ranges.
> However, with the following change, this address range was no
> longer conflict free as in this change, the IO address range
> (value of ACPI_PCIHP_SIZE) was incremented by four bytes:
> 
> b32bd763a1ca92 ("pci: introduce acpi-index property for PCI device")
> 
> This can be seen from the output of QMP command 'info mtree' :
> 
> 0000000000000600-0000000000000603 (prio 0, i/o): acpi-evt
> 0000000000000604-0000000000000605 (prio 0, i/o): acpi-cnt
> 0000000000000608-000000000000060b (prio 0, i/o): acpi-tmr
> 0000000000000620-000000000000062f (prio 0, i/o): acpi-gpe0
> 0000000000000630-0000000000000637 (prio 0, i/o): acpi-smi
> 0000000000000cc4-0000000000000cdb (prio 0, i/o): acpi-pci-hotplug
> 0000000000000cd8-0000000000000ce3 (prio 0, i/o): acpi-cpu-hotplug
> 
> It shows that there is a region of conflict between IO regions of acpi
> pci hotplug and acpi cpu hotplug.
> 
> Unfortunately, the change caf108bc58790 did not update the IO address range
> appropriately before it was pushed upstream to accomodate the increased
> length of the IO address space introduced in change b32bd763a1ca92.
> 
> Due to this bug, windows guests complain 'This device cannot find
> enough free resources it can use' in the device manager panel for extended
> IO buses. This issue also breaks the correct functioning of pci hotplug as the
> following shows that the IO space for pci hotplug has been truncated:
> 
> (qemu) info mtree -f
> FlatView #0
>  AS "I/O", root: io
>  Root memory region: io
>   0000000000000cc4-0000000000000cd7 (prio 0, i/o): acpi-pci-hotplug
>   0000000000000cd8-0000000000000cf7 (prio 0, i/o): acpi-cpu-hotplug
> 
> Therefore, in this fix, we adjust the IO address range for the acpi pci
> hotplug so that it does not conflict with cpu hotplug and there is no
> truncation of IO spaces. The starting IO address of PCI hotplug region
> has been decremented by four bytes in order to accomodate four byte
> increment in the IO address space introduced by change
> b32bd763a1ca92 ("pci: introduce acpi-index property for PCI device")
> 
> After fixing, the following are the corrected IO ranges:
> 
> 0000000000000600-0000000000000603 (prio 0, i/o): acpi-evt
> 0000000000000604-0000000000000605 (prio 0, i/o): acpi-cnt
> 0000000000000608-000000000000060b (prio 0, i/o): acpi-tmr
> 0000000000000620-000000000000062f (prio 0, i/o): acpi-gpe0
> 0000000000000630-0000000000000637 (prio 0, i/o): acpi-smi
> 0000000000000cc0-0000000000000cd7 (prio 0, i/o): acpi-pci-hotplug
> 0000000000000cd8-0000000000000ce3 (prio 0, i/o): acpi-cpu-hotplug
> 
> This change has been tested using a Windows Server 2019 guest VM. Windows
> no longer complains after this change.
> 
> Fixes: caf108bc58790 ("hw/i386/acpi-build: Add ACPI PCI hot-plug methods to 
> Q35")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/561
> 
> Signed-off-by: Ani Sinha <ani@anisinha.ca>

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

> ---
>  include/hw/acpi/ich9.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
> index a329ce43ab..f04f1791bd 100644
> --- a/include/hw/acpi/ich9.h
> +++ b/include/hw/acpi/ich9.h
> @@ -29,7 +29,7 @@
>  #include "hw/acpi/acpi_dev_interface.h"
>  #include "hw/acpi/tco.h"
>  
> -#define ACPI_PCIHP_ADDR_ICH9 0x0cc4
> +#define ACPI_PCIHP_ADDR_ICH9 0x0cc0
>  
>  typedef struct ICH9LPCPMRegs {
>      /*