[PATCH 00/12] ICH9 cleanup

[PATCH 00/12] ICH9 cleanup

[Bernhard Beschow]

This series includes ICH9 cleanups such as further QOM'ification, making it more
self-contained, and reducing its x86/pc dependencies. While reducing x86
dependencies, the IOAPIC sources are moved from hw/i386 to hw/intc which is
consistent with the header files.

The series was originally part of
https://lore.kernel.org/qemu-devel/20230131115326.12454-1-shentey@gmail.com/ ,
a series which has been split into two (see Based-on tag below). It is a respin
with Reviewd-By tags picked up.

Testing done:
* `make check`
* `make check-avocado`
* `qemu-system-x86_64 -M q35 -m 2G -cdrom \
   manjaro-kde-21.3.2-220704-linux515.iso`

Based-on: 20230213162004.2797-1-shentey@gmail.com
          "[PATCH v4 0/9] PC cleanups"

Bernhard Beschow (12):
  hw/i386/ich9: Rename Q35_MASK to ICH9_MASK
  hw/isa/lpc_ich9: Unexport PIRQ functions
  hw/isa/lpc_ich9: Eliminate ICH9LPCState::isa_bus
  hw/i2c/smbus_ich9: Move ich9_smb_set_irq() in front of
    ich9_smbus_realize()
  hw/i2c/smbus_ich9: Inline ich9_smb_init() and remove it
  hw/i386/pc_q35: Allow for setting properties before realizing
    TYPE_ICH9_LPC_DEVICE
  hw/isa/lpc_ich9: Connect pm stuff to lpc internally
  hw/isa/lpc_ich9: Remove redundant ich9_lpc_reset() invocation
  hw/i386/ich9: Remove redundant GSI_NUM_PINS
  hw: Move ioapic*.h to intc/
  hw/i386/ich9: Clean up includes
  hw: Move ich9.h to southbridge/

 MAINTAINERS                                 |  2 ++
 include/hw/acpi/ich9.h                      |  6 ++--
 include/hw/i386/x86.h                       |  3 +-
 include/hw/{i386 => intc}/ioapic.h          |  6 ++--
 include/hw/{i386 => intc}/ioapic_internal.h |  8 ++---
 include/hw/{i386 => southbridge}/ich9.h     | 35 ++++++++----------
 hw/acpi/ich9.c                              | 10 ++----
 hw/acpi/ich9_tco.c                          |  2 +-
 hw/i2c/smbus_ich9.c                         | 39 +++++++++------------
 hw/i386/acpi-build.c                        |  3 +-
 hw/i386/kvm/ioapic.c                        |  3 +-
 hw/i386/pc.c                                |  6 ++--
 hw/i386/pc_q35.c                            | 34 +++++++++---------
 hw/intc/apic.c                              |  2 +-
 hw/intc/ioapic.c                            |  4 +--
 hw/intc/ioapic_common.c                     |  4 +--
 hw/isa/lpc_ich9.c                           | 34 +++++++++---------
 hw/pci-bridge/i82801b11.c                   |  2 +-
 target/i386/whpx/whpx-all.c                 |  2 +-
 tests/qtest/tco-test.c                      |  2 +-
 20 files changed, 94 insertions(+), 113 deletions(-)
 rename include/hw/{i386 => intc}/ioapic.h (93%)
 rename include/hw/{i386 => intc}/ioapic_internal.h (96%)
 rename include/hw/{i386 => southbridge}/ich9.h (91%)

-- 
2.39.1

[PATCH 01/12] hw/i386/ich9: Rename Q35_MASK to ICH9_MASK

[Bernhard Beschow]

The Q35_MASK macro is already defined by TYPE_Q35_HOST_DEVICE, so let
TYPE_ICH9_LPC_DEVICE have its own one to prevent potential name clash.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 include/hw/i386/ich9.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index 222781e8b9..36e0ccd16a 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -75,7 +75,7 @@ struct ICH9LPCState {
     qemu_irq gsi[GSI_NUM_PINS];
 };
 
-#define Q35_MASK(bit, ms_bit, ls_bit) \
+#define ICH9_MASK(bit, ms_bit, ls_bit) \
 ((uint##bit##_t)(((1ULL << ((ms_bit) + 1)) - 1) & ~((1ULL << ls_bit) - 1)))
 
 /* ICH9: Chipset Configuration Registers */
@@ -137,13 +137,13 @@ struct ICH9LPCState {
 #define ICH9_LPC_NB_PIRQS                       8       /* PCI A-H */
 
 #define ICH9_LPC_PMBASE                         0x40
-#define ICH9_LPC_PMBASE_BASE_ADDRESS_MASK       Q35_MASK(32, 15, 7)
+#define ICH9_LPC_PMBASE_BASE_ADDRESS_MASK       ICH9_MASK(32, 15, 7)
 #define ICH9_LPC_PMBASE_RTE                     0x1
 #define ICH9_LPC_PMBASE_DEFAULT                 0x1
 
 #define ICH9_LPC_ACPI_CTRL                      0x44
 #define ICH9_LPC_ACPI_CTRL_ACPI_EN              0x80
-#define ICH9_LPC_ACPI_CTRL_SCI_IRQ_SEL_MASK     Q35_MASK(8, 2, 0)
+#define ICH9_LPC_ACPI_CTRL_SCI_IRQ_SEL_MASK     ICH9_MASK(8, 2, 0)
 #define ICH9_LPC_ACPI_CTRL_9                    0x0
 #define ICH9_LPC_ACPI_CTRL_10                   0x1
 #define ICH9_LPC_ACPI_CTRL_11                   0x2
@@ -162,7 +162,7 @@ struct ICH9LPCState {
 #define ICH9_LPC_PIRQH_ROUT                     0x6b
 
 #define ICH9_LPC_PIRQ_ROUT_IRQEN                0x80
-#define ICH9_LPC_PIRQ_ROUT_MASK                 Q35_MASK(8, 3, 0)
+#define ICH9_LPC_PIRQ_ROUT_MASK                 ICH9_MASK(8, 3, 0)
 #define ICH9_LPC_PIRQ_ROUT_DEFAULT              0x80
 
 #define ICH9_LPC_GEN_PMCON_1                    0xa0
@@ -172,7 +172,7 @@ struct ICH9LPCState {
 #define ICH9_LPC_GEN_PMCON_LOCK                 0xa6
 
 #define ICH9_LPC_RCBA                           0xf0
-#define ICH9_LPC_RCBA_BA_MASK                   Q35_MASK(32, 31, 14)
+#define ICH9_LPC_RCBA_BA_MASK                   ICH9_MASK(32, 31, 14)
 #define ICH9_LPC_RCBA_EN                        0x1
 #define ICH9_LPC_RCBA_DEFAULT                   0x0
 
-- 
2.39.1

[PATCH 02/12] hw/isa/lpc_ich9: Unexport PIRQ functions

[Bernhard Beschow]

No need to rely on the board to wire up the ICH9 PCI IRQs. All functions
access private state of the LPC device which suggests that it should
wire up the IRQs.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 include/hw/i386/ich9.h |  3 ---
 hw/i386/pc_q35.c       |  3 ---
 hw/isa/lpc_ich9.c      | 11 ++++++++---
 3 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index 36e0ccd16a..921e4c7ef6 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -9,9 +9,6 @@
 #include "hw/acpi/ich9.h"
 #include "qom/object.h"
 
-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);
 void ich9_lpc_pm_init(PCIDevice *pci_lpc, bool smm_enabled);
 I2CBus *ich9_smb_init(PCIBus *bus, int devfn, uint32_t smb_io_base);
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index a5ffb77ed8..8fac0e57bc 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -271,9 +271,6 @@ static void pc_q35_init(MachineState *machine)
     for (i = 0; i < GSI_NUM_PINS; i++) {
         qdev_connect_gpio_out_named(lpc_dev, ICH9_GPIO_GSI, i, x86ms->gsi[i]);
     }
-    pci_bus_irqs(host_bus, ich9_lpc_set_irq, ich9_lpc, ICH9_LPC_NB_PIRQS);
-    pci_bus_map_irqs(host_bus, ich9_lpc_map_irq);
-    pci_bus_set_route_irq_fn(host_bus, ich9_route_intx_pin_to_irq);
     isa_bus = ich9_lpc->isa_bus;
 
     if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 1fba3c210c..54a8839cd2 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -259,7 +259,7 @@ static void ich9_lpc_update_apic(ICH9LPCState *lpc, int gsi)
     qemu_set_irq(lpc->gsi[gsi], level);
 }
 
-void ich9_lpc_set_irq(void *opaque, int pirq, int level)
+static void ich9_lpc_set_irq(void *opaque, int pirq, int level)
 {
     ICH9LPCState *lpc = opaque;
     int pic_irq, pic_dis;
@@ -275,7 +275,7 @@ void ich9_lpc_set_irq(void *opaque, int pirq, int level)
 /* return the pirq number (PIRQ[A-H]:0-7) corresponding to
  * a given device irq pin.
  */
-int ich9_lpc_map_irq(PCIDevice *pci_dev, int intx)
+static int ich9_lpc_map_irq(PCIDevice *pci_dev, int intx)
 {
     BusState *bus = qdev_get_parent_bus(&pci_dev->qdev);
     PCIBus *pci_bus = PCI_BUS(bus);
@@ -286,7 +286,7 @@ int ich9_lpc_map_irq(PCIDevice *pci_dev, int intx)
     return lpc->irr[PCI_SLOT(pci_dev->devfn)][intx];
 }
 
-PCIINTxRoute ich9_route_intx_pin_to_irq(void *opaque, int pirq_pin)
+static PCIINTxRoute ich9_route_intx_pin_to_irq(void *opaque, int pirq_pin)
 {
     ICH9LPCState *lpc = opaque;
     PCIINTxRoute route;
@@ -680,6 +680,7 @@ static void ich9_lpc_realize(PCIDevice *d, Error **errp)
 {
     ICH9LPCState *lpc = ICH9_LPC_DEVICE(d);
     DeviceState *dev = DEVICE(d);
+    PCIBus *pci_bus = pci_get_bus(d);
     ISABus *isa_bus;
 
     if ((lpc->smi_host_features & BIT_ULL(ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT)) &&
@@ -728,6 +729,10 @@ static void ich9_lpc_realize(PCIDevice *d, Error **errp)
     isa_bus_irqs(isa_bus, lpc->gsi);
 
     i8257_dma_init(isa_bus, 0);
+
+    pci_bus_irqs(pci_bus, ich9_lpc_set_irq, d, ICH9_LPC_NB_PIRQS);
+    pci_bus_map_irqs(pci_bus, ich9_lpc_map_irq);
+    pci_bus_set_route_irq_fn(pci_bus, ich9_route_intx_pin_to_irq);
 }
 
 static bool ich9_rst_cnt_needed(void *opaque)
-- 
2.39.1

[PATCH 03/12] hw/isa/lpc_ich9: Eliminate ICH9LPCState::isa_bus

[Bernhard Beschow]

By using qdev_get_child_bus() we can eliminate ICH9LPCState::isa_bus and
spare the ich9_lpc variable in pc_q35, too.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 include/hw/i386/ich9.h | 3 ---
 hw/i386/pc_q35.c       | 4 +---
 hw/isa/lpc_ich9.c      | 5 +----
 3 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index 921e4c7ef6..05464f6965 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -1,7 +1,6 @@
 #ifndef HW_ICH9_H
 #define HW_ICH9_H
 
-#include "hw/isa/isa.h"
 #include "hw/sysbus.h"
 #include "hw/i386/pc.h"
 #include "hw/isa/apm.h"
@@ -64,8 +63,6 @@ struct ICH9LPCState {
                                        * triggers feature lockdown */
     uint64_t smi_negotiated_features; /* guest-invisible, host endian */
 
-    /* isa bus */
-    ISABus *isa_bus;
     MemoryRegion rcrb_mem; /* root complex register block */
     Notifier machine_ready;
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 8fac0e57bc..4af8474f31 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -132,7 +132,6 @@ static void pc_q35_init(MachineState *machine)
     GSIState *gsi_state;
     ISABus *isa_bus;
     int i;
-    ICH9LPCState *ich9_lpc;
     PCIDevice *ahci;
     ram_addr_t lowmem;
     DriveInfo *hd[MAX_SATA_PORTS];
@@ -266,12 +265,11 @@ static void pc_q35_init(MachineState *machine)
     /* irq lines */
     gsi_state = pc_gsi_create(&x86ms->gsi, pcmc->pci_enabled);
 
-    ich9_lpc = ICH9_LPC_DEVICE(lpc);
     lpc_dev = DEVICE(lpc);
     for (i = 0; i < GSI_NUM_PINS; i++) {
         qdev_connect_gpio_out_named(lpc_dev, ICH9_GPIO_GSI, i, x86ms->gsi[i]);
     }
-    isa_bus = ich9_lpc->isa_bus;
+    isa_bus = ISA_BUS(qdev_get_child_bus(lpc_dev, "isa.0"));
 
     if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
         pc_i8259_create(isa_bus, gsi_state->i8259_irq);
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 54a8839cd2..71f7c18a2e 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -710,8 +710,6 @@ static void ich9_lpc_realize(PCIDevice *d, Error **errp)
     memory_region_init_io(&lpc->rcrb_mem, OBJECT(d), &rcrb_mmio_ops, lpc,
                           "lpc-rcrb-mmio", ICH9_CC_SIZE);
 
-    lpc->isa_bus = isa_bus;
-
     ich9_cc_init(lpc);
     apm_init(d, &lpc->apm, ich9_apm_ctrl_changed, lpc);
 
@@ -818,8 +816,7 @@ static void ich9_send_gpe(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
 static void build_ich9_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
 {
     Aml *field;
-    ICH9LPCState *s = ICH9_LPC_DEVICE(adev);
-    BusState *bus = BUS(s->isa_bus);
+    BusState *bus = qdev_get_child_bus(DEVICE(adev), "isa.0");
     Aml *sb_scope = aml_scope("\\_SB");
 
     /* ICH9 PCI to ISA irq remapping */
-- 
2.39.1

[PATCH 04/12] hw/i2c/smbus_ich9: Move ich9_smb_set_irq() in front of ich9_smbus_realize()

[Bernhard Beschow]

This is a preparation for the next commit to make it cleaner.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i2c/smbus_ich9.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/hw/i2c/smbus_ich9.c b/hw/i2c/smbus_ich9.c
index 52ba77f3fc..d29c0f6ffa 100644
--- a/hw/i2c/smbus_ich9.c
+++ b/hw/i2c/smbus_ich9.c
@@ -80,6 +80,18 @@ static void ich9_smbus_write_config(PCIDevice *d, uint32_t address,
     }
 }
 
+static void ich9_smb_set_irq(PMSMBus *pmsmb, bool enabled)
+{
+    ICH9SMBState *s = pmsmb->opaque;
+
+    if (enabled == s->irq_enabled) {
+        return;
+    }
+
+    s->irq_enabled = enabled;
+    pci_set_irq(&s->dev, enabled);
+}
+
 static void ich9_smbus_realize(PCIDevice *d, Error **errp)
 {
     ICH9SMBState *s = ICH9_SMB_DEVICE(d);
@@ -125,18 +137,6 @@ static void ich9_smb_class_init(ObjectClass *klass, void *data)
     adevc->build_dev_aml = build_ich9_smb_aml;
 }
 
-static void ich9_smb_set_irq(PMSMBus *pmsmb, bool enabled)
-{
-    ICH9SMBState *s = pmsmb->opaque;
-
-    if (enabled == s->irq_enabled) {
-        return;
-    }
-
-    s->irq_enabled = enabled;
-    pci_set_irq(&s->dev, enabled);
-}
-
 I2CBus *ich9_smb_init(PCIBus *bus, int devfn, uint32_t smb_io_base)
 {
     PCIDevice *d =
-- 
2.39.1

[PATCH 05/12] hw/i2c/smbus_ich9: Inline ich9_smb_init() and remove it

[Bernhard Beschow]

ich9_smb_init() is a legacy init function, so modernize the code.

Note that the smb_io_base parameter was unused.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 include/hw/i386/ich9.h |  1 -
 hw/i2c/smbus_ich9.c    | 13 +++----------
 hw/i386/pc_q35.c       | 11 ++++++++---
 3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index 05464f6965..52ea116f44 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -9,7 +9,6 @@
 #include "qom/object.h"
 
 void ich9_lpc_pm_init(PCIDevice *pci_lpc, bool smm_enabled);
-I2CBus *ich9_smb_init(PCIBus *bus, int devfn, uint32_t smb_io_base);
 
 void ich9_generate_smi(void);
 
diff --git a/hw/i2c/smbus_ich9.c b/hw/i2c/smbus_ich9.c
index d29c0f6ffa..f0dd3cb147 100644
--- a/hw/i2c/smbus_ich9.c
+++ b/hw/i2c/smbus_ich9.c
@@ -105,6 +105,9 @@ static void ich9_smbus_realize(PCIDevice *d, Error **errp)
     pm_smbus_init(&d->qdev, &s->smb, false);
     pci_register_bar(d, ICH9_SMB_SMB_BASE_BAR, PCI_BASE_ADDRESS_SPACE_IO,
                      &s->smb.io);
+
+    s->smb.set_irq = ich9_smb_set_irq;
+    s->smb.opaque = s;
 }
 
 static void build_ich9_smb_aml(AcpiDevAmlIf *adev, Aml *scope)
@@ -137,16 +140,6 @@ static void ich9_smb_class_init(ObjectClass *klass, void *data)
     adevc->build_dev_aml = build_ich9_smb_aml;
 }
 
-I2CBus *ich9_smb_init(PCIBus *bus, int devfn, uint32_t smb_io_base)
-{
-    PCIDevice *d =
-        pci_create_simple_multifunction(bus, devfn, true, TYPE_ICH9_SMB_DEVICE);
-    ICH9SMBState *s = ICH9_SMB_DEVICE(d);
-    s->smb.set_irq = ich9_smb_set_irq;
-    s->smb.opaque = s;
-    return s->smb.smbus;
-}
-
 static const TypeInfo ich9_smb_info = {
     .name   = TYPE_ICH9_SMB_DEVICE,
     .parent = TYPE_PCI_DEVICE,
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 4af8474f31..85ba8ed951 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -316,10 +316,15 @@ static void pc_q35_init(MachineState *machine)
     }
 
     if (pcms->smbus_enabled) {
+        PCIDevice *smb;
+
         /* TODO: Populate SPD eeprom data.  */
-        pcms->smbus = ich9_smb_init(host_bus,
-                                    PCI_DEVFN(ICH9_SMB_DEV, ICH9_SMB_FUNC),
-                                    0xb100);
+        smb = pci_create_simple_multifunction(host_bus,
+                                              PCI_DEVFN(ICH9_SMB_DEV,
+                                                        ICH9_SMB_FUNC),
+                                              true, TYPE_ICH9_SMB_DEVICE);
+        pcms->smbus = I2C_BUS(qdev_get_child_bus(DEVICE(smb), "i2c"));
+
         smbus_eeprom_init(pcms->smbus, 8, NULL, 0);
     }
 
-- 
2.39.1

[PATCH 06/12] hw/i386/pc_q35: Allow for setting properties before realizing TYPE_ICH9_LPC_DEVICE

[Bernhard Beschow]

This is a preparation to make the next patch cleaner.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i386/pc_q35.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 85ba8ed951..dbd2160d4e 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -236,9 +236,9 @@ static void pc_q35_init(MachineState *machine)
     phb = PCI_HOST_BRIDGE(q35_host);
     host_bus = phb->bus;
     /* create ISA bus */
-    lpc = pci_create_simple_multifunction(host_bus, PCI_DEVFN(ICH9_LPC_DEV,
-                                          ICH9_LPC_FUNC), true,
-                                          TYPE_ICH9_LPC_DEVICE);
+    lpc = pci_new_multifunction(PCI_DEVFN(ICH9_LPC_DEV, ICH9_LPC_FUNC), true,
+                                TYPE_ICH9_LPC_DEVICE);
+    pci_realize_and_unref(lpc, host_bus, &error_fatal);
 
     object_property_add_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
                              TYPE_HOTPLUG_HANDLER,
-- 
2.39.1

[PATCH 07/12] hw/isa/lpc_ich9: Connect pm stuff to lpc internally

[Bernhard Beschow]

Make TYPE_ICH9_LPC_DEVICE more self-contained by moving the call to
ich9_lpc_pm_init() from board code to its realize function. In order
to propagate x86_machine_is_smm_enabled(), introduce an "smm-enabled"
property like we have in piix4.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 include/hw/acpi/ich9.h | 6 ++----
 include/hw/i386/ich9.h | 2 --
 hw/acpi/ich9.c         | 8 ++------
 hw/i386/pc_q35.c       | 5 ++---
 hw/isa/lpc_ich9.c      | 8 +++++---
 5 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
index d41866a229..57a542c4b8 100644
--- a/include/hw/acpi/ich9.h
+++ b/include/hw/acpi/ich9.h
@@ -64,7 +64,7 @@ typedef struct ICH9LPCPMRegs {
     uint8_t disable_s3;
     uint8_t disable_s4;
     uint8_t s4_val;
-    uint8_t smm_enabled;
+    bool smm_enabled;
     bool smm_compat;
     bool enable_tco;
     TCOIORegs tco_regs;
@@ -72,9 +72,7 @@ typedef struct ICH9LPCPMRegs {
 
 #define ACPI_PM_PROP_TCO_ENABLED "enable_tco"
 
-void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
-                  bool smm_enabled,
-                  qemu_irq sci_irq);
+void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm, qemu_irq sci_irq);
 
 void ich9_pm_iospace_update(ICH9LPCPMRegs *pm, uint32_t pm_io_base);
 extern const VMStateDescription vmstate_ich9_pm;
diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index 52ea116f44..433c8942c9 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -8,8 +8,6 @@
 #include "hw/acpi/ich9.h"
 #include "qom/object.h"
 
-void ich9_lpc_pm_init(PCIDevice *pci_lpc, bool smm_enabled);
-
 void ich9_generate_smi(void);
 
 #define ICH9_CC_SIZE (16 * 1024) /* 16KB. Chipset configuration registers */
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index a93c470e9d..54bb3d83b3 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -291,9 +291,7 @@ static void pm_powerdown_req(Notifier *n, void *opaque)
     acpi_pm1_evt_power_down(&pm->acpi_regs);
 }
 
-void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
-                  bool smm_enabled,
-                  qemu_irq sci_irq)
+void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm, qemu_irq sci_irq)
 {
     memory_region_init(&pm->io, OBJECT(lpc_pci), "ich9-pm", ICH9_PMIO_SIZE);
     memory_region_set_enabled(&pm->io, false);
@@ -303,7 +301,7 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
     acpi_pm_tmr_init(&pm->acpi_regs, ich9_pm_update_sci_fn, &pm->io);
     acpi_pm1_evt_init(&pm->acpi_regs, ich9_pm_update_sci_fn, &pm->io);
     acpi_pm1_cnt_init(&pm->acpi_regs, &pm->io, pm->disable_s3, pm->disable_s4,
-                      pm->s4_val, !pm->smm_compat && !smm_enabled);
+                      pm->s4_val, !pm->smm_compat && !pm->smm_enabled);
 
     acpi_gpe_init(&pm->acpi_regs, ICH9_PMIO_GPE0_LEN);
     memory_region_init_io(&pm->io_gpe, OBJECT(lpc_pci), &ich9_gpe_ops, pm,
@@ -314,8 +312,6 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
                           "acpi-smi", 8);
     memory_region_add_subregion(&pm->io, ICH9_PMIO_SMI_EN, &pm->io_smi);
 
-    pm->smm_enabled = smm_enabled;
-
     if (pm->enable_tco) {
         acpi_pm_tco_init(&pm->tco_regs, &pm->io);
     }
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index dbd2160d4e..f13b516f2e 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -238,6 +238,8 @@ static void pc_q35_init(MachineState *machine)
     /* create ISA bus */
     lpc = pci_new_multifunction(PCI_DEVFN(ICH9_LPC_DEV, ICH9_LPC_FUNC), true,
                                 TYPE_ICH9_LPC_DEVICE);
+    qdev_prop_set_bit(DEVICE(lpc), "smm-enabled",
+                      x86_machine_is_smm_enabled(x86ms));
     pci_realize_and_unref(lpc, host_bus, &error_fatal);
 
     object_property_add_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
@@ -292,9 +294,6 @@ static void pc_q35_init(MachineState *machine)
     pc_basic_device_init(pcms, isa_bus, x86ms->gsi, &rtc_state, !mc->no_floppy,
                          0xff0104);
 
-    /* connect pm stuff to lpc */
-    ich9_lpc_pm_init(lpc, x86_machine_is_smm_enabled(x86ms));
-
     if (pcms->sata_enabled) {
         /* ahci and SATA device, for q35 1 ahci controller is built-in */
         ahci = pci_create_simple_multifunction(host_bus,
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 71f7c18a2e..c5060d145f 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -407,14 +407,13 @@ static void smi_features_ok_callback(void *opaque)
     lpc->smi_features_ok = 1;
 }
 
-void ich9_lpc_pm_init(PCIDevice *lpc_pci, bool smm_enabled)
+static void ich9_lpc_pm_init(ICH9LPCState *lpc)
 {
-    ICH9LPCState *lpc = ICH9_LPC_DEVICE(lpc_pci);
     qemu_irq sci_irq;
     FWCfgState *fw_cfg = fw_cfg_find();
 
     sci_irq = qemu_allocate_irq(ich9_set_sci, lpc, 0);
-    ich9_pm_init(lpc_pci, &lpc->pm, smm_enabled, sci_irq);
+    ich9_pm_init(PCI_DEVICE(lpc), &lpc->pm, sci_irq);
 
     if (lpc->smi_host_features && fw_cfg) {
         uint64_t host_features_le;
@@ -731,6 +730,8 @@ static void ich9_lpc_realize(PCIDevice *d, Error **errp)
     pci_bus_irqs(pci_bus, ich9_lpc_set_irq, d, ICH9_LPC_NB_PIRQS);
     pci_bus_map_irqs(pci_bus, ich9_lpc_map_irq);
     pci_bus_set_route_irq_fn(pci_bus, ich9_route_intx_pin_to_irq);
+
+    ich9_lpc_pm_init(lpc);
 }
 
 static bool ich9_rst_cnt_needed(void *opaque)
@@ -797,6 +798,7 @@ static const VMStateDescription vmstate_ich9_lpc = {
 static Property ich9_lpc_properties[] = {
     DEFINE_PROP_BOOL("noreboot", ICH9LPCState, pin_strap.spkr_hi, false),
     DEFINE_PROP_BOOL("smm-compat", ICH9LPCState, pm.smm_compat, false),
+    DEFINE_PROP_BOOL("smm-enabled", ICH9LPCState, pm.smm_enabled, false),
     DEFINE_PROP_BIT64("x-smi-broadcast", ICH9LPCState, smi_host_features,
                       ICH9_LPC_SMI_F_BROADCAST_BIT, true),
     DEFINE_PROP_BIT64("x-smi-cpu-hotplug", ICH9LPCState, smi_host_features,
-- 
2.39.1

[PATCH 08/12] hw/isa/lpc_ich9: Remove redundant ich9_lpc_reset() invocation

[Bernhard Beschow]

ich9_lpc_reset() is the dc->reset callback which is called
automatically. No need to call it explicitly during k->realize.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/lpc_ich9.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index c5060d145f..2a4baac129 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -57,8 +57,6 @@
 /*****************************************************************************/
 /* ICH9 LPC PCI to ISA bridge */
 
-static void ich9_lpc_reset(DeviceState *qdev);
-
 /* chipset configuration register
  * to access chipset configuration registers, pci_[sg]et_{byte, word, long}
  * are used.
@@ -439,8 +437,6 @@ static void ich9_lpc_pm_init(ICH9LPCState *lpc)
                                  sizeof lpc->smi_features_ok,
                                  true);
     }
-
-    ich9_lpc_reset(DEVICE(lpc));
 }
 
 /* APM */
-- 
2.39.1

[PATCH 09/12] hw/i386/ich9: Remove redundant GSI_NUM_PINS

[Bernhard Beschow]

Most code uses IOAPIC_NUM_PINS. The only place where GSI_NUM_PINS defines
the size of an array is ICH9LPCState::gsi which needs to match
IOAPIC_NUM_PINS. Remove GSI_NUM_PINS for consistency.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 include/hw/i386/ich9.h | 2 +-
 include/hw/i386/x86.h  | 1 -
 hw/i386/pc.c           | 6 +++---
 hw/i386/pc_q35.c       | 3 ++-
 hw/isa/lpc_ich9.c      | 2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index 433c8942c9..d29090a9b7 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -63,7 +63,7 @@ struct ICH9LPCState {
     MemoryRegion rcrb_mem; /* root complex register block */
     Notifier machine_ready;
 
-    qemu_irq gsi[GSI_NUM_PINS];
+    qemu_irq gsi[IOAPIC_NUM_PINS];
 };
 
 #define ICH9_MASK(bit, ms_bit, ls_bit) \
diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
index ba6912b721..6f52f8fe95 100644
--- a/include/hw/i386/x86.h
+++ b/include/hw/i386/x86.h
@@ -136,7 +136,6 @@ bool x86_machine_is_acpi_enabled(const X86MachineState *x86ms);
 
 /* Global System Interrupts */
 
-#define GSI_NUM_PINS IOAPIC_NUM_PINS
 #define ACPI_BUILD_PCI_IRQS ((1<<5) | (1<<9) | (1<<10) | (1<<11))
 
 typedef struct GSIState {
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 8898cc9961..cbca3f5db5 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -28,7 +28,7 @@
 #include "hw/i386/pc.h"
 #include "hw/char/serial.h"
 #include "hw/char/parallel.h"
-#include "hw/i386/apic.h"
+#include "hw/i386/ioapic.h"
 #include "hw/i386/topology.h"
 #include "hw/i386/fw_cfg.h"
 #include "hw/i386/vmport.h"
@@ -406,7 +406,7 @@ GSIState *pc_gsi_create(qemu_irq **irqs, bool pci_enabled)
     if (kvm_ioapic_in_kernel()) {
         kvm_pc_setup_irq_routing(pci_enabled);
     }
-    *irqs = qemu_allocate_irqs(gsi_handler, s, GSI_NUM_PINS);
+    *irqs = qemu_allocate_irqs(gsi_handler, s, IOAPIC_NUM_PINS);
 
     return s;
 }
@@ -1295,7 +1295,7 @@ void pc_basic_device_init(struct PCMachineState *pcms,
         sysbus_realize_and_unref(SYS_BUS_DEVICE(hpet), &error_fatal);
         sysbus_mmio_map(SYS_BUS_DEVICE(hpet), 0, HPET_BASE);
 
-        for (i = 0; i < GSI_NUM_PINS; i++) {
+        for (i = 0; i < IOAPIC_NUM_PINS; i++) {
             sysbus_connect_irq(SYS_BUS_DEVICE(hpet), i, gsi[i]);
         }
         pit_isa_irq = -1;
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index f13b516f2e..28434612af 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -43,6 +43,7 @@
 #include "hw/i386/ich9.h"
 #include "hw/i386/amd_iommu.h"
 #include "hw/i386/intel_iommu.h"
+#include "hw/i386/ioapic.h"
 #include "hw/display/ramfb.h"
 #include "hw/firmware/smbios.h"
 #include "hw/ide/pci.h"
@@ -268,7 +269,7 @@ static void pc_q35_init(MachineState *machine)
     gsi_state = pc_gsi_create(&x86ms->gsi, pcmc->pci_enabled);
 
     lpc_dev = DEVICE(lpc);
-    for (i = 0; i < GSI_NUM_PINS; i++) {
+    for (i = 0; i < IOAPIC_NUM_PINS; i++) {
         qdev_connect_gpio_out_named(lpc_dev, ICH9_GPIO_GSI, i, x86ms->gsi[i]);
     }
     isa_bus = ISA_BUS(qdev_get_child_bus(lpc_dev, "isa.0"));
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 2a4baac129..e3385ca7be 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -717,7 +717,7 @@ static void ich9_lpc_realize(PCIDevice *d, Error **errp)
                                         ICH9_RST_CNT_IOPORT, &lpc->rst_cnt_mem,
                                         1);
 
-    qdev_init_gpio_out_named(dev, lpc->gsi, ICH9_GPIO_GSI, GSI_NUM_PINS);
+    qdev_init_gpio_out_named(dev, lpc->gsi, ICH9_GPIO_GSI, IOAPIC_NUM_PINS);
 
     isa_bus_irqs(isa_bus, lpc->gsi);
 
-- 
2.39.1

[PATCH 10/12] hw: Move ioapic*.h to intc/

[Bernhard Beschow]

The ioapic sources reside in hw/intc already. Move the headers there as
well.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 MAINTAINERS                                 | 1 +
 include/hw/i386/x86.h                       | 2 +-
 include/hw/{i386 => intc}/ioapic.h          | 6 +++---
 include/hw/{i386 => intc}/ioapic_internal.h | 8 ++++----
 hw/i386/kvm/ioapic.c                        | 3 +--
 hw/i386/pc.c                                | 2 +-
 hw/i386/pc_q35.c                            | 2 +-
 hw/intc/apic.c                              | 2 +-
 hw/intc/ioapic.c                            | 4 ++--
 hw/intc/ioapic_common.c                     | 4 ++--
 target/i386/whpx/whpx-all.c                 | 2 +-
 11 files changed, 18 insertions(+), 18 deletions(-)
 rename include/hw/{i386 => intc}/ioapic.h (93%)
 rename include/hw/{i386 => intc}/ioapic_internal.h (96%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 96e25f62ac..20460ce254 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1708,6 +1708,7 @@ F: include/hw/char/parallel.h
 F: include/hw/dma/i8257.h
 F: include/hw/i2c/pm_smbus.h
 F: include/hw/input/i8042.h
+F: include/hw/intc/ioapic*
 F: include/hw/isa/i8259_internal.h
 F: include/hw/isa/superio.h
 F: include/hw/timer/hpet.h
diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
index 6f52f8fe95..5256c1e848 100644
--- a/include/hw/i386/x86.h
+++ b/include/hw/i386/x86.h
@@ -23,8 +23,8 @@
 #include "hw/i386/topology.h"
 #include "hw/boards.h"
 #include "hw/nmi.h"
+#include "hw/intc/ioapic.h"
 #include "hw/isa/isa.h"
-#include "hw/i386/ioapic.h"
 #include "qom/object.h"
 
 struct X86MachineClass {
diff --git a/include/hw/i386/ioapic.h b/include/hw/intc/ioapic.h
similarity index 93%
rename from include/hw/i386/ioapic.h
rename to include/hw/intc/ioapic.h
index ef37b8a9fd..aa122e25e3 100644
--- a/include/hw/i386/ioapic.h
+++ b/include/hw/intc/ioapic.h
@@ -17,8 +17,8 @@
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
 
-#ifndef HW_IOAPIC_H
-#define HW_IOAPIC_H
+#ifndef HW_INTC_IOAPIC_H
+#define HW_INTC_IOAPIC_H
 
 #define IOAPIC_NUM_PINS 24
 #define IO_APIC_DEFAULT_ADDRESS 0xfec00000
@@ -30,4 +30,4 @@
 
 void ioapic_eoi_broadcast(int vector);
 
-#endif /* HW_IOAPIC_H */
+#endif /* HW_INTC_IOAPIC_H */
diff --git a/include/hw/i386/ioapic_internal.h b/include/hw/intc/ioapic_internal.h
similarity index 96%
rename from include/hw/i386/ioapic_internal.h
rename to include/hw/intc/ioapic_internal.h
index e8ff338d7f..37b8565539 100644
--- a/include/hw/i386/ioapic_internal.h
+++ b/include/hw/intc/ioapic_internal.h
@@ -19,11 +19,11 @@
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
 
-#ifndef QEMU_IOAPIC_INTERNAL_H
-#define QEMU_IOAPIC_INTERNAL_H
+#ifndef HW_INTC_IOAPIC_INTERNAL_H
+#define HW_INTC_IOAPIC_INTERNAL_H
 
 #include "exec/memory.h"
-#include "hw/i386/ioapic.h"
+#include "hw/intc/ioapic.h"
 #include "hw/sysbus.h"
 #include "qemu/notify.h"
 #include "qom/object.h"
@@ -115,4 +115,4 @@ void ioapic_reset_common(DeviceState *dev);
 
 void ioapic_stat_update_irq(IOAPICCommonState *s, int irq, int level);
 
-#endif /* QEMU_IOAPIC_INTERNAL_H */
+#endif /* HW_INTC_IOAPIC_INTERNAL_H */
diff --git a/hw/i386/kvm/ioapic.c b/hw/i386/kvm/ioapic.c
index 272e26b4a2..cd5ea5d60b 100644
--- a/hw/i386/kvm/ioapic.c
+++ b/hw/i386/kvm/ioapic.c
@@ -12,9 +12,8 @@
 
 #include "qemu/osdep.h"
 #include "monitor/monitor.h"
-#include "hw/i386/x86.h"
 #include "hw/qdev-properties.h"
-#include "hw/i386/ioapic_internal.h"
+#include "hw/intc/ioapic_internal.h"
 #include "hw/intc/kvm_irqcount.h"
 #include "sysemu/kvm.h"
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index cbca3f5db5..b6c353346c 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -28,7 +28,6 @@
 #include "hw/i386/pc.h"
 #include "hw/char/serial.h"
 #include "hw/char/parallel.h"
-#include "hw/i386/ioapic.h"
 #include "hw/i386/topology.h"
 #include "hw/i386/fw_cfg.h"
 #include "hw/i386/vmport.h"
@@ -47,6 +46,7 @@
 #include "multiboot.h"
 #include "hw/rtc/mc146818rtc.h"
 #include "hw/intc/i8259.h"
+#include "hw/intc/ioapic.h"
 #include "hw/timer/i8254.h"
 #include "hw/input/i8042.h"
 #include "hw/irq.h"
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 28434612af..da2fa11b6f 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -43,11 +43,11 @@
 #include "hw/i386/ich9.h"
 #include "hw/i386/amd_iommu.h"
 #include "hw/i386/intel_iommu.h"
-#include "hw/i386/ioapic.h"
 #include "hw/display/ramfb.h"
 #include "hw/firmware/smbios.h"
 #include "hw/ide/pci.h"
 #include "hw/ide/ahci.h"
+#include "hw/intc/ioapic.h"
 #include "hw/usb.h"
 #include "hw/usb/hcd-uhci.h"
 #include "qapi/error.h"
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 2d3e55f4e2..0ff060f721 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -20,7 +20,7 @@
 #include "qemu/thread.h"
 #include "hw/i386/apic_internal.h"
 #include "hw/i386/apic.h"
-#include "hw/i386/ioapic.h"
+#include "hw/intc/ioapic.h"
 #include "hw/intc/i8259.h"
 #include "hw/intc/kvm_irqcount.h"
 #include "hw/pci/msi.h"
diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index 264262959d..6364ecab1b 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -24,10 +24,10 @@
 #include "qapi/error.h"
 #include "monitor/monitor.h"
 #include "hw/i386/apic.h"
-#include "hw/i386/ioapic.h"
-#include "hw/i386/ioapic_internal.h"
 #include "hw/i386/x86.h"
 #include "hw/intc/i8259.h"
+#include "hw/intc/ioapic.h"
+#include "hw/intc/ioapic_internal.h"
 #include "hw/pci/msi.h"
 #include "hw/qdev-properties.h"
 #include "sysemu/kvm.h"
diff --git a/hw/intc/ioapic_common.c b/hw/intc/ioapic_common.c
index aa5f760871..b05f436dac 100644
--- a/hw/intc/ioapic_common.c
+++ b/hw/intc/ioapic_common.c
@@ -24,9 +24,9 @@
 #include "qemu/module.h"
 #include "migration/vmstate.h"
 #include "monitor/monitor.h"
-#include "hw/i386/ioapic.h"
-#include "hw/i386/ioapic_internal.h"
 #include "hw/intc/intc.h"
+#include "hw/intc/ioapic.h"
+#include "hw/intc/ioapic_internal.h"
 #include "hw/sysbus.h"
 
 /* ioapic_no count start from 0 to MAX_IOAPICS,
diff --git a/target/i386/whpx/whpx-all.c b/target/i386/whpx/whpx-all.c
index e738d83e81..3d0c0b375f 100644
--- a/target/i386/whpx/whpx-all.c
+++ b/target/i386/whpx/whpx-all.c
@@ -19,7 +19,7 @@
 #include "sysemu/runstate.h"
 #include "qemu/main-loop.h"
 #include "hw/boards.h"
-#include "hw/i386/ioapic.h"
+#include "hw/intc/ioapic.h"
 #include "hw/i386/apic_internal.h"
 #include "qemu/error-report.h"
 #include "qapi/error.h"
-- 
2.39.1

[PATCH 11/12] hw/i386/ich9: Clean up includes

[Bernhard Beschow]

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 include/hw/i386/ich9.h | 8 +++++---
 hw/i386/acpi-build.c   | 1 +
 hw/isa/lpc_ich9.c      | 2 +-
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index d29090a9b7..3125863049 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -1,11 +1,13 @@
 #ifndef HW_ICH9_H
 #define HW_ICH9_H
 
-#include "hw/sysbus.h"
-#include "hw/i386/pc.h"
 #include "hw/isa/apm.h"
-#include "hw/acpi/acpi.h"
 #include "hw/acpi/ich9.h"
+#include "hw/intc/ioapic.h"
+#include "hw/pci/pci.h"
+#include "hw/pci/pci_device.h"
+#include "exec/memory.h"
+#include "qemu/notify.h"
 #include "qom/object.h"
 
 void ich9_generate_smi(void);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b67dcbbb37..1bb73c3e9a 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -59,6 +59,7 @@
 #include "hw/acpi/pcihp.h"
 #include "hw/i386/fw_cfg.h"
 #include "hw/i386/ich9.h"
+#include "hw/i386/pc.h"
 #include "hw/pci/pci_bus.h"
 #include "hw/pci-host/i440fx.h"
 #include "hw/pci-host/q35.h"
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index e3385ca7be..ce946760bb 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -40,8 +40,8 @@
 #include "hw/irq.h"
 #include "hw/isa/apm.h"
 #include "hw/pci/pci.h"
-#include "hw/pci/pci_bridge.h"
 #include "hw/i386/ich9.h"
+#include "hw/i386/pc.h"
 #include "hw/acpi/acpi.h"
 #include "hw/acpi/ich9.h"
 #include "hw/pci/pci_bus.h"
-- 
2.39.1

[PATCH 12/12] hw: Move ich9.h to southbridge/

[Bernhard Beschow]

ICH9 is a south bridge which doesn't necessarily depend on x86, so move
it into the southbridge folder, analoguous to PIIX.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 MAINTAINERS                             | 1 +
 include/hw/{i386 => southbridge}/ich9.h | 6 +++---
 hw/acpi/ich9.c                          | 2 +-
 hw/acpi/ich9_tco.c                      | 2 +-
 hw/i2c/smbus_ich9.c                     | 2 +-
 hw/i386/acpi-build.c                    | 2 +-
 hw/i386/pc_q35.c                        | 2 +-
 hw/isa/lpc_ich9.c                       | 2 +-
 hw/pci-bridge/i82801b11.c               | 2 +-
 tests/qtest/tco-test.c                  | 2 +-
 10 files changed, 12 insertions(+), 11 deletions(-)
 rename include/hw/{i386 => southbridge}/ich9.h (99%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 20460ce254..eb9ee6178a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1675,6 +1675,7 @@ F: hw/i2c/smbus_ich9.c
 F: hw/acpi/piix4.c
 F: hw/acpi/ich9*.c
 F: include/hw/acpi/ich9*.h
+F: include/hw/southbridge/ich9.h
 F: include/hw/southbridge/piix.h
 F: hw/misc/sga.c
 F: hw/isa/apm.c
diff --git a/include/hw/i386/ich9.h b/include/hw/southbridge/ich9.h
similarity index 99%
rename from include/hw/i386/ich9.h
rename to include/hw/southbridge/ich9.h
index 3125863049..7004eecbf9 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/southbridge/ich9.h
@@ -1,5 +1,5 @@
-#ifndef HW_ICH9_H
-#define HW_ICH9_H
+#ifndef HW_SOUTHBRIDGE_ICH9_H
+#define HW_SOUTHBRIDGE_ICH9_H
 
 #include "hw/isa/apm.h"
 #include "hw/acpi/ich9.h"
@@ -242,4 +242,4 @@ struct ICH9LPCState {
 #define ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT          1
 #define ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT       2
 
-#endif /* HW_ICH9_H */
+#endif /* HW_SOUTHBRIDGE_ICH9_H */
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 54bb3d83b3..d23bfcaa6b 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -36,7 +36,7 @@
 #include "hw/acpi/acpi.h"
 #include "hw/acpi/ich9_tco.h"
 
-#include "hw/i386/ich9.h"
+#include "hw/southbridge/ich9.h"
 #include "hw/mem/pc-dimm.h"
 #include "hw/mem/nvdimm.h"
 
diff --git a/hw/acpi/ich9_tco.c b/hw/acpi/ich9_tco.c
index fbf97f81f4..1540f4fd46 100644
--- a/hw/acpi/ich9_tco.c
+++ b/hw/acpi/ich9_tco.c
@@ -9,7 +9,7 @@
 
 #include "qemu/osdep.h"
 #include "sysemu/watchdog.h"
-#include "hw/i386/ich9.h"
+#include "hw/southbridge/ich9.h"
 #include "migration/vmstate.h"
 
 #include "hw/acpi/ich9_tco.h"
diff --git a/hw/i2c/smbus_ich9.c b/hw/i2c/smbus_ich9.c
index f0dd3cb147..18d40e93c1 100644
--- a/hw/i2c/smbus_ich9.c
+++ b/hw/i2c/smbus_ich9.c
@@ -27,7 +27,7 @@
 #include "migration/vmstate.h"
 #include "qemu/module.h"
 
-#include "hw/i386/ich9.h"
+#include "hw/southbridge/ich9.h"
 #include "qom/object.h"
 #include "hw/acpi/acpi_aml_interface.h"
 
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 1bb73c3e9a..d27921fd8f 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -55,10 +55,10 @@
 #include "hw/hyperv/vmbus-bridge.h"
 
 /* Supported chipsets: */
+#include "hw/southbridge/ich9.h"
 #include "hw/southbridge/piix.h"
 #include "hw/acpi/pcihp.h"
 #include "hw/i386/fw_cfg.h"
-#include "hw/i386/ich9.h"
 #include "hw/i386/pc.h"
 #include "hw/pci/pci_bus.h"
 #include "hw/pci-host/i440fx.h"
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index da2fa11b6f..93b34027a3 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -40,7 +40,6 @@
 #include "hw/qdev-properties.h"
 #include "hw/i386/x86.h"
 #include "hw/i386/pc.h"
-#include "hw/i386/ich9.h"
 #include "hw/i386/amd_iommu.h"
 #include "hw/i386/intel_iommu.h"
 #include "hw/display/ramfb.h"
@@ -48,6 +47,7 @@
 #include "hw/ide/pci.h"
 #include "hw/ide/ahci.h"
 #include "hw/intc/ioapic.h"
+#include "hw/southbridge/ich9.h"
 #include "hw/usb.h"
 #include "hw/usb/hcd-uhci.h"
 #include "qapi/error.h"
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index ce946760bb..96fd500502 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -40,7 +40,7 @@
 #include "hw/irq.h"
 #include "hw/isa/apm.h"
 #include "hw/pci/pci.h"
-#include "hw/i386/ich9.h"
+#include "hw/southbridge/ich9.h"
 #include "hw/i386/pc.h"
 #include "hw/acpi/acpi.h"
 #include "hw/acpi/ich9.h"
diff --git a/hw/pci-bridge/i82801b11.c b/hw/pci-bridge/i82801b11.c
index f3b4a14611..0e83cd11b2 100644
--- a/hw/pci-bridge/i82801b11.c
+++ b/hw/pci-bridge/i82801b11.c
@@ -45,7 +45,7 @@
 #include "hw/pci/pci_bridge.h"
 #include "migration/vmstate.h"
 #include "qemu/module.h"
-#include "hw/i386/ich9.h"
+#include "hw/southbridge/ich9.h"
 
 /*****************************************************************************/
 /* ICH9 DMI-to-PCI bridge */
diff --git a/tests/qtest/tco-test.c b/tests/qtest/tco-test.c
index d865e95dfc..0547d41173 100644
--- a/tests/qtest/tco-test.c
+++ b/tests/qtest/tco-test.c
@@ -14,7 +14,7 @@
 #include "libqos/pci-pc.h"
 #include "qapi/qmp/qdict.h"
 #include "hw/pci/pci_regs.h"
-#include "hw/i386/ich9.h"
+#include "hw/southbridge/ich9.h"
 #include "hw/acpi/ich9.h"
 #include "hw/acpi/ich9_tco.h"
 
-- 
2.39.1

Re: [PATCH 05/12] hw/i2c/smbus_ich9: Inline ich9_smb_init() and remove it

[Corey Minyard]

On Mon, Feb 13, 2023 at 06:30:26PM +0100, Bernhard Beschow wrote:
> ich9_smb_init() is a legacy init function, so modernize the code.
> 
> Note that the smb_io_base parameter was unused.

Acked-by: Corey Minyard <cminyard@mvista.com>

> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>  include/hw/i386/ich9.h |  1 -
>  hw/i2c/smbus_ich9.c    | 13 +++----------
>  hw/i386/pc_q35.c       | 11 ++++++++---
>  3 files changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
> index 05464f6965..52ea116f44 100644
> --- a/include/hw/i386/ich9.h
> +++ b/include/hw/i386/ich9.h
> @@ -9,7 +9,6 @@
>  #include "qom/object.h"
>  
>  void ich9_lpc_pm_init(PCIDevice *pci_lpc, bool smm_enabled);
> -I2CBus *ich9_smb_init(PCIBus *bus, int devfn, uint32_t smb_io_base);
>  
>  void ich9_generate_smi(void);
>  
> diff --git a/hw/i2c/smbus_ich9.c b/hw/i2c/smbus_ich9.c
> index d29c0f6ffa..f0dd3cb147 100644
> --- a/hw/i2c/smbus_ich9.c
> +++ b/hw/i2c/smbus_ich9.c
> @@ -105,6 +105,9 @@ static void ich9_smbus_realize(PCIDevice *d, Error **errp)
>      pm_smbus_init(&d->qdev, &s->smb, false);
>      pci_register_bar(d, ICH9_SMB_SMB_BASE_BAR, PCI_BASE_ADDRESS_SPACE_IO,
>                       &s->smb.io);
> +
> +    s->smb.set_irq = ich9_smb_set_irq;
> +    s->smb.opaque = s;
>  }
>  
>  static void build_ich9_smb_aml(AcpiDevAmlIf *adev, Aml *scope)
> @@ -137,16 +140,6 @@ static void ich9_smb_class_init(ObjectClass *klass, void *data)
>      adevc->build_dev_aml = build_ich9_smb_aml;
>  }
>  
> -I2CBus *ich9_smb_init(PCIBus *bus, int devfn, uint32_t smb_io_base)
> -{
> -    PCIDevice *d =
> -        pci_create_simple_multifunction(bus, devfn, true, TYPE_ICH9_SMB_DEVICE);
> -    ICH9SMBState *s = ICH9_SMB_DEVICE(d);
> -    s->smb.set_irq = ich9_smb_set_irq;
> -    s->smb.opaque = s;
> -    return s->smb.smbus;
> -}
> -
>  static const TypeInfo ich9_smb_info = {
>      .name   = TYPE_ICH9_SMB_DEVICE,
>      .parent = TYPE_PCI_DEVICE,
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 4af8474f31..85ba8ed951 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -316,10 +316,15 @@ static void pc_q35_init(MachineState *machine)
>      }
>  
>      if (pcms->smbus_enabled) {
> +        PCIDevice *smb;
> +
>          /* TODO: Populate SPD eeprom data.  */
> -        pcms->smbus = ich9_smb_init(host_bus,
> -                                    PCI_DEVFN(ICH9_SMB_DEV, ICH9_SMB_FUNC),
> -                                    0xb100);
> +        smb = pci_create_simple_multifunction(host_bus,
> +                                              PCI_DEVFN(ICH9_SMB_DEV,
> +                                                        ICH9_SMB_FUNC),
> +                                              true, TYPE_ICH9_SMB_DEVICE);
> +        pcms->smbus = I2C_BUS(qdev_get_child_bus(DEVICE(smb), "i2c"));
> +
>          smbus_eeprom_init(pcms->smbus, 8, NULL, 0);
>      }
>  
> -- 
> 2.39.1
> 
> 

Re: [PATCH 05/12] hw/i2c/smbus_ich9: Inline ich9_smb_init() and remove it

[Philippe Mathieu-Daudé]

On 18/2/23 21:25, Corey Minyard wrote:
> On Mon, Feb 13, 2023 at 06:30:26PM +0100, Bernhard Beschow wrote:
>> ich9_smb_init() is a legacy init function, so modernize the code.
>>
>> Note that the smb_io_base parameter was unused.
> 
> Acked-by: Corey Minyard <cminyard@mvista.com>
> 
>>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>>   include/hw/i386/ich9.h |  1 -
>>   hw/i2c/smbus_ich9.c    | 13 +++----------
>>   hw/i386/pc_q35.c       | 11 ++++++++---
>>   3 files changed, 11 insertions(+), 14 deletions(-)
>>
>> diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
>> index 05464f6965..52ea116f44 100644
>> --- a/include/hw/i386/ich9.h
>> +++ b/include/hw/i386/ich9.h
>> @@ -9,7 +9,6 @@
>>   #include "qom/object.h"
>>   
>>   void ich9_lpc_pm_init(PCIDevice *pci_lpc, bool smm_enabled);
>> -I2CBus *ich9_smb_init(PCIBus *bus, int devfn, uint32_t smb_io_base);
>>   
>>   void ich9_generate_smi(void);
>>   
>> diff --git a/hw/i2c/smbus_ich9.c b/hw/i2c/smbus_ich9.c
>> index d29c0f6ffa..f0dd3cb147 100644
>> --- a/hw/i2c/smbus_ich9.c
>> +++ b/hw/i2c/smbus_ich9.c
>> @@ -105,6 +105,9 @@ static void ich9_smbus_realize(PCIDevice *d, Error **errp)
>>       pm_smbus_init(&d->qdev, &s->smb, false);
>>       pci_register_bar(d, ICH9_SMB_SMB_BASE_BAR, PCI_BASE_ADDRESS_SPACE_IO,
>>                        &s->smb.io);
>> +
>> +    s->smb.set_irq = ich9_smb_set_irq;
>> +    s->smb.opaque = s;

Corey, shouldn't we expand pm_smbus_init() to take set_irq / opaque
arguments?

>>   }

Re: [PATCH 05/12] hw/i2c/smbus_ich9: Inline ich9_smb_init() and remove it

[Corey Minyard]

On Sun, Feb 19, 2023 at 02:45:44PM +0100, Philippe Mathieu-Daudé wrote:
> On 18/2/23 21:25, Corey Minyard wrote:
> > On Mon, Feb 13, 2023 at 06:30:26PM +0100, Bernhard Beschow wrote:
> > > ich9_smb_init() is a legacy init function, so modernize the code.
> > > 
> > > Note that the smb_io_base parameter was unused.
> > 
> > Acked-by: Corey Minyard <cminyard@mvista.com>
> > 
> > > 
> > > Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> > > ---
> > >   include/hw/i386/ich9.h |  1 -
> > >   hw/i2c/smbus_ich9.c    | 13 +++----------
> > >   hw/i386/pc_q35.c       | 11 ++++++++---
> > >   3 files changed, 11 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
> > > index 05464f6965..52ea116f44 100644
> > > --- a/include/hw/i386/ich9.h
> > > +++ b/include/hw/i386/ich9.h
> > > @@ -9,7 +9,6 @@
> > >   #include "qom/object.h"
> > >   void ich9_lpc_pm_init(PCIDevice *pci_lpc, bool smm_enabled);
> > > -I2CBus *ich9_smb_init(PCIBus *bus, int devfn, uint32_t smb_io_base);
> > >   void ich9_generate_smi(void);
> > > diff --git a/hw/i2c/smbus_ich9.c b/hw/i2c/smbus_ich9.c
> > > index d29c0f6ffa..f0dd3cb147 100644
> > > --- a/hw/i2c/smbus_ich9.c
> > > +++ b/hw/i2c/smbus_ich9.c
> > > @@ -105,6 +105,9 @@ static void ich9_smbus_realize(PCIDevice *d, Error **errp)
> > >       pm_smbus_init(&d->qdev, &s->smb, false);
> > >       pci_register_bar(d, ICH9_SMB_SMB_BASE_BAR, PCI_BASE_ADDRESS_SPACE_IO,
> > >                        &s->smb.io);
> > > +
> > > +    s->smb.set_irq = ich9_smb_set_irq;
> > > +    s->smb.opaque = s;
> 
> Corey, shouldn't we expand pm_smbus_init() to take set_irq / opaque
> arguments?

That would be nice, but the other two user of this function,
hw/isa/vt82c686.c and hw/acpi/piix4.c, don't use these fields.
I doubt we are getting any new users.  I'm fine either way, but the
value is not large.

-corey

> 
> > >   }
> 

Re: [PATCH 01/12] hw/i386/ich9: Rename Q35_MASK to ICH9_MASK

[Philippe Mathieu-Daudé]

On 13/2/23 18:30, Bernhard Beschow wrote:
> The Q35_MASK macro is already defined by TYPE_Q35_HOST_DEVICE, so let
> TYPE_ICH9_LPC_DEVICE have its own one to prevent potential name clash.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> ---
>   include/hw/i386/ich9.h | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 02/12] hw/isa/lpc_ich9: Unexport PIRQ functions

[Philippe Mathieu-Daudé]

On 13/2/23 18:30, Bernhard Beschow wrote:
> No need to rely on the board to wire up the ICH9 PCI IRQs. All functions
> access private state of the LPC device which suggests that it should
> wire up the IRQs.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   include/hw/i386/ich9.h |  3 ---
>   hw/i386/pc_q35.c       |  3 ---
>   hw/isa/lpc_ich9.c      | 11 ++++++++---
>   3 files changed, 8 insertions(+), 9 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 03/12] hw/isa/lpc_ich9: Eliminate ICH9LPCState::isa_bus

[Philippe Mathieu-Daudé]

On 13/2/23 18:30, Bernhard Beschow wrote:
> By using qdev_get_child_bus() we can eliminate ICH9LPCState::isa_bus and
> spare the ich9_lpc variable in pc_q35, too.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> ---
>   include/hw/i386/ich9.h | 3 ---
>   hw/i386/pc_q35.c       | 4 +---
>   hw/isa/lpc_ich9.c      | 5 +----
>   3 files changed, 2 insertions(+), 10 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 04/12] hw/i2c/smbus_ich9: Move ich9_smb_set_irq() in front of ich9_smbus_realize()

[Philippe Mathieu-Daudé]

On 13/2/23 18:30, Bernhard Beschow wrote:
> This is a preparation for the next commit to make it cleaner.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/i2c/smbus_ich9.c | 24 ++++++++++++------------
>   1 file changed, 12 insertions(+), 12 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 05/12] hw/i2c/smbus_ich9: Inline ich9_smb_init() and remove it

[Philippe Mathieu-Daudé]

On 19/2/23 15:21, Corey Minyard wrote:
> On Sun, Feb 19, 2023 at 02:45:44PM +0100, Philippe Mathieu-Daudé wrote:
>> On 18/2/23 21:25, Corey Minyard wrote:
>>> On Mon, Feb 13, 2023 at 06:30:26PM +0100, Bernhard Beschow wrote:
>>>> ich9_smb_init() is a legacy init function, so modernize the code.
>>>>
>>>> Note that the smb_io_base parameter was unused.
>>>
>>> Acked-by: Corey Minyard <cminyard@mvista.com>
>>>
>>>>
>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>>> ---
>>>>    include/hw/i386/ich9.h |  1 -
>>>>    hw/i2c/smbus_ich9.c    | 13 +++----------
>>>>    hw/i386/pc_q35.c       | 11 ++++++++---
>>>>    3 files changed, 11 insertions(+), 14 deletions(-)
>>>>
>>>> diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
>>>> index 05464f6965..52ea116f44 100644
>>>> --- a/include/hw/i386/ich9.h
>>>> +++ b/include/hw/i386/ich9.h
>>>> @@ -9,7 +9,6 @@
>>>>    #include "qom/object.h"
>>>>    void ich9_lpc_pm_init(PCIDevice *pci_lpc, bool smm_enabled);
>>>> -I2CBus *ich9_smb_init(PCIBus *bus, int devfn, uint32_t smb_io_base);
>>>>    void ich9_generate_smi(void);
>>>> diff --git a/hw/i2c/smbus_ich9.c b/hw/i2c/smbus_ich9.c
>>>> index d29c0f6ffa..f0dd3cb147 100644
>>>> --- a/hw/i2c/smbus_ich9.c
>>>> +++ b/hw/i2c/smbus_ich9.c
>>>> @@ -105,6 +105,9 @@ static void ich9_smbus_realize(PCIDevice *d, Error **errp)
>>>>        pm_smbus_init(&d->qdev, &s->smb, false);
>>>>        pci_register_bar(d, ICH9_SMB_SMB_BASE_BAR, PCI_BASE_ADDRESS_SPACE_IO,
>>>>                         &s->smb.io);
>>>> +
>>>> +    s->smb.set_irq = ich9_smb_set_irq;
>>>> +    s->smb.opaque = s;
>>
>> Corey, shouldn't we expand pm_smbus_init() to take set_irq / opaque
>> arguments?
> 
> That would be nice, but the other two user of this function,
> hw/isa/vt82c686.c and hw/acpi/piix4.c, don't use these fields.
> I doubt we are getting any new users.  I'm fine either way, but the
> value is not large.

The value is in enforcing stricter APIs (providing good examples).

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 06/12] hw/i386/pc_q35: Allow for setting properties before realizing TYPE_ICH9_LPC_DEVICE

[Philippe Mathieu-Daudé]

On 13/2/23 18:30, Bernhard Beschow wrote:
> This is a preparation to make the next patch cleaner.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/i386/pc_q35.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 07/12] hw/isa/lpc_ich9: Connect pm stuff to lpc internally

[Philippe Mathieu-Daudé]

On 13/2/23 18:30, Bernhard Beschow wrote:
> Make TYPE_ICH9_LPC_DEVICE more self-contained by moving the call to
> ich9_lpc_pm_init() from board code to its realize function. In order
> to propagate x86_machine_is_smm_enabled(), introduce an "smm-enabled"
> property like we have in piix4.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   include/hw/acpi/ich9.h | 6 ++----
>   include/hw/i386/ich9.h | 2 --
>   hw/acpi/ich9.c         | 8 ++------
>   hw/i386/pc_q35.c       | 5 ++---
>   hw/isa/lpc_ich9.c      | 8 +++++---
>   5 files changed, 11 insertions(+), 18 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 08/12] hw/isa/lpc_ich9: Remove redundant ich9_lpc_reset() invocation

[Philippe Mathieu-Daudé]

On 13/2/23 18:30, Bernhard Beschow wrote:
> ich9_lpc_reset() is the dc->reset callback which is called
> automatically. No need to call it explicitly during k->realize.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/isa/lpc_ich9.c | 4 ----
>   1 file changed, 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 09/12] hw/i386/ich9: Remove redundant GSI_NUM_PINS

[Philippe Mathieu-Daudé]

On 13/2/23 18:30, Bernhard Beschow wrote:
> Most code uses IOAPIC_NUM_PINS. The only place where GSI_NUM_PINS defines
> the size of an array is ICH9LPCState::gsi which needs to match
> IOAPIC_NUM_PINS. Remove GSI_NUM_PINS for consistency.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   include/hw/i386/ich9.h | 2 +-
>   include/hw/i386/x86.h  | 1 -
>   hw/i386/pc.c           | 6 +++---
>   hw/i386/pc_q35.c       | 3 ++-
>   hw/isa/lpc_ich9.c      | 2 +-
>   5 files changed, 7 insertions(+), 7 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 12/12] hw: Move ich9.h to southbridge/

[Philippe Mathieu-Daudé]

On 13/2/23 18:30, Bernhard Beschow wrote:
> ICH9 is a south bridge which doesn't necessarily depend on x86, so move
> it into the southbridge folder, analoguous to PIIX.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   MAINTAINERS                             | 1 +
>   include/hw/{i386 => southbridge}/ich9.h | 6 +++---
>   hw/acpi/ich9.c                          | 2 +-
>   hw/acpi/ich9_tco.c                      | 2 +-
>   hw/i2c/smbus_ich9.c                     | 2 +-
>   hw/i386/acpi-build.c                    | 2 +-
>   hw/i386/pc_q35.c                        | 2 +-
>   hw/isa/lpc_ich9.c                       | 2 +-
>   hw/pci-bridge/i82801b11.c               | 2 +-
>   tests/qtest/tco-test.c                  | 2 +-
>   10 files changed, 12 insertions(+), 11 deletions(-)
>   rename include/hw/{i386 => southbridge}/ich9.h (99%)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 10/12] hw: Move ioapic*.h to intc/

[Philippe Mathieu-Daudé]

On 13/2/23 18:30, Bernhard Beschow wrote:
> The ioapic sources reside in hw/intc already. Move the headers there as
> well.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   MAINTAINERS                                 | 1 +
>   include/hw/i386/x86.h                       | 2 +-
>   include/hw/{i386 => intc}/ioapic.h          | 6 +++---
>   include/hw/{i386 => intc}/ioapic_internal.h | 8 ++++----

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

>   hw/i386/kvm/ioapic.c                        | 3 +--
>   hw/i386/pc.c                                | 2 +-
>   hw/i386/pc_q35.c                            | 2 +-
>   hw/intc/apic.c                              | 2 +-
>   hw/intc/ioapic.c                            | 4 ++--
>   hw/intc/ioapic_common.c                     | 4 ++--
>   target/i386/whpx/whpx-all.c                 | 2 +-
>   11 files changed, 18 insertions(+), 18 deletions(-)
>   rename include/hw/{i386 => intc}/ioapic.h (93%)
>   rename include/hw/{i386 => intc}/ioapic_internal.h (96%)

Re: [PATCH 11/12] hw/i386/ich9: Clean up includes

[Philippe Mathieu-Daudé]

On 13/2/23 18:30, Bernhard Beschow wrote:
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   include/hw/i386/ich9.h | 8 +++++---
>   hw/i386/acpi-build.c   | 1 +
>   hw/isa/lpc_ich9.c      | 2 +-
>   3 files changed, 7 insertions(+), 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 12/12] hw: Move ich9.h to southbridge/

[Philippe Mathieu-Daudé]

On 13/2/23 18:30, Bernhard Beschow wrote:
> ICH9 is a south bridge which doesn't necessarily depend on x86, so move
> it into the southbridge folder, analoguous to PIIX.

However it is still tied to it due to:

hw/isa/lpc_ich9.c:315:    cpu_interrupt(first_cpu, CPU_INTERRUPT_SMI);
hw/isa/lpc_ich9.c:462:                cpu_interrupt(cs, CPU_INTERRUPT_SMI);
hw/isa/lpc_ich9.c:465:            cpu_interrupt(current_cpu, 
CPU_INTERRUPT_SMI);
target/i386/cpu.h:1145:#define CPU_INTERRUPT_SMI 
CPU_INTERRUPT_TGT_EXT_2

> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   MAINTAINERS                             | 1 +
>   include/hw/{i386 => southbridge}/ich9.h | 6 +++---
>   hw/acpi/ich9.c                          | 2 +-
>   hw/acpi/ich9_tco.c                      | 2 +-
>   hw/i2c/smbus_ich9.c                     | 2 +-
>   hw/i386/acpi-build.c                    | 2 +-
>   hw/i386/pc_q35.c                        | 2 +-
>   hw/isa/lpc_ich9.c                       | 2 +-
>   hw/pci-bridge/i82801b11.c               | 2 +-
>   tests/qtest/tco-test.c                  | 2 +-
>   10 files changed, 12 insertions(+), 11 deletions(-)
>   rename include/hw/{i386 => southbridge}/ich9.h (99%)

Re: [PATCH 05/12] hw/i2c/smbus_ich9: Inline ich9_smb_init() and remove it

[Corey Minyard]

On Mon, Feb 27, 2023 at 12:53:23PM +0100, Philippe Mathieu-Daudé wrote:
> On 19/2/23 15:21, Corey Minyard wrote:
> > On Sun, Feb 19, 2023 at 02:45:44PM +0100, Philippe Mathieu-Daudé wrote:
> > > On 18/2/23 21:25, Corey Minyard wrote:
> > > > On Mon, Feb 13, 2023 at 06:30:26PM +0100, Bernhard Beschow wrote:
> > > > > ich9_smb_init() is a legacy init function, so modernize the code.
> > > > > 
> > > > > Note that the smb_io_base parameter was unused.
> > > > 
> > > > Acked-by: Corey Minyard <cminyard@mvista.com>
> > > > 
> > > > > 
> > > > > Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> > > > > ---
> > > > >    include/hw/i386/ich9.h |  1 -
> > > > >    hw/i2c/smbus_ich9.c    | 13 +++----------
> > > > >    hw/i386/pc_q35.c       | 11 ++++++++---
> > > > >    3 files changed, 11 insertions(+), 14 deletions(-)
> > > > > 
> > > > > diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
> > > > > index 05464f6965..52ea116f44 100644
> > > > > --- a/include/hw/i386/ich9.h
> > > > > +++ b/include/hw/i386/ich9.h
> > > > > @@ -9,7 +9,6 @@
> > > > >    #include "qom/object.h"
> > > > >    void ich9_lpc_pm_init(PCIDevice *pci_lpc, bool smm_enabled);
> > > > > -I2CBus *ich9_smb_init(PCIBus *bus, int devfn, uint32_t smb_io_base);
> > > > >    void ich9_generate_smi(void);
> > > > > diff --git a/hw/i2c/smbus_ich9.c b/hw/i2c/smbus_ich9.c
> > > > > index d29c0f6ffa..f0dd3cb147 100644
> > > > > --- a/hw/i2c/smbus_ich9.c
> > > > > +++ b/hw/i2c/smbus_ich9.c
> > > > > @@ -105,6 +105,9 @@ static void ich9_smbus_realize(PCIDevice *d, Error **errp)
> > > > >        pm_smbus_init(&d->qdev, &s->smb, false);
> > > > >        pci_register_bar(d, ICH9_SMB_SMB_BASE_BAR, PCI_BASE_ADDRESS_SPACE_IO,
> > > > >                         &s->smb.io);
> > > > > +
> > > > > +    s->smb.set_irq = ich9_smb_set_irq;
> > > > > +    s->smb.opaque = s;
> > > 
> > > Corey, shouldn't we expand pm_smbus_init() to take set_irq / opaque
> > > arguments?
> > 
> > That would be nice, but the other two user of this function,
> > hw/isa/vt82c686.c and hw/acpi/piix4.c, don't use these fields.
> > I doubt we are getting any new users.  I'm fine either way, but the
> > value is not large.
> 
> The value is in enforcing stricter APIs (providing good examples).

I agree, and the change would be good, but IMHO it's beyond the scope of
this change and would slow things down.

I'll prepare a patch that does this.

Thanks,

-corey

> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> 

Re: [PATCH 12/12] hw: Move ich9.h to southbridge/

[Michael S. Tsirkin]

On Mon, Feb 27, 2023 at 01:22:37PM +0100, Philippe Mathieu-Daudé wrote:
> On 13/2/23 18:30, Bernhard Beschow wrote:
> > ICH9 is a south bridge which doesn't necessarily depend on x86, so move
> > it into the southbridge folder, analoguous to PIIX.
> 
> However it is still tied to it due to:
> 
> hw/isa/lpc_ich9.c:315:    cpu_interrupt(first_cpu, CPU_INTERRUPT_SMI);
> hw/isa/lpc_ich9.c:462:                cpu_interrupt(cs, CPU_INTERRUPT_SMI);
> hw/isa/lpc_ich9.c:465:            cpu_interrupt(current_cpu,
> CPU_INTERRUPT_SMI);
> target/i386/cpu.h:1145:#define CPU_INTERRUPT_SMI CPU_INTERRUPT_TGT_EXT_2

I guess at least the commit log should be changed then.


> > Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> > ---
> >   MAINTAINERS                             | 1 +
> >   include/hw/{i386 => southbridge}/ich9.h | 6 +++---
> >   hw/acpi/ich9.c                          | 2 +-
> >   hw/acpi/ich9_tco.c                      | 2 +-
> >   hw/i2c/smbus_ich9.c                     | 2 +-
> >   hw/i386/acpi-build.c                    | 2 +-
> >   hw/i386/pc_q35.c                        | 2 +-
> >   hw/isa/lpc_ich9.c                       | 2 +-
> >   hw/pci-bridge/i82801b11.c               | 2 +-
> >   tests/qtest/tco-test.c                  | 2 +-
> >   10 files changed, 12 insertions(+), 11 deletions(-)
> >   rename include/hw/{i386 => southbridge}/ich9.h (99%)