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

[Julia Suvorova]

On Tue, Sep 14, 2021 at 6:54 AM 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

s/accomodate/accommodate

> 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

same

> 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>

On the code itself:
Reviewed-by: Julia Suvorova <jusual@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 {
>      /*
> --
> 2.25.1
>