Re: [Qemu-devel] [PATCH v2 3/4] i386: generate pc guest info

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Jul 08, 2013 at 02:10:03PM -0500, Anthony Liguori wrote:
> > +    uint16_t sci_int;
> > +    uint8_t acpi_enable_cmd;
> > +    uint8_t acpi_disable_cmd;
> > +    uint32_t gpe0_blk;
> > +    uint32_t gpe0_blk_len;

...

> 
> This is all stuff that should be obtained via QOM.

Okay, so I am addressing this - of course there's more stuff that should
be moved out of this structure, but for these specific fields, I'd like
to hear whether this is going into the right direction.  Basically I
added APIs to get the needed info from the internal structures and to
find these structures using QOM.

Then added device-specific code to acpi-build.c

This is an incremental patch, of course it will be
rolled into one with the original for the final
submission.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

---

diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 4c89360..985ae40 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -219,8 +219,6 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
                           "apci-gpe0", ICH9_PMIO_GPE0_LEN);
     memory_region_add_subregion(&pm->io, ICH9_PMIO_GPE0_STS, &pm->io_gpe);
 
-    guest_info->gpe0_blk = PC_GUEST_PORT_ACPI_PM_BASE + ICH9_PMIO_GPE0_STS;
-    guest_info->gpe0_blk_len = ICH9_PMIO_GPE0_LEN;
     guest_info->fix_rtc = true;
     guest_info->platform_timer = false;
 
@@ -233,3 +231,9 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
     pm->powerdown_notifier.notify = pm_powerdown_req;
     qemu_register_powerdown_notifier(&pm->powerdown_notifier);
 }
+
+void ich9_pm_get_acpi_pm_info(ICH9LPCPMRegs *pm, AcpiPmInfo *info)
+{
+    info->gpe0_blk = PC_GUEST_PORT_ACPI_PM_BASE + ICH9_PMIO_GPE0_STS;
+    info->gpe0_blk_len = ICH9_PMIO_GPE0_LEN;
+}
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index fdbd2af..4d1c45d 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -29,6 +29,7 @@
 #include "exec/ioport.h"
 #include "hw/nvram/fw_cfg.h"
 #include "exec/address-spaces.h"
+#include "hw/acpi/piix4.h"
 
 //#define DEBUG
 
@@ -473,6 +474,30 @@ static int piix4_pm_initfn(PCIDevice *dev)
     return 0;
 }
 
+PIIX4PMState *piix4_pm_find(void)
+{
+    bool ambig;
+    Object *o = object_resolve_path_type("", "PIIX4_PM", &ambig);
+
+    if (ambig || !o) {
+        return NULL;
+    }
+    return OBJECT_CHECK(PIIX4PMState, o, "PIIX4_PM");
+}
+
+void piix4_pm_get_acpi_pm_info(PIIX4PMState *s, AcpiPmInfo *info)
+{
+        info->s3_disabled = s->disable_s3;
+        info->s4_disabled = s->disable_s4;
+        info->s4_val = s->s4_val;
+
+        info->acpi_enable_cmd = ACPI_ENABLE;
+        info->acpi_disable_cmd = ACPI_DISABLE;
+        info->gpe0_blk = GPE_BASE;
+        info->gpe0_blk_len = GPE_LEN;
+        info->sci_int = 9;
+}
+
 i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
                        qemu_irq sci_irq, qemu_irq smi_irq,
                        int kvm_enabled, FWCfgState *fw_cfg,
@@ -501,15 +526,6 @@ i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
 
     if (guest_info) {
         s->guest_info = guest_info;
-
-        guest_info->s3_disabled = s->disable_s3;
-        guest_info->s4_disabled = s->disable_s4;
-        guest_info->s4_val = s->s4_val;
-
-        guest_info->acpi_enable_cmd = ACPI_ENABLE;
-        guest_info->acpi_disable_cmd = ACPI_DISABLE;
-        guest_info->gpe0_blk = GPE_BASE;
-        guest_info->gpe0_blk_len = GPE_LEN;
         guest_info->fix_rtc = false;
         guest_info->platform_timer = true;
     }
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index a59829e..99a40f2 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -36,6 +36,24 @@
 #include "hw/i386/bios-linker-loader.h"
 #include "hw/loader.h"
 
+/* Supported chipsets: */
+#include "hw/acpi/piix4.h"
+#include "hw/i386/ich9.h"
+
+static void acpi_get_pm_info(AcpiPmInfo *info)
+{
+    PIIX4PMState *piix = piix4_pm_find();
+    ICH9LPCState *lpc = ich9_lpc_find();
+    assert(!!piix != !!lpc);
+
+    if (piix) {
+        piix4_pm_get_acpi_pm_info(piix, info);
+    }
+    if (lpc) {
+        ich9_lpc_get_acpi_pm_info(lpc, info);
+    }
+}
+
 #define ACPI_BUILD_APPNAME  "Bochs"
 #define ACPI_BUILD_APPNAME6 "BOCHS "
 #define ACPI_BUILD_APPNAME4 "BXPC"
@@ -99,18 +117,18 @@ static void fadt_setup(AcpiFadtDescriptorRev1 *fadt, PcGuestInfo *guest_info)
 {
     fadt->model = 1;
     fadt->reserved1 = 0;
-    fadt->sci_int = cpu_to_le16(guest_info->sci_int);
+    fadt->sci_int = cpu_to_le16(guest_info->pm.sci_int);
     fadt->smi_cmd = cpu_to_le32(ACPI_PORT_SMI_CMD);
-    fadt->acpi_enable = guest_info->acpi_enable_cmd;
-    fadt->acpi_disable = guest_info->acpi_disable_cmd;
+    fadt->acpi_enable = guest_info->pm.acpi_enable_cmd;
+    fadt->acpi_disable = guest_info->pm.acpi_disable_cmd;
     fadt->pm1a_evt_blk = cpu_to_le32(ACPI_PORT_PM_BASE);
     fadt->pm1a_cnt_blk = cpu_to_le32(ACPI_PORT_PM_BASE + 0x04);
     fadt->pm_tmr_blk = cpu_to_le32(ACPI_PORT_PM_BASE + 0x08);
-    fadt->gpe0_blk = cpu_to_le32(guest_info->gpe0_blk);
+    fadt->gpe0_blk = cpu_to_le32(guest_info->pm.gpe0_blk);
     fadt->pm1_evt_len = 4;
     fadt->pm1_cnt_len = 2;
     fadt->pm_tmr_len = 4;
-    fadt->gpe0_blk_len = guest_info->gpe0_blk_len;
+    fadt->gpe0_blk_len = guest_info->pm.gpe0_blk_len;
     fadt->plvl2_lat = cpu_to_le16(0xfff); /* C2 state not supported */
     fadt->plvl3_lat = cpu_to_le16(0xfff); /* C3 state not supported */
     fadt->flags = cpu_to_le32((1 << ACPI_FADT_F_WBINVD) |
@@ -339,14 +357,14 @@ build_ssdt(GArray *table_data, GArray *linker,
 
     /* Copy header and encode fwcfg values in the S3_ / S4_ / S5_ packages */
     memcpy(ssdt_ptr, ssdp_misc_aml, sizeof(ssdp_misc_aml));
-    if (guest_info->s3_disabled) {
+    if (guest_info->pm.s3_disabled) {
         ssdt_ptr[acpi_s3_name[0]] = 'X';
     }
-    if (guest_info->s4_disabled) {
+    if (guest_info->pm.s4_disabled) {
         ssdt_ptr[acpi_s4_name[0]] = 'X';
     } else {
         ssdt_ptr[acpi_s4_pkg[0] + 1] = ssdt[acpi_s4_pkg[0] + 3] =
-            guest_info->s4_val;
+            guest_info->pm.s4_val;
     }
 
     *(uint32_t*)&ssdt_ptr[acpi_pci32_start[0]] =
@@ -652,6 +670,8 @@ void acpi_setup(PcGuestInfo *guest_info)
         return;
     }
 
+    acpi_get_pm_info(&guest_info->pm);
+
     table_data = g_array_new(false, true /* clear */, 1);
     table_offsets = g_array_new(false, true /* clear */,
                                         sizeof(uint32_t));
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 58e6796..114a23b 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -244,7 +244,6 @@ static void pc_init1(MemoryRegion *system_memory,
                               gsi[9], *smi_irq,
                               kvm_enabled(), fw_cfg,
                               guest_info);
-        guest_info->sci_int = 9;
         smbus_eeprom_init(smbus, 8, NULL, 0);
     }
 
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 56b137f..6b20bda 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -312,13 +312,6 @@ PCIINTxRoute ich9_route_intx_pin_to_irq(void *opaque, int pirq_pin)
     return route;
 }
 
-void ich9_lpc_set_guest_info(PcGuestInfo *guest_info)
-{
-    guest_info->sci_int = 9;
-    guest_info->acpi_enable_cmd = ICH9_APM_ACPI_ENABLE;
-    guest_info->acpi_disable_cmd = ICH9_APM_ACPI_DISABLE;
-}
-
 static int ich9_lpc_sci_irq(ICH9LPCState *lpc)
 {
     switch (lpc->d.config[ICH9_LPC_ACPI_CTRL] &
@@ -569,6 +562,25 @@ static bool ich9_rst_cnt_needed(void *opaque)
     return (lpc->rst_cnt != 0);
 }
 
+ICH9LPCState *ich9_lpc_find(void)
+{
+    bool ambig;
+    Object *o = object_resolve_path_type("", TYPE_ICH9_LPC_DEVICE, &ambig);
+
+    if (ambig) {
+        return NULL;
+    }
+    return ICH9_LPC_DEVICE(o);
+}
+
+void ich9_pm_get_acpi_pm_info(ICH9LPCState *lpc, AcpiPmInfo *info)
+{
+    info->sci_int = 9;
+    info->acpi_enable_cmd = ICH9_APM_ACPI_ENABLE;
+    info->acpi_disable_cmd = ICH9_APM_ACPI_DISABLE;
+    ich9_pm_get_acpi_pm_info(&lpc->pm, info);
+}
+
 static const VMStateDescription vmstate_ich9_rst_cnt = {
     .name = "ICH9LPC/rst_cnt",
     .version_id = 1,
diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
index 66ab31a..1092705 100644
--- a/include/hw/acpi/ich9.h
+++ b/include/hw/acpi/ich9.h
@@ -49,4 +49,6 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
 void ich9_pm_iospace_update(ICH9LPCPMRegs *pm, uint32_t pm_io_base);
 extern const VMStateDescription vmstate_ich9_pm;
 
+void ich9_pm_get_acpi_pm_info(ICH9LPCPMRegs *, AcpiPmInfo *);
+
 #endif /* HW_ACPI_ICH9_H */
diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index 7428452..d46b17b 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -15,7 +15,6 @@
 #include "hw/acpi/ich9.h"
 #include "hw/pci/pci_bus.h"
 
-void ich9_lpc_set_guest_info(PcGuestInfo *guest_info);
 void ich9_lpc_set_irq(void *opaque, int irq_num, int level);
 int ich9_lpc_map_irq(PCIDevice *pci_dev, int intx);
 PCIINTxRoute ich9_route_intx_pin_to_irq(void *opaque, int pirq_pin);
@@ -67,6 +66,9 @@ typedef struct ICH9LPCState {
     qemu_irq *ioapic;
 } ICH9LPCState;
 
+ICH9LPCState *ich9_lpc_find(void);
+void ich9_lpc_get_acpi_pm_info(ICH9LPCState *, AcpiPmInfo *);
+
 #define Q35_MASK(bit, ms_bit, ls_bit) \
 ((uint##bit##_t)(((1ULL << ((ms_bit) + 1)) - 1) & ~((1ULL << ls_bit) - 1)))
 
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 9bd74b0..a721424 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -23,6 +23,17 @@ typedef struct PcPciInfo {
 /* Matches the value hard-coded in BIOS */
 #define PC_GUEST_PORT_ACPI_PM_BASE      0xb000
 
+typedef struct AcpiPmInfo {
+    bool s3_disabled;
+    bool s4_disabled;
+    uint8_t s4_val;
+    uint16_t sci_int;
+    uint8_t acpi_enable_cmd;
+    uint8_t acpi_disable_cmd;
+    uint32_t gpe0_blk;
+    uint32_t gpe0_blk_len;
+} AcpiPmInfo;
+
 struct PcGuestInfo {
     PcPciInfo pci_info;
     bool has_pci_info;
@@ -34,15 +45,8 @@ struct PcGuestInfo {
     uint64_t *node_mem;
     uint64_t *node_cpu;
     DECLARE_BITMAP(found_cpus, MAX_CPUMASK_BITS + 1);
-    bool s3_disabled;
-    bool s4_disabled;
-    uint8_t s4_val;
     DECLARE_BITMAP(slot_hotplug_enable, PCI_SLOT_MAX);
-    uint16_t sci_int;
-    uint8_t acpi_enable_cmd;
-    uint8_t acpi_disable_cmd;
-    uint32_t gpe0_blk;
-    uint32_t gpe0_blk_len;
+    AcpiPmInfo pm;
     bool fix_rtc;
     bool platform_timer;
     uint64_t mcfg_base;
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index ac9f8d4..cb66e19 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -65,5 +65,6 @@ typedef struct QEMUSGList QEMUSGList;
 typedef struct SHPCDevice SHPCDevice;
 typedef struct FWCfgState FWCfgState;
 typedef struct PcGuestInfo PcGuestInfo;
+typedef struct AcpiPmInfo AcpiPmInfo;
 
 #endif /* QEMU_TYPEDEFS_H */


-- 
MST