[PATCH 0/3] use full io address space on q35

[Gerd Hoffmann]

Hi,

First patch swapped for v2 of this series, using the patch from Paolo
now, which removes the pmbase dependency in the smm code altogether.

Second patch (unmodified) makes acpi pmbase a variable and figure where
to place the registers at runtime.  No functional change.

With these preparations in place the third patch (unmodified too) moves
the acpi pm registers below 0x1000 for new enougth qemu (2.0+).  The
whole 0x1000 -> 0xffff range is free then on q35.

cheers,
  Gerd

Gerd Hoffmann (2):
  acpi: remove PORT_ACPI_PM_BASE constant
  Allow using full io region on q35.

Paolo Bonzini (1):
  smm: remove code to handle ACPI disable/enable

src/fw/acpi.c       | 14 +++++++-------
 src/fw/biostables.c |  1 +
 src/fw/paravirt.c   |  5 +++++
 src/fw/paravirt.h   |  2 --
 src/fw/pciinit.c    | 32 +++++++++++++++++---------------
 src/fw/smm.c        | 29 ++---------------------------
 src/util.h          |  1 +
 7 files changed, 33 insertions(+), 51 deletions(-)

[PATCH 1/3] smm: remove code to handle ACPI disable/enable

[Gerd Hoffmann]

From: Paolo Bonzini

This is handled already in QEMU, no need to do it in SMM.

Signed-off-by: Paolo Bonzini 
Signed-off-by: Gerd Hoffmann 
---
 src/fw/smm.c | 25 -------------------------
 1 file changed, 25 deletions(-)

diff --git a/src/fw/smm.c b/src/fw/smm.c
index 1032ffb..5ae6f32 100644
--- a/src/fw/smm.c
+++ b/src/fw/smm.c
@@ -45,34 +45,9 @@ ASM32FLAT(

extern u8 smm_code_start, smm_code_end;
 ASM32FLAT(
-    /* minimal SMM code to enable or disable ACPI */
     ".global smm_code_start, smm_code_end\n"
     "  .code16gcc\n"
     "smm_code_start:\n"
-    "  movw $" __stringify(PORT_SMI_CMD) ", %dx\n"
-    "  inb %dx, %al\n"
-    "  cmpb $0xf0, %al\n"
-    "  jne 1f\n"
-
-    /* ACPI disable */
-    "  movw $" __stringify(PORT_ACPI_PM_BASE) " + 0x04, %dx\n" /* PMCNTRL */
-    "  inw %dx, %ax\n"
-    "  andw $~1, %ax\n"
-    "  outw %ax, %dx\n"
-
-    "  jmp 2f\n"
-
-    "1:\n"
-    "  cmpb $0xf1, %al\n"
-    "  jne 2f\n"
-
-    /* ACPI enable */
-    "  movw $" __stringify(PORT_ACPI_PM_BASE) " + 0x04, %dx\n" /* PMCNTRL */
-    "  inw %dx, %ax\n"
-    "  orw $1, %ax\n"
-    "  outw %ax, %dx\n"
-
-    "2:\n"
     "  rsm\n"
     "smm_code_end:\n"
     "  .code32\n"

[PATCH 2/3] acpi: remove PORT_ACPI_PM_BASE constant

[Gerd Hoffmann]

Use the new acpi_pm_base variable instead.

Signed-off-by: Gerd Hoffmann 
---
 src/fw/acpi.c       | 14 +++++++-------
 src/fw/biostables.c |  1 +
 src/fw/paravirt.h   |  2 --
 src/fw/pciinit.c    | 16 ++++++++--------
 src/fw/smm.c        |  4 ++--
 src/util.h          |  1 +
 6 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/src/fw/acpi.c b/src/fw/acpi.c
index 5ad2eec..733ca4d 100644
--- a/src/fw/acpi.c
+++ b/src/fw/acpi.c
@@ -55,9 +55,9 @@ static void piix4_fadt_setup(struct pci_device *pci, void *arg)
     fadt->smi_cmd = cpu_to_le32(PORT_SMI_CMD);
     fadt->acpi_enable = PIIX4_ACPI_ENABLE;
     fadt->acpi_disable = PIIX4_ACPI_DISABLE;
-    fadt->pm1a_evt_blk = cpu_to_le32(PORT_ACPI_PM_BASE);
-    fadt->pm1a_cnt_blk = cpu_to_le32(PORT_ACPI_PM_BASE + 0x04);
-    fadt->pm_tmr_blk = cpu_to_le32(PORT_ACPI_PM_BASE + 0x08);
+    fadt->pm1a_evt_blk = cpu_to_le32(acpi_pm_base);
+    fadt->pm1a_cnt_blk = cpu_to_le32(acpi_pm_base + 0x04);
+    fadt->pm_tmr_blk = cpu_to_le32(acpi_pm_base + 0x08);
     fadt->gpe0_blk = cpu_to_le32(PIIX4_GPE0_BLK);
     fadt->pm1_evt_len = 4;
     fadt->pm1_cnt_len = 2;
@@ -81,10 +81,10 @@ static void ich9_lpc_fadt_setup(struct pci_device *dev, void *arg)
     fadt->smi_cmd = cpu_to_le32(PORT_SMI_CMD);
     fadt->acpi_enable = ICH9_ACPI_ENABLE;
     fadt->acpi_disable = ICH9_ACPI_DISABLE;
-    fadt->pm1a_evt_blk = cpu_to_le32(PORT_ACPI_PM_BASE);
-    fadt->pm1a_cnt_blk = cpu_to_le32(PORT_ACPI_PM_BASE + 0x04);
-    fadt->pm_tmr_blk = cpu_to_le32(PORT_ACPI_PM_BASE + 0x08);
-    fadt->gpe0_blk = cpu_to_le32(PORT_ACPI_PM_BASE + ICH9_PMIO_GPE0_STS);
+    fadt->pm1a_evt_blk = cpu_to_le32(acpi_pm_base);
+    fadt->pm1a_cnt_blk = cpu_to_le32(acpi_pm_base + 0x04);
+    fadt->pm_tmr_blk = cpu_to_le32(acpi_pm_base + 0x08);
+    fadt->gpe0_blk = cpu_to_le32(acpi_pm_base + ICH9_PMIO_GPE0_STS);
     fadt->pm1_evt_len = 4;
     fadt->pm1_cnt_len = 2;
     fadt->pm_tmr_len = 4;
diff --git a/src/fw/biostables.c b/src/fw/biostables.c
index 17bee8e..50a891b 100644
--- a/src/fw/biostables.c
+++ b/src/fw/biostables.c
@@ -170,6 +170,7 @@ find_resume_vector(void)
 static struct acpi_20_generic_address acpi_reset_reg;
 static u8 acpi_reset_val;
 u32 acpi_pm1a_cnt VARFSEG;
+u16 acpi_pm_base = 0xb000;

#define acpi_ga_to_bdf(addr) pci_to_bdf(0, (addr >> 32) & 0xffff, (addr >> 16) & 0xffff)

diff --git a/src/fw/paravirt.h b/src/fw/paravirt.h
index 04fb4b9..95ffb92 100644
--- a/src/fw/paravirt.h
+++ b/src/fw/paravirt.h
@@ -29,8 +29,6 @@ static inline int runningOnKVM(void) {
 #define PORT_SMI_STATUS        0x00b3
 #define PORT_QEMU_CFG_CTL      0x0510
 #define PORT_QEMU_CFG_DATA     0x0511
-#define PORT_ACPI_PM_BASE      0xb000
-#define PORT_SMB_BASE          0xb100

void qemu_preinit(void);
 void qemu_platform_setup(void);
diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
index bbaecd6..9ffda49 100644
--- a/src/fw/pciinit.c
+++ b/src/fw/pciinit.c
@@ -185,13 +185,13 @@ static void mch_isa_bridge_setup(struct pci_device *dev, void *arg)

/* pm io base */
     pci_config_writel(bdf, ICH9_LPC_PMBASE,
-                      PORT_ACPI_PM_BASE | ICH9_LPC_PMBASE_RTE);
+                      acpi_pm_base | ICH9_LPC_PMBASE_RTE);

/* acpi enable, SCI: IRQ9 000b = irq9*/
     pci_config_writeb(bdf, ICH9_LPC_ACPI_CTRL, ICH9_LPC_ACPI_CTRL_ACPI_EN);

-    acpi_pm1a_cnt = PORT_ACPI_PM_BASE + 0x04;
-    pmtimer_setup(PORT_ACPI_PM_BASE + 0x08);
+    acpi_pm1a_cnt = acpi_pm_base + 0x04;
+    pmtimer_setup(acpi_pm_base + 0x08);
 }

static void storage_ide_setup(struct pci_device *pci, void *arg)
@@ -228,9 +228,9 @@ static void piix4_pm_config_setup(u16 bdf)
     // acpi sci is hardwired to 9
     pci_config_writeb(bdf, PCI_INTERRUPT_LINE, 9);

-    pci_config_writel(bdf, 0x40, PORT_ACPI_PM_BASE | 1);
+    pci_config_writel(bdf, 0x40, acpi_pm_base | 1);
     pci_config_writeb(bdf, 0x80, 0x01); /* enable PM io space */
-    pci_config_writel(bdf, 0x90, PORT_SMB_BASE | 1);
+    pci_config_writel(bdf, 0x90, (acpi_pm_base + 0x100) | 1);
     pci_config_writeb(bdf, 0xd2, 0x09); /* enable SMBus io space */
 }

@@ -242,8 +242,8 @@ static void piix4_pm_setup(struct pci_device *pci, void *arg)
     PiixPmBDF = pci->bdf;
     piix4_pm_config_setup(pci->bdf);

-    acpi_pm1a_cnt = PORT_ACPI_PM_BASE + 0x04;
-    pmtimer_setup(PORT_ACPI_PM_BASE + 0x08);
+    acpi_pm1a_cnt = acpi_pm_base + 0x04;
+    pmtimer_setup(acpi_pm_base + 0x08);
 }

/* ICH9 SMBUS */
@@ -253,7 +253,7 @@ static void ich9_smbus_setup(struct pci_device *dev, void *arg)
     u16 bdf = dev->bdf;
     /* map smbus into io space */
     pci_config_writel(bdf, ICH9_SMB_SMB_BASE,
-                      PORT_SMB_BASE | PCI_BASE_ADDRESS_SPACE_IO);
+                      (acpi_pm_base + 0x100) | PCI_BASE_ADDRESS_SPACE_IO);

/* enable SMBus */
     pci_config_writeb(bdf, ICH9_SMB_HOSTC, ICH9_SMB_HOSTC_HST_EN);
diff --git a/src/fw/smm.c b/src/fw/smm.c
index 5ae6f32..0f59f20 100644
--- a/src/fw/smm.c
+++ b/src/fw/smm.c
@@ -116,7 +116,7 @@ static void piix4_apmc_smm_setup(int isabdf, int i440_bdf)
 void ich9_lpc_apmc_smm_setup(int isabdf, int mch_bdf)
 {
     /* check if SMM init is already done */
-    u32 value = inl(PORT_ACPI_PM_BASE + ICH9_PMIO_SMI_EN);
+    u32 value = inl(acpi_pm_base + ICH9_PMIO_SMI_EN);
     if (value & ICH9_PMIO_SMI_EN_APMC_EN)
         return;

@@ -127,7 +127,7 @@ void ich9_lpc_apmc_smm_setup(int isabdf, int mch_bdf)

/* enable SMI generation when writing to the APMC register */
     outl(value | ICH9_PMIO_SMI_EN_APMC_EN,
-         PORT_ACPI_PM_BASE + ICH9_PMIO_SMI_EN);
+         acpi_pm_base + ICH9_PMIO_SMI_EN);

smm_relocate_and_restore();

diff --git a/src/util.h b/src/util.h
index 0cbea48..b54271b 100644
--- a/src/util.h
+++ b/src/util.h
@@ -70,6 +70,7 @@ extern struct pir_header *PirAddr;
 void copy_acpi_rsdp(void *pos);
 extern struct rsdp_descriptor *RsdpAddr;
 extern u32 acpi_pm1a_cnt;
+extern u16 acpi_pm_base;
 void *find_acpi_rsdp(void);
 u32 find_resume_vector(void);
 void acpi_reboot(void);

[PATCH 3/3] Allow using full io region on q35.

[Gerd Hoffmann]

If qemu is new enough to support acpi table loading,
then go move pmbase out of the way.  This allows to
use the whole 0x1000 -> 0xffff io address space on q35.
piix has hotplug ports in the 0xa000 -> 0xafff area,
so we can't do the same there.

Signed-off-by: Gerd Hoffmann 
---
 src/fw/paravirt.c |  5 +++++
 src/fw/pciinit.c  | 16 +++++++++-------
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
index 569fd25..db22ae8 100644
--- a/src/fw/paravirt.c
+++ b/src/fw/paravirt.c
@@ -440,4 +440,9 @@ void qemu_cfg_init(void)
     }

qemu_cfg_e820();
+
+    if (romfile_find("etc/table-loader")) {
+        acpi_pm_base = 0x0600;
+        dprintf(1, "Moving pm_base to 0x%x\n", acpi_pm_base);
+    }
 }
diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
index 9ffda49..2e6382f 100644
--- a/src/fw/pciinit.c
+++ b/src/fw/pciinit.c
@@ -36,6 +36,7 @@ u64 pcimem_start   = BUILD_PCIMEM_START;
 u64 pcimem_end     = BUILD_PCIMEM_END;
 u64 pcimem64_start = BUILD_PCIMEM64_START;
 u64 pcimem64_end   = BUILD_PCIMEM64_END;
+u64 pci_io_low_end = 0xa000;

struct pci_region_entry {
     struct pci_device *dev;
@@ -404,6 +405,12 @@ static void mch_mem_addr_setup(struct pci_device *dev, void *arg)
     pcimem_start = addr + size;

pci_slot_get_irq = mch_pci_slot_get_irq;
+
+    /* setup io address space */
+    if (acpi_pm_base < 0x1000)
+        pci_io_low_end = 0x10000;
+    else
+        pci_io_low_end = acpi_pm_base;
 }

static const struct pci_device_id pci_platform_tbl[] = {
@@ -721,16 +728,11 @@ static int pci_bios_init_root_regions_io(struct pci_bus *bus)
     if (sum < 0x4000) {
         /* traditional region is big enougth, use it */
         r_io->base = 0xc000;
-    } else if (sum < 0x9000) {
+    } else if (sum < pci_io_low_end - 0x1000) {
         /* use the larger region at 0x1000 */
         r_io->base = 0x1000;
     } else {
-        /*
-         * Not enougth io address space -> error out.
-         *
-         * TODO: on q35 we can move PORT_ACPI_PM_BASE out of
-         * the way, then use the whole 1000 -> ffff region.
-         */
+        /* not enouth io address space -> error out */
         return -1;
     }
     dprintf(1, "PCI: IO: %4llx - %4llx\n", r_io->base, r_io->base + sum - 1);

[PATCH 3/3] Allow using full io region on q35.

[Igor Mammedov]

On Tue, 20 May 2014 12:20:37 +0200
Gerd Hoffmann  wrote:
> If qemu is new enough to support acpi table loading,
> then go move pmbase out of the way.  This allows to
> use the whole 0x1000 -> 0xffff io address space on q35.
> piix has hotplug ports in the 0xa000 -> 0xafff area,
> so we can't do the same there.

Are there QEMU patches for this?
> Signed-off-by: Gerd Hoffmann
>
> src/fw/paravirt.c |  5 +++++
>  src/fw/pciinit.c  | 16 +++++++++-------
>  2 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
> index 569fd25..db22ae8 100644
> --- a/src/fw/paravirt.c
> +++ b/src/fw/paravirt.c
> @@ -440,4 +440,9 @@ void qemu_cfg_init(void)
>      }
>  
>      qemu_cfg_e820();
> if (romfile_find("etc/table-loader")) {
>    acpi_pm_base = 0x0600;
>    dprintf(1, "Moving pm_base to 0x%x\n", acpi_pm_base);
> }
>
> }
> diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
> index 9ffda49..2e6382f 100644
> --- a/src/fw/pciinit.c
> +++ b/src/fw/pciinit.c
> @@ -36,6 +36,7 @@ u64 pcimem_start   = BUILD_PCIMEM_START;
>  u64 pcimem_end     = BUILD_PCIMEM_END;
>  u64 pcimem64_start = BUILD_PCIMEM64_START;
>  u64 pcimem64_end   = BUILD_PCIMEM64_END;
> +u64 pci_io_low_end = 0xa000;
>  
>  struct pci_region_entry {
>      struct pci_device *dev;
> @@ -404,6 +405,12 @@ static void mch_mem_addr_setup(struct pci_device *dev, void *arg)
>      pcimem_start = addr + size;
>  
>      pci_slot_get_irq = mch_pci_slot_get_irq;
> /* setup io address space */
> if (acpi_pm_base < 0x1000)
>    pci_io_low_end = 0x10000;
> else
>    pci_io_low_end = acpi_pm_base;
>
> }
>  
>  static const struct pci_device_id pci_platform_tbl[] = {
> @@ -721,16 +728,11 @@ static int pci_bios_init_root_regions_io(struct pci_bus *bus)
>      if (sum < 0x4000) {
>          /* traditional region is big enougth, use it */
>          r_io->base = 0xc000;
> } else if (sum < 0x9000) {
> } else if (sum < pci_io_low_end - 0x1000) {
>     /* use the larger region at 0x1000 */
>     r_io->base = 0x1000;
> } else {
>    /*
>     * Not enougth io address space -> error out.
>     *
>     * TODO: on q35 we can move PORT_ACPI_PM_BASE out of
>     * the way, then use the whole 1000 -> ffff region.
>     */
>    /* not enouth io address space -> error out */
>    return -1;
> }
>    dprintf(1, "PCI: IO: %4llx - %4llx\n", r_io->base, r_io->base + sum - 1);

[PATCH 3/3] Allow using full io region on q35.

[Gerd Hoffmann]

On Di, 2014-05-20 at 16:42 +0200, Igor Mammedov wrote:
> On Tue, 20 May 2014 12:20:37 +0200
> Gerd Hoffmann  wrote:
> > If qemu is new enough to support acpi table loading,
> > then go move pmbase out of the way.  This allows to
> > use the whole 0x1000 -> 0xffff io address space on q35.
> > piix has hotplug ports in the 0xa000 -> 0xafff area,
> > so we can't do the same there.
>
> Are there QEMU patches for this?

No.  qemu 2.0+ (qemu acpi table generator to be exact) is required
though, to make sure the pm register addresses in the acpi tables are
correct.

cheers,
  Gerd

[Kevin O'Connor]

On Tue, May 20, 2014 at 12:20:34PM +0200, Gerd Hoffmann wrote:
> Hi,
>
> First patch swapped for v2 of this series, using the patch from Paolo
> now, which removes the pmbase dependency in the smm code altogether.
>
> Second patch (unmodified) makes acpi pmbase a variable and figure where
> to place the registers at runtime.  No functional change.
>
> With these preparations in place the third patch (unmodified too) moves
> the acpi pm registers below 0x1000 for new enougth qemu (2.0+).  The
> whole 0x1000 -> 0xffff range is free then on q35.

Looks good to me.

-Kevin