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

[Igor Mammedov]

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?

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),
I'm inclined to fix it without adding compat stuff.
Michael, what do you think?

> We need to change one or the other.
> 
> From the windows device manager, I see that the other IO address range is
> 0x0620 to 0x062F which is reserved for GPE0.
> 
> .
> > We need a find out a reason why Windows doesn't like it. You might get more
> > detailed error running Windows debug build with ACPI debugger attached.  
> 
> bummer. This is beyond my expertize and I do not have a windows debug
> build.
never mind, you already found the issue.
  
> > > Hence, this
> > > change modifies the IO address range so that windows can allocate the 
> > > address
> > > range without any conflict. The new address range would start at 0x0dd4 
> > > and end
> > > at address 0x0deb.
> > >
> > > This change has been tested using a Windows Server 2019 guest VM.
> > >
> > > 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>
> > > ---
> > >  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..b68c5a2174 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 0x0dd4
maybe 0xcc0 to fit right under acpi-cpu-hotplug as it was intended?

> >
> > that's ABI change, it must be versioned
> >  
> > >
> > >  typedef struct ICH9LPCPMRegs {
> > >      /*  
> >
> >  
>