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

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

[Ani Sinha]

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. 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
 
 typedef struct ICH9LPCPMRegs {
     /*
-- 
2.25.1

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

[Ani Sinha]

On Wed, 8 Sep 2021, Ani Sinha 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. 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.
>

I realize that this breaks bios-tables-test.c which I will correct if we
are ok with this fix.

--- /tmp/asl-0FVI90.dsl	2021-09-08 10:05:59.260579343 +0530
+++ /tmp/asl-7TYI90.dsl	2021-09-08 10:05:59.252579221 +0530
@@ -1,30 +1,30 @@
 /*
  * Intel ACPI Component Architecture
  * AML/ASL+ Disassembler version 20190509 (64-bit version)
  * Copyright (c) 2000 - 2019 Intel Corporation
  *
  * Disassembling to symbolic ASL+ operators
  *
- * Disassembly of tests/data/acpi/q35/DSDT, Wed Sep  8 10:05:59 2021
+ * Disassembly of /tmp/aml-QVYI90, Wed Sep  8 10:05:59 2021
  *
  * Original Table Header:
  *     Signature        "DSDT"
  *     Length           0x00002061 (8289)
  *     Revision         0x01 **** 32-bit table (V1), no 64-bit math
support
- *     Checksum         0xE5
+ *     Checksum         0x90
  *     OEM ID           "BOCHS "
  *     OEM Table ID     "BXPC    "
  *     OEM Revision     0x00000001 (1)
  *     Compiler ID      "BXPC"
  *     Compiler Version 0x00000001 (1)
  */
 DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPC    ", 0x00000001)
 {
     Scope (\)
     {
         OperationRegion (DBG, SystemIO, 0x0402, One)
         Field (DBG, ByteAcc, NoLock, Preserve)
         {
             DBGB,   8
         }

@@ -226,46 +226,46 @@
             Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource
Settings
             {
                 IO (Decode16,
                     0x0070,             // Range Minimum
                     0x0070,             // Range Maximum
                     0x01,               // Alignment
                     0x08,               // Length
                     )
                 IRQNoFlags ()
                     {8}
             })
         }
     }

     Scope (_SB.PCI0)
     {
-        OperationRegion (PCST, SystemIO, 0x0CC4, 0x08)
+        OperationRegion (PCST, SystemIO, 0x0DD4, 0x08)
         Field (PCST, DWordAcc, NoLock, WriteAsZeros)
         {
             PCIU,   32,
             PCID,   32
         }

-        OperationRegion (SEJ, SystemIO, 0x0CCC, 0x04)
+        OperationRegion (SEJ, SystemIO, 0x0DDC, 0x04)
         Field (SEJ, DWordAcc, NoLock, WriteAsZeros)
         {
             B0EJ,   32
         }

-        OperationRegion (BNMR, SystemIO, 0x0CD4, 0x08)
+        OperationRegion (BNMR, SystemIO, 0x0DE4, 0x08)
         Field (BNMR, DWordAcc, NoLock, WriteAsZeros)
         {
             BNUM,   32,
             PIDX,   32
         }

         Mutex (BLCK, 0x00)
         Method (PCEJ, 2, NotSerialized)
         {
             Acquire (BLCK, 0xFFFF)
             BNUM = Arg0
             B0EJ = (One << Arg1)
             Release (BLCK)
             Return (Zero)
         }

@@ -3185,34 +3185,34 @@
                     0x0620,             // Range Minimum
                     0x0620,             // Range Maximum
                     0x01,               // Alignment
                     0x10,               // Length
                     )
             })
         }

         Device (PHPR)
         {
             Name (_HID, "PNP0A06" /* Generic Container Device */)  //
_HID: Hardware ID
             Name (_UID, "PCI Hotplug resources")  // _UID: Unique ID
             Name (_STA, 0x0B)  // _STA: Status
             Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource
Settings
             {
                 IO (Decode16,
-                    0x0CC4,             // Range Minimum
-                    0x0CC4,             // Range Maximum
+                    0x0DD4,             // Range Minimum
+                    0x0DD4,             // Range Maximum
                     0x01,               // Alignment
                     0x18,               // Length
                     )
             })
         }
     }

     Scope (\)
     {
         Name (_S3, Package (0x04)  // _S3_: S3 System State
         {
             One,
             One,
             Zero,
             Zero
         })


> 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
>
>  typedef struct ICH9LPCPMRegs {
>      /*
> --
> 2.25.1
>
>

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

[Igor Mammedov]

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

> 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

that's ABI change, it must be versioned 

>  
>  typedef struct ICH9LPCPMRegs {
>      /*

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

[Ani Sinha]

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 .

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.

>
> > 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
>
> that's ABI change, it must be versioned
>
> >
> >  typedef struct ICH9LPCPMRegs {
> >      /*
>
>

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

[Ani Sinha]

On Wed, 8 Sep 2021, Ani Sinha 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 .
>

Also verified that setting ACPI_PCIHP_ADDR_ICH9 to 0x0cc0 also works and
resolves the conflict.

> 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.
>
> >
> > > 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
> >
> > that's ABI change, it must be versioned

Any pointers how to do this? I see some property values for attributes
being set in hw/i386/pc.c for older models but this one is not exactly a
property as it is currently written.


> >
> > >
> > >  typedef struct ICH9LPCPMRegs {
> > >      /*
> >
> >
>

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

[Igor Mammedov]

On Wed, 8 Sep 2021 14:13:20 +0530 (IST)
Ani Sinha <ani@anisinha.ca> wrote:

> On Wed, 8 Sep 2021, Ani Sinha 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 .
> >  
> 
> Also verified that setting ACPI_PCIHP_ADDR_ICH9 to 0x0cc0 also works and
> resolves the conflict.
> 
> > 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.
> >  
> > >  
> > > > 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  
> > >
> > > that's ABI change, it must be versioned  
> 
> Any pointers how to do this? I see some property values for attributes
> being set in hw/i386/pc.c for older models but this one is not exactly a
> property as it is currently written.
in case we would need it, it should be converted to set-able property.
But we might not need it after all, lets see what Michael thinks about it. 

> 
> 
> > >  
> > > >
> > > >  typedef struct ICH9LPCPMRegs {
> > > >      /*  
> > >
> > >  
> >  
> 

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 {
> > >      /*  
> >
> >  
> 

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

[Ani Sinha]

On Wed, 8 Sep 2021, 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?
>
> 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).
>

Will do.

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

After fixing, this is what it looks like:

$ virsh qemu-monitor-command --hmp win2k19 'info mtree' | grep acpi-
      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

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

[Philippe Mathieu-Daudé]

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

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

[Igor Mammedov]

On Wed, 8 Sep 2021 12:21:26 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> 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

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

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

[Ani Sinha]

[-- Attachment #1: Type: text/plain, Size: 2534 bytes --]



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

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

[Ani Sinha]

On Wed, 8 Sep 2021, 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?
>
> 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?
>

Michael, we would need your inputs on this one.

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

[Michael S. Tsirkin]

On Wed, Sep 08, 2021 at 10:43:51AM +0200, 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?
> 
> 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?

Agreed.

> > 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 {
> > > >      /*  
> > >
> > >  
> >