Re: [PATCH] hw/i386/acpi-build: adjust q35 IO addr range for acpi pci hotplug

[Ani Sinha]

On Wed, 8 Sep 2021, Philippe Mathieu-Daudé wrote:

> On 9/8/21 10:43 AM, Igor Mammedov wrote:
> > On Wed, 8 Sep 2021 12:51:04 +0530 (IST)
> > Ani Sinha <ani@anisinha.ca> wrote:
> >
> >> On Wed, 8 Sep 2021, Igor Mammedov wrote:
> >>
> >>> On Wed,  8 Sep 2021 09:41:39 +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. It was assumed that this 
> >>>> address
> >>>> range was free and available. However, upon more testing, it seems this 
> >>>> address
> >>>> range to be not available for some latest versions of windows.
> >>>
> >>> The range is something assigned by QEMU, and guest has no say where it 
> >>> should be.
> >>> but perhaps we failed to describe it properly or something similar, so 
> >>> one gets
> >>> 'no resource' error.
> >>
> >> OK dug deeper. The existing range of IO address conflicts with the CPU
> >> hotplug range.
> >>
> >> CPU hotplug range (ICH9_CPU_HOTPLUG_IO_BASE) is 0x0cd8 to 0x0ce3
> >>
> >> This intersects with range 0x0cc4 to 0x0cdb for ACPI_PCIHP_ADDR_ICH9 .
> >
> > Looking at 'info mtree' it's indeed wrong:
> >
> >     0000000000000cc4-0000000000000cdb (prio 0, i/o): acpi-pci-hotplug
> >     0000000000000cd8-0000000000000cf7 (prio 0, i/o): acpi-cpu-hotplug
> >
> > which of them eventually handles IO request in intersection range?
>
> (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
>
> >
> > Please, add to commit message your findings, so it would point out
> > where problem comes from and what it breaks(doesn't work as expect).
> >
> > Given it's broken to begin with (and possibly regression if it broke cpu 
> > hotplug),

right. I did some foresic analysis on this. So the value 0x0cc4 comes from
the original RFC patch unchanged that Julia posted:

https://patchew.org/QEMU/20200924070013.165026-1-jusual@redhat.com/20200924070013.165026-3-jusual@redhat.com/

Meanwhile, between the time that RFC patch was posted and when it was
actually pushed, you made the following change:
b32bd763a1ca9296 ("pci: introduce acpi-index property for PCI device")

This change did this:

-#define ACPI_PCIHP_SIZE 0x0014
+#define ACPI_PCIHP_SIZE 0x0018

So now the IO address ranges are no longer mutually exclusive :-)