[PATCH 00/12] hw/i386/pc: Move PC-machine specific declarations to 'pc_internal.h'

[PATCH 00/12] hw/i386/pc: Move PC-machine specific declarations to 'pc_internal.h'

[Philippe Mathieu-Daudé]

Hi Paolo,

Since you posted your "x86: allow building without PC machine
types" series [1], I looked at my past work on this topic
(restrict "hw/i386/pc.h" to the X86 architecture).
I'm glad to see in [2] you remove most (all) of the last uses.
Since I haven't looked at this for some time, my WiP branch was
quite diverged from QEMU master. I guess I could salvage most of
the easy patches. The rest is QOMification of GSI/IOAPIC which
require various changes with the i8259, so I'll keep that for
later.

[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg664760.html
[2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg664765.html

Philippe Mathieu-Daudé (12):
  hw/i386/pc: Convert DPRINTF() to trace events
  hw/i386/pc: Move kvm_i8259_init() declaration to sysemu/kvm.h
  hw/i386/pc: Remove obsolete pc_pci_device_init() declaration
  hw/i386/pc: Remove obsolete cpu_set_smm_t typedef
  hw/i386/ich9: Remove unused include
  hw/i386/ich9: Move unnecessary "pci_bridge.h" include
  hw/ide/piix: Remove superfluous DEVICE() cast
  hw/ide/piix: Use ARRAY_SIZE() instead of magic numbers
  hw/intc/ioapic: Make ioapic_print_redtbl() static
  hw/i386/pc: Rename allocate_cpu_irq from 'pc' to 'x86_machine'
  hw/i386/pc: Move x86_machine_allocate_cpu_irq() to 'hw/i386/x86.c'
  hw/i386/pc: Move PC-machine specific declarations to 'pc_internal.h'

 hw/i386/pc_internal.h             | 144 ++++++++++++++++++++++++++++++
 include/hw/i386/ich9.h            |   2 -
 include/hw/i386/ioapic_internal.h |   1 -
 include/hw/i386/pc.h              | 133 ---------------------------
 include/hw/i386/x86.h             |   2 +
 include/sysemu/kvm.h              |   1 +
 hw/i386/acpi-build.c              |   2 +
 hw/i386/microvm.c                 |   2 +-
 hw/i386/pc.c                      |  47 ++--------
 hw/i386/pc_piix.c                 |   1 +
 hw/i386/pc_q35.c                  |   1 +
 hw/i386/pc_sysfw.c                |   1 +
 hw/i386/x86.c                     |  30 +++++++
 hw/i386/xen/xen-hvm.c             |   1 +
 hw/ide/piix.c                     |  29 +++---
 hw/intc/ioapic_common.c           |   2 +-
 hw/pci-bridge/i82801b11.c         |   1 +
 hw/i386/trace-events              |   6 ++
 18 files changed, 211 insertions(+), 195 deletions(-)
 create mode 100644 hw/i386/pc_internal.h

-- 
2.21.0

[PATCH 01/12] hw/i386/pc: Convert DPRINTF() to trace events

[Philippe Mathieu-Daudé]

Convert the deprecated DPRINTF() macro to trace events.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2: rename pc_pic -> x86_pic
---
 hw/i386/pc.c         | 19 +++++--------------
 hw/i386/trace-events |  6 ++++++
 2 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index ac08e63604..5f8e39c025 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -90,16 +90,7 @@
 #include "config-devices.h"
 #include "e820_memory_layout.h"
 #include "fw_cfg.h"
-
-/* debug PC/ISA interrupts */
-//#define DEBUG_IRQ
-
-#ifdef DEBUG_IRQ
-#define DPRINTF(fmt, ...)                                       \
-    do { printf("CPUIRQ: " fmt , ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...)
-#endif
+#include "trace.h"
 
 struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
 
@@ -348,7 +339,7 @@ void gsi_handler(void *opaque, int n, int level)
 {
     GSIState *s = opaque;
 
-    DPRINTF("pc: %s GSI %d\n", level ? "raising" : "lowering", n);
+    trace_x86_gsi_interrupt(n, level);
     if (n < ISA_NUM_IRQS) {
         qemu_set_irq(s->i8259_irq[n], level);
     }
@@ -426,7 +417,7 @@ static void pic_irq_request(void *opaque, int irq, int level)
     CPUState *cs = first_cpu;
     X86CPU *cpu = X86_CPU(cs);
 
-    DPRINTF("pic_irqs: %s irq %d\n", level? "raise" : "lower", irq);
+    trace_x86_pic_interrupt(irq, level);
     if (cpu->apic_state && !kvm_irqchip_in_kernel()) {
         CPU_FOREACH(cs) {
             cpu = X86_CPU(cs);
@@ -760,7 +751,7 @@ static void port92_write(void *opaque, hwaddr addr, uint64_t val,
     Port92State *s = opaque;
     int oldval = s->outport;
 
-    DPRINTF("port92: write 0x%02" PRIx64 "\n", val);
+    trace_port92_write(val);
     s->outport = val;
     qemu_set_irq(s->a20_out, (val >> 1) & 1);
     if ((val & 1) && !(oldval & 1)) {
@@ -775,7 +766,7 @@ static uint64_t port92_read(void *opaque, hwaddr addr,
     uint32_t ret;
 
     ret = s->outport;
-    DPRINTF("port92: read 0x%02x\n", ret);
+    trace_port92_read(ret);
     return ret;
 }
 
diff --git a/hw/i386/trace-events b/hw/i386/trace-events
index c8bc464bc5..a608a5b635 100644
--- a/hw/i386/trace-events
+++ b/hw/i386/trace-events
@@ -111,3 +111,9 @@ amdvi_ir_irte_ga_val(uint64_t hi, uint64_t lo) "hi 0x%"PRIx64" lo 0x%"PRIx64
 # vmport.c
 vmport_register(unsigned char command, void *func, void *opaque) "command: 0x%02x func: %p opaque: %p"
 vmport_command(unsigned char command) "command: 0x%02x"
+
+# pc.c
+x86_gsi_interrupt(int irqn, int level) "GSI interrupt #%d level:%d"
+x86_pic_interrupt(int irqn, int level) "PIC interrupt #%d level:%d"
+port92_read(uint8_t val) "port92: read 0x%02x"
+port92_write(uint8_t val) "port92: write 0x%02x"
-- 
2.21.0

[PATCH 02/12] hw/i386/pc: Move kvm_i8259_init() declaration to sysemu/kvm.h

[Philippe Mathieu-Daudé]

Move the KVM-related call to "sysemu/kvm.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/hw/i386/pc.h | 1 -
 include/sysemu/kvm.h | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 1f86eba3f9..9866dfbd60 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -138,7 +138,6 @@ typedef struct PCMachineClass {
 
 extern DeviceState *isa_pic;
 qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq);
-qemu_irq *kvm_i8259_init(ISABus *bus);
 int pic_read_irq(DeviceState *d);
 int pic_get_output(DeviceState *d);
 
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 9fe233b9bf..0f57667f79 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -518,6 +518,7 @@ void kvm_irqchip_set_qemuirq_gsi(KVMState *s, qemu_irq irq, int gsi);
 void kvm_pc_gsi_handler(void *opaque, int n, int level);
 void kvm_pc_setup_irq_routing(bool pci_enabled);
 void kvm_init_irq_routing(KVMState *s);
+qemu_irq *kvm_i8259_init(ISABus *bus);
 
 /**
  * kvm_arch_irqchip_create:
-- 
2.21.0

[PATCH 03/12] hw/i386/pc: Remove obsolete pc_pci_device_init() declaration

[Philippe Mathieu-Daudé]

In commit 1454509726 we removed the pc_pci_device_init()
deprecated function and its calls, but we forgot to remove
its prototype. Do that now.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/hw/i386/pc.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 9866dfbd60..bc7d855aaa 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -211,7 +211,6 @@ void pc_cmos_init(PCMachineState *pcms,
                   BusState *ide0, BusState *ide1,
                   ISADevice *s);
 void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus);
-void pc_pci_device_init(PCIBus *pci_bus);
 
 typedef void (*cpu_set_smm_t)(int smm, void *arg);
 
-- 
2.21.0

[PATCH 04/12] hw/i386/pc: Remove obsolete cpu_set_smm_t typedef

[Philippe Mathieu-Daudé]

In commit f809c6051 we replaced the use of cpu_set_smm_t callbacks
by using a Notifier to modify the MemoryRegion. This prototype is
now not used anymore, we can safely remove it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/hw/i386/pc.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index bc7d855aaa..743141e107 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -212,8 +212,6 @@ void pc_cmos_init(PCMachineState *pcms,
                   ISADevice *s);
 void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus);
 
-typedef void (*cpu_set_smm_t)(int smm, void *arg);
-
 void pc_i8259_create(ISABus *isa_bus, qemu_irq *i8259_irqs);
 void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name);
 
-- 
2.21.0

[PATCH 05/12] hw/i386/ich9: Remove unused include

[Philippe Mathieu-Daudé]

The "pcie_host.h" header is used by devices providing a PCI-e bus,
usually North Bridges. The ICH9 is a South Bridge.
Since we don't need this header, do not include it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/hw/i386/ich9.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index 72e803f6e2..eeb79ac1fe 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -7,7 +7,6 @@
 #include "hw/isa/apm.h"
 #include "hw/i386/ioapic.h"
 #include "hw/pci/pci.h"
-#include "hw/pci/pcie_host.h"
 #include "hw/pci/pci_bridge.h"
 #include "hw/acpi/acpi.h"
 #include "hw/acpi/ich9.h"
-- 
2.21.0

[PATCH 06/12] hw/i386/ich9: Move unnecessary "pci_bridge.h" include

[Philippe Mathieu-Daudé]

While the ICH9 chipset is a 'South Bridge', it is not a PCI bridge.
Nothing in "hw/i386/ich9.h" requires definitions from "pci_bridge.h"
so move its inclusion where it is required.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/hw/i386/ich9.h    | 1 -
 hw/i386/acpi-build.c      | 1 +
 hw/pci-bridge/i82801b11.c | 1 +
 3 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index eeb79ac1fe..369bc64671 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -7,7 +7,6 @@
 #include "hw/isa/apm.h"
 #include "hw/i386/ioapic.h"
 #include "hw/pci/pci.h"
-#include "hw/pci/pci_bridge.h"
 #include "hw/acpi/acpi.h"
 #include "hw/acpi/ich9.h"
 #include "hw/pci/pci_bus.h"
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 12ff55fcfb..291909fa05 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -27,6 +27,7 @@
 #include "qemu/bitmap.h"
 #include "qemu/error-report.h"
 #include "hw/pci/pci.h"
+#include "hw/pci/pci_bridge.h"
 #include "hw/core/cpu.h"
 #include "target/i386/cpu.h"
 #include "hw/misc/pvpanic.h"
diff --git a/hw/pci-bridge/i82801b11.c b/hw/pci-bridge/i82801b11.c
index 2b3907655b..033b3c43c4 100644
--- a/hw/pci-bridge/i82801b11.c
+++ b/hw/pci-bridge/i82801b11.c
@@ -43,6 +43,7 @@
 
 #include "qemu/osdep.h"
 #include "hw/pci/pci.h"
+#include "hw/pci/pci_bridge.h"
 #include "migration/vmstate.h"
 #include "qemu/module.h"
 #include "hw/i386/ich9.h"
-- 
2.21.0

[PATCH 07/12] hw/ide/piix: Remove superfluous DEVICE() cast

[Philippe Mathieu-Daudé]

Commit 02a9594b4f0 already converted 'dev' to DeviceState.
Since the cast is superfluous, remove it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/ide/piix.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index db313dd3b1..ffeff4e095 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -193,7 +193,8 @@ int pci_piix3_xen_ide_unplug(DeviceState *dev, bool aux)
             blk_unref(blk);
         }
     }
-    qdev_reset_all(DEVICE(dev));
+    qdev_reset_all(dev);
+
     return 0;
 }
 
-- 
2.21.0

[PATCH 08/12] hw/ide/piix: Use ARRAY_SIZE() instead of magic numbers

[Philippe Mathieu-Daudé]

Using magic numbers is dangerous because the structures PCIIDEState
might be modified and this source file consuming the "ide/pci.h"
header would be out of sync, eventually accessing out of bound
array members.
Use the ARRAY_SIZE() to keep the source file sync.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/ide/piix.c | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index ffeff4e095..ab23613a44 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -87,10 +87,9 @@ static const MemoryRegionOps piix_bmdma_ops = {
 
 static void bmdma_setup_bar(PCIIDEState *d)
 {
-    int i;
-
     memory_region_init(&d->bmdma_bar, OBJECT(d), "piix-bmdma-container", 16);
-    for(i = 0;i < 2; i++) {
+
+    for (size_t i = 0; i < ARRAY_SIZE(d->bmdma); i++) {
         BMDMAState *bm = &d->bmdma[i];
 
         memory_region_init_io(&bm->extra_io, OBJECT(d), &piix_bmdma_ops, bm,
@@ -107,9 +106,8 @@ static void piix_ide_reset(DeviceState *dev)
     PCIIDEState *d = PCI_IDE(dev);
     PCIDevice *pd = PCI_DEVICE(d);
     uint8_t *pci_conf = pd->config;
-    int i;
 
-    for (i = 0; i < 2; i++) {
+    for (size_t i = 0; i < ARRAY_SIZE(d->bus); i++) {
         ide_bus_reset(&d->bus[i]);
     }
 
@@ -132,10 +130,10 @@ static void pci_piix_init_ports(PCIIDEState *d) {
         {0x1f0, 0x3f6, 14},
         {0x170, 0x376, 15},
     };
-    int i;
 
-    for (i = 0; i < 2; i++) {
-        ide_bus_new(&d->bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
+    for (size_t i = 0; i < ARRAY_SIZE(d->bus); i++) {
+        ide_bus_new(&d->bus[i], sizeof(d->bus[i]), DEVICE(d), i,
+                    ARRAY_SIZE(d->bus[0].ifs));
         ide_init_ioport(&d->bus[i], NULL, port_info[i].iobase,
                         port_info[i].iobase2);
         ide_init2(&d->bus[i], isa_get_irq(NULL, port_info[i].isairq));
@@ -163,14 +161,13 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp)
 
 int pci_piix3_xen_ide_unplug(DeviceState *dev, bool aux)
 {
-    PCIIDEState *pci_ide;
+    PCIIDEState *pci_ide = PCI_IDE(dev);
     DriveInfo *di;
-    int i;
     IDEDevice *idedev;
+    const size_t idedev_max = ARRAY_SIZE(pci_ide->bus)
+                            * ARRAY_SIZE(pci_ide->bus[0].ifs);
 
-    pci_ide = PCI_IDE(dev);
-
-    for (i = aux ? 1 : 0; i < 4; i++) {
+    for (size_t i = aux ? 1 : 0; i < idedev_max; i++) {
         di = drive_get_by_index(IF_IDE, i);
         if (di != NULL && !di->media_cd) {
             BlockBackend *blk = blk_by_legacy_dinfo(di);
@@ -210,9 +207,8 @@ PCIDevice *pci_piix3_xen_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
 static void pci_piix_ide_exitfn(PCIDevice *dev)
 {
     PCIIDEState *d = PCI_IDE(dev);
-    unsigned i;
 
-    for (i = 0; i < 2; ++i) {
+    for (size_t i = 0; i < ARRAY_SIZE(d->bmdma); ++i) {
         memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io);
         memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport);
     }
-- 
2.21.0

[PATCH 09/12] hw/intc/ioapic: Make ioapic_print_redtbl() static

[Philippe Mathieu-Daudé]

Since commit 0c8465440 the ioapic_print_redtbl() function is not
used outside of ioapic_common.c, make it static, and remove its
prototype declaration in "ioapic_internal.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/hw/i386/ioapic_internal.h | 1 -
 hw/intc/ioapic_common.c           | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/hw/i386/ioapic_internal.h b/include/hw/i386/ioapic_internal.h
index d46c87c510..8b743aeed0 100644
--- a/include/hw/i386/ioapic_internal.h
+++ b/include/hw/i386/ioapic_internal.h
@@ -118,7 +118,6 @@ struct IOAPICCommonState {
 
 void ioapic_reset_common(DeviceState *dev);
 
-void ioapic_print_redtbl(Monitor *mon, IOAPICCommonState *s);
 void ioapic_stat_update_irq(IOAPICCommonState *s, int irq, int level);
 
 #endif /* QEMU_IOAPIC_INTERNAL_H */
diff --git a/hw/intc/ioapic_common.c b/hw/intc/ioapic_common.c
index 5538b5b86e..72ea945377 100644
--- a/hw/intc/ioapic_common.c
+++ b/hw/intc/ioapic_common.c
@@ -76,7 +76,7 @@ static void ioapic_irr_dump(Monitor *mon, const char *name, uint32_t bitmap)
     monitor_printf(mon, "\n");
 }
 
-void ioapic_print_redtbl(Monitor *mon, IOAPICCommonState *s)
+static void ioapic_print_redtbl(Monitor *mon, IOAPICCommonState *s)
 {
     static const char *delm_str[] = {
         "fixed", "lowest", "SMI", "...", "NMI", "INIT", "...", "extINT"};
-- 
2.21.0

[PATCH 10/12] hw/i386/pc: Rename allocate_cpu_irq from 'pc' to 'x86_machine'

[Philippe Mathieu-Daudé]

All the X86 machines use an interrupt controller.
Rename the function to something more generic.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/hw/i386/pc.h | 2 +-
 hw/i386/microvm.c    | 2 +-
 hw/i386/pc.c         | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 743141e107..244dbf2ec0 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -198,7 +198,7 @@ void pc_memory_init(PCMachineState *pcms,
                     MemoryRegion *rom_memory,
                     MemoryRegion **ram_memory);
 uint64_t pc_pci_hole64_start(void);
-qemu_irq pc_allocate_cpu_irq(void);
+qemu_irq x86_machine_allocate_cpu_irq(void);
 DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus);
 void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
                           ISADevice **rtc_state,
diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index def37e60f7..7ac008460a 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -132,7 +132,7 @@ static void microvm_devices_init(MicrovmMachineState *mms)
     if (mms->pic == ON_OFF_AUTO_ON || mms->pic == ON_OFF_AUTO_AUTO) {
         qemu_irq *i8259;
 
-        i8259 = i8259_init(isa_bus, pc_allocate_cpu_irq());
+        i8259 = i8259_init(isa_bus, x86_machine_allocate_cpu_irq());
         for (i = 0; i < ISA_NUM_IRQS; i++) {
             gsi_state->i8259_irq[i] = i8259[i];
         }
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 5f8e39c025..4defee274f 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1282,7 +1282,7 @@ uint64_t pc_pci_hole64_start(void)
     return ROUND_UP(hole64_start, 1 * GiB);
 }
 
-qemu_irq pc_allocate_cpu_irq(void)
+qemu_irq x86_machine_allocate_cpu_irq(void)
 {
     return qemu_allocate_irq(pic_irq_request, NULL, 0);
 }
@@ -1463,7 +1463,7 @@ void pc_i8259_create(ISABus *isa_bus, qemu_irq *i8259_irqs)
     } else if (xen_enabled()) {
         i8259 = xen_interrupt_controller_init();
     } else {
-        i8259 = i8259_init(isa_bus, pc_allocate_cpu_irq());
+        i8259 = i8259_init(isa_bus, x86_machine_allocate_cpu_irq());
     }
 
     for (size_t i = 0; i < ISA_NUM_IRQS; i++) {
-- 
2.21.0

[PATCH 11/12] hw/i386/pc: Move x86_machine_allocate_cpu_irq() to 'hw/i386/x86.c'

[Philippe Mathieu-Daudé]

Keep 'pc.c' for PC-machine specific code, and use 'x86.c' for code
used by all the X86-based machines.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/hw/i386/pc.h  |  1 -
 include/hw/i386/x86.h |  2 ++
 hw/i386/pc.c          | 27 ---------------------------
 hw/i386/x86.c         | 30 ++++++++++++++++++++++++++++++
 4 files changed, 32 insertions(+), 28 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 244dbf2ec0..2ef6e2cfff 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -198,7 +198,6 @@ void pc_memory_init(PCMachineState *pcms,
                     MemoryRegion *rom_memory,
                     MemoryRegion **ram_memory);
 uint64_t pc_pci_hole64_start(void);
-qemu_irq x86_machine_allocate_cpu_irq(void);
 DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus);
 void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
                           ISADevice **rtc_state,
diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
index 4b84917885..4c3dd6f33e 100644
--- a/include/hw/i386/x86.h
+++ b/include/hw/i386/x86.h
@@ -95,4 +95,6 @@ void x86_load_linux(X86MachineState *x86ms,
                     bool pvh_enabled,
                     bool linuxboot_dma_enabled);
 
+qemu_irq x86_machine_allocate_cpu_irq(void);
+
 #endif
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 4defee274f..4c018735b0 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -412,28 +412,6 @@ int cpu_get_pic_interrupt(CPUX86State *env)
     return intno;
 }
 
-static void pic_irq_request(void *opaque, int irq, int level)
-{
-    CPUState *cs = first_cpu;
-    X86CPU *cpu = X86_CPU(cs);
-
-    trace_x86_pic_interrupt(irq, level);
-    if (cpu->apic_state && !kvm_irqchip_in_kernel()) {
-        CPU_FOREACH(cs) {
-            cpu = X86_CPU(cs);
-            if (apic_accept_pic_intr(cpu->apic_state)) {
-                apic_deliver_pic_intr(cpu->apic_state, level);
-            }
-        }
-    } else {
-        if (level) {
-            cpu_interrupt(cs, CPU_INTERRUPT_HARD);
-        } else {
-            cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
-        }
-    }
-}
-
 /* PC cmos mappings */
 
 #define REG_EQUIPMENT_BYTE          0x14
@@ -1282,11 +1260,6 @@ uint64_t pc_pci_hole64_start(void)
     return ROUND_UP(hole64_start, 1 * GiB);
 }
 
-qemu_irq x86_machine_allocate_cpu_irq(void)
-{
-    return qemu_allocate_irq(pic_irq_request, NULL, 0);
-}
-
 DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus)
 {
     DeviceState *dev = NULL;
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 394edc2f72..a6a394ca36 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -34,6 +34,7 @@
 #include "sysemu/numa.h"
 #include "sysemu/replay.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/kvm.h"
 
 #include "hw/i386/x86.h"
 #include "target/i386/cpu.h"
@@ -41,11 +42,13 @@
 #include "hw/i386/fw_cfg.h"
 
 #include "hw/acpi/cpu_hotplug.h"
+#include "hw/irq.h"
 #include "hw/nmi.h"
 #include "hw/loader.h"
 #include "multiboot.h"
 #include "elf.h"
 #include "standard-headers/asm-x86/bootparam.h"
+#include "trace.h"
 
 #define BIOS_FILENAME "bios.bin"
 
@@ -206,6 +209,33 @@ static void x86_nmi(NMIState *n, int cpu_index, Error **errp)
     }
 }
 
+static void pic_irq_request(void *opaque, int irq, int level)
+{
+    CPUState *cs = first_cpu;
+    X86CPU *cpu = X86_CPU(cs);
+
+    trace_x86_pic_interrupt(irq, level);
+    if (cpu->apic_state && !kvm_irqchip_in_kernel()) {
+        CPU_FOREACH(cs) {
+            cpu = X86_CPU(cs);
+            if (apic_accept_pic_intr(cpu->apic_state)) {
+                apic_deliver_pic_intr(cpu->apic_state, level);
+            }
+        }
+    } else {
+        if (level) {
+            cpu_interrupt(cs, CPU_INTERRUPT_HARD);
+        } else {
+            cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
+        }
+    }
+}
+
+qemu_irq x86_machine_allocate_cpu_irq(void)
+{
+    return qemu_allocate_irq(pic_irq_request, NULL, 0);
+}
+
 static long get_file_size(FILE *f)
 {
     long where, size;
-- 
2.21.0

[PATCH 12/12] hw/i386/pc: Move PC-machine specific declarations to 'pc_internal.h'

[Philippe Mathieu-Daudé]

Historically, QEMU started with only one X86 machine: the PC.
The 'hw/i386/pc.h' header was used to store all X86 and PC
declarations. Since we have now multiple machines based on the
X86 architecture, move the PC-specific declarations in a new
header.
We use 'internal' in the name to explicit this header is restricted
to the X86 architecture. Other architecture can not access it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Maybe name it 'pc_machine.h'?
---
 hw/i386/pc_internal.h | 144 ++++++++++++++++++++++++++++++++++++++++++
 include/hw/i386/pc.h  | 128 -------------------------------------
 hw/i386/acpi-build.c  |   1 +
 hw/i386/pc.c          |   1 +
 hw/i386/pc_piix.c     |   1 +
 hw/i386/pc_q35.c      |   1 +
 hw/i386/pc_sysfw.c    |   1 +
 hw/i386/xen/xen-hvm.c |   1 +
 8 files changed, 150 insertions(+), 128 deletions(-)
 create mode 100644 hw/i386/pc_internal.h

diff --git a/hw/i386/pc_internal.h b/hw/i386/pc_internal.h
new file mode 100644
index 0000000000..c9be64e815
--- /dev/null
+++ b/hw/i386/pc_internal.h
@@ -0,0 +1,144 @@
+#ifndef HW_I386_PC_INTERNAL_H
+#define HW_I386_PC_INTERNAL_H
+
+#include "hw/hw.h"
+#include "hw/isa/isa.h"
+#include "hw/i386/pc.h"
+#include "hw/block/fdc.h"
+#include "net/net.h"
+
+#define PC_MACHINE_ACPI_DEVICE_PROP     "acpi-device"
+#define PC_MACHINE_DEVMEM_REGION_SIZE   "device-memory-region-size"
+#define PC_MACHINE_MAX_RAM_BELOW_4G     "max-ram-below-4g"
+#define PC_MACHINE_VMPORT               "vmport"
+#define PC_MACHINE_SMM                  "smm"
+#define PC_MACHINE_SMBUS                "smbus"
+#define PC_MACHINE_SATA                 "sata"
+#define PC_MACHINE_PIT                  "pit"
+
+void pc_register_ferr_irq(qemu_irq irq);
+void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
+
+void pc_hot_add_cpu(MachineState *ms, const int64_t id, Error **errp);
+void pc_smp_parse(MachineState *ms, QemuOpts *opts);
+
+void pc_guest_info_init(PCMachineState *pcms);
+
+void xen_load_linux(PCMachineState *pcms);
+void pc_memory_init(PCMachineState *pcms,
+                    MemoryRegion *system_memory,
+                    MemoryRegion *rom_memory,
+                    MemoryRegion **ram_memory);
+
+void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
+                          ISADevice **rtc_state,
+                          bool create_fdctrl,
+                          bool no_vmport,
+                          bool has_pit,
+                          uint32_t hpet_irqs);
+void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd);
+void pc_cmos_init(PCMachineState *pcms,
+                  BusState *ide0, BusState *ide1,
+                  ISADevice *s);
+void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus);
+
+ISADevice *pc_find_fdc0(void);
+int cmos_get_fd_drive_type(FloppyDriveType fd0);
+
+#define FW_CFG_IO_BASE     0x510
+
+/* pc_sysfw.c */
+void pc_system_flash_create(PCMachineState *pcms);
+void pc_system_firmware_init(PCMachineState *pcms, MemoryRegion *rom_memory);
+
+extern GlobalProperty pc_compat_4_1[];
+extern const size_t pc_compat_4_1_len;
+
+extern GlobalProperty pc_compat_4_0[];
+extern const size_t pc_compat_4_0_len;
+
+extern GlobalProperty pc_compat_3_1[];
+extern const size_t pc_compat_3_1_len;
+
+extern GlobalProperty pc_compat_3_0[];
+extern const size_t pc_compat_3_0_len;
+
+extern GlobalProperty pc_compat_2_12[];
+extern const size_t pc_compat_2_12_len;
+
+extern GlobalProperty pc_compat_2_11[];
+extern const size_t pc_compat_2_11_len;
+
+extern GlobalProperty pc_compat_2_10[];
+extern const size_t pc_compat_2_10_len;
+
+extern GlobalProperty pc_compat_2_9[];
+extern const size_t pc_compat_2_9_len;
+
+extern GlobalProperty pc_compat_2_8[];
+extern const size_t pc_compat_2_8_len;
+
+extern GlobalProperty pc_compat_2_7[];
+extern const size_t pc_compat_2_7_len;
+
+extern GlobalProperty pc_compat_2_6[];
+extern const size_t pc_compat_2_6_len;
+
+extern GlobalProperty pc_compat_2_5[];
+extern const size_t pc_compat_2_5_len;
+
+extern GlobalProperty pc_compat_2_4[];
+extern const size_t pc_compat_2_4_len;
+
+extern GlobalProperty pc_compat_2_3[];
+extern const size_t pc_compat_2_3_len;
+
+extern GlobalProperty pc_compat_2_2[];
+extern const size_t pc_compat_2_2_len;
+
+extern GlobalProperty pc_compat_2_1[];
+extern const size_t pc_compat_2_1_len;
+
+extern GlobalProperty pc_compat_2_0[];
+extern const size_t pc_compat_2_0_len;
+
+extern GlobalProperty pc_compat_1_7[];
+extern const size_t pc_compat_1_7_len;
+
+extern GlobalProperty pc_compat_1_6[];
+extern const size_t pc_compat_1_6_len;
+
+extern GlobalProperty pc_compat_1_5[];
+extern const size_t pc_compat_1_5_len;
+
+extern GlobalProperty pc_compat_1_4[];
+extern const size_t pc_compat_1_4_len;
+
+/*
+ * Helper for setting model-id for CPU models that changed model-id
+ * depending on QEMU versions up to QEMU 2.4.
+ */
+#define PC_CPU_MODEL_IDS(v) \
+    { "qemu32-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },\
+    { "qemu64-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },\
+    { "athlon-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },
+
+#define DEFINE_PC_MACHINE(suffix, namestr, initfn, optsfn) \
+    static void pc_machine_##suffix##_class_init(ObjectClass *oc, void *data) \
+    { \
+        MachineClass *mc = MACHINE_CLASS(oc); \
+        optsfn(mc); \
+        mc->init = initfn; \
+    } \
+    static const TypeInfo pc_machine_type_##suffix = { \
+        .name       = namestr TYPE_MACHINE_SUFFIX, \
+        .parent     = TYPE_PC_MACHINE, \
+        .class_init = pc_machine_##suffix##_class_init, \
+    }; \
+    static void pc_machine_init_##suffix(void) \
+    { \
+        type_register(&pc_machine_type_##suffix); \
+    } \
+    type_init(pc_machine_init_##suffix)
+
+#endif
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 2ef6e2cfff..9a5633a394 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -4,9 +4,7 @@
 #include "exec/memory.h"
 #include "hw/boards.h"
 #include "hw/isa/isa.h"
-#include "hw/block/fdc.h"
 #include "hw/block/flash.h"
-#include "net/net.h"
 #include "hw/i386/ioapic.h"
 #include "hw/i386/x86.h"
 
@@ -58,14 +56,6 @@ struct PCMachineState {
     hwaddr memhp_io_base;
 };
 
-#define PC_MACHINE_ACPI_DEVICE_PROP "acpi-device"
-#define PC_MACHINE_DEVMEM_REGION_SIZE "device-memory-region-size"
-#define PC_MACHINE_VMPORT           "vmport"
-#define PC_MACHINE_SMM              "smm"
-#define PC_MACHINE_SMBUS            "smbus"
-#define PC_MACHINE_SATA             "sata"
-#define PC_MACHINE_PIT              "pit"
-
 /**
  * PCMachineClass:
  *
@@ -173,12 +163,6 @@ void vmmouse_set_data(const uint32_t *data);
 extern int fd_bootchk;
 
 bool pc_machine_is_smm_enabled(PCMachineState *pcms);
-void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
-
-void pc_hot_add_cpu(MachineState *ms, const int64_t id, Error **errp);
-void pc_smp_parse(MachineState *ms, QemuOpts *opts);
-
-void pc_guest_info_init(PCMachineState *pcms);
 
 #define PCI_HOST_PROP_PCI_HOLE_START   "pci-hole-start"
 #define PCI_HOST_PROP_PCI_HOLE_END     "pci-hole-end"
@@ -192,31 +176,12 @@ void pc_guest_info_init(PCMachineState *pcms);
 void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
                             MemoryRegion *pci_address_space);
 
-void xen_load_linux(PCMachineState *pcms);
-void pc_memory_init(PCMachineState *pcms,
-                    MemoryRegion *system_memory,
-                    MemoryRegion *rom_memory,
-                    MemoryRegion **ram_memory);
 uint64_t pc_pci_hole64_start(void);
 DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus);
-void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
-                          ISADevice **rtc_state,
-                          bool create_fdctrl,
-                          bool no_vmport,
-                          bool has_pit,
-                          uint32_t hpet_irqs);
-void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd);
-void pc_cmos_init(PCMachineState *pcms,
-                  BusState *ide0, BusState *ide1,
-                  ISADevice *s);
-void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus);
 
 void pc_i8259_create(ISABus *isa_bus, qemu_irq *i8259_irqs);
 void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name);
 
-ISADevice *pc_find_fdc0(void);
-int cmos_get_fd_drive_type(FloppyDriveType fd0);
-
 #define FW_CFG_IO_BASE     0x510
 
 #define PORT92_A20_LINE "a20"
@@ -224,102 +189,9 @@ int cmos_get_fd_drive_type(FloppyDriveType fd0);
 /* hpet.c */
 extern int no_hpet;
 
-/* pc_sysfw.c */
-void pc_system_flash_create(PCMachineState *pcms);
-void pc_system_firmware_init(PCMachineState *pcms, MemoryRegion *rom_memory);
-
 /* acpi-build.c */
 void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
                        const CPUArchIdList *apic_ids, GArray *entry);
 
-extern GlobalProperty pc_compat_4_1[];
-extern const size_t pc_compat_4_1_len;
-
-extern GlobalProperty pc_compat_4_0[];
-extern const size_t pc_compat_4_0_len;
-
-extern GlobalProperty pc_compat_3_1[];
-extern const size_t pc_compat_3_1_len;
-
-extern GlobalProperty pc_compat_3_0[];
-extern const size_t pc_compat_3_0_len;
-
-extern GlobalProperty pc_compat_2_12[];
-extern const size_t pc_compat_2_12_len;
-
-extern GlobalProperty pc_compat_2_11[];
-extern const size_t pc_compat_2_11_len;
-
-extern GlobalProperty pc_compat_2_10[];
-extern const size_t pc_compat_2_10_len;
-
-extern GlobalProperty pc_compat_2_9[];
-extern const size_t pc_compat_2_9_len;
-
-extern GlobalProperty pc_compat_2_8[];
-extern const size_t pc_compat_2_8_len;
-
-extern GlobalProperty pc_compat_2_7[];
-extern const size_t pc_compat_2_7_len;
-
-extern GlobalProperty pc_compat_2_6[];
-extern const size_t pc_compat_2_6_len;
-
-extern GlobalProperty pc_compat_2_5[];
-extern const size_t pc_compat_2_5_len;
-
-extern GlobalProperty pc_compat_2_4[];
-extern const size_t pc_compat_2_4_len;
-
-extern GlobalProperty pc_compat_2_3[];
-extern const size_t pc_compat_2_3_len;
-
-extern GlobalProperty pc_compat_2_2[];
-extern const size_t pc_compat_2_2_len;
-
-extern GlobalProperty pc_compat_2_1[];
-extern const size_t pc_compat_2_1_len;
-
-extern GlobalProperty pc_compat_2_0[];
-extern const size_t pc_compat_2_0_len;
-
-extern GlobalProperty pc_compat_1_7[];
-extern const size_t pc_compat_1_7_len;
-
-extern GlobalProperty pc_compat_1_6[];
-extern const size_t pc_compat_1_6_len;
-
-extern GlobalProperty pc_compat_1_5[];
-extern const size_t pc_compat_1_5_len;
-
-extern GlobalProperty pc_compat_1_4[];
-extern const size_t pc_compat_1_4_len;
-
-/* Helper for setting model-id for CPU models that changed model-id
- * depending on QEMU versions up to QEMU 2.4.
- */
-#define PC_CPU_MODEL_IDS(v) \
-    { "qemu32-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },\
-    { "qemu64-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },\
-    { "athlon-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },
-
-#define DEFINE_PC_MACHINE(suffix, namestr, initfn, optsfn) \
-    static void pc_machine_##suffix##_class_init(ObjectClass *oc, void *data) \
-    { \
-        MachineClass *mc = MACHINE_CLASS(oc); \
-        optsfn(mc); \
-        mc->init = initfn; \
-    } \
-    static const TypeInfo pc_machine_type_##suffix = { \
-        .name       = namestr TYPE_MACHINE_SUFFIX, \
-        .parent     = TYPE_PC_MACHINE, \
-        .class_init = pc_machine_##suffix##_class_init, \
-    }; \
-    static void pc_machine_init_##suffix(void) \
-    { \
-        type_register(&pc_machine_type_##suffix); \
-    } \
-    type_init(pc_machine_init_##suffix)
-
 extern void igd_passthrough_isa_bridge_create(PCIBus *bus, uint16_t gpu_dev_id);
 #endif
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 291909fa05..7267e9754f 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -64,6 +64,7 @@
 #include "hw/acpi/pci.h"
 
 #include "qom/qom-qobject.h"
+#include "hw/i386/pc_internal.h"
 #include "hw/i386/amd_iommu.h"
 #include "hw/i386/intel_iommu.h"
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 4c018735b0..df879ff8e5 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -26,6 +26,7 @@
 #include "qemu/units.h"
 #include "hw/i386/x86.h"
 #include "hw/i386/pc.h"
+#include "hw/i386/pc_internal.h"
 #include "hw/char/serial.h"
 #include "hw/char/parallel.h"
 #include "hw/i386/apic.h"
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 1bd70d1abb..a7f67f39a8 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -29,6 +29,7 @@
 #include "hw/loader.h"
 #include "hw/i386/x86.h"
 #include "hw/i386/pc.h"
+#include "hw/i386/pc_internal.h"
 #include "hw/i386/apic.h"
 #include "hw/pci-host/i440fx.h"
 #include "hw/southbridge/piix.h"
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 385e5cffb1..d1d251cb26 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -42,6 +42,7 @@
 #include "exec/address-spaces.h"
 #include "hw/i386/x86.h"
 #include "hw/i386/pc.h"
+#include "hw/i386/pc_internal.h"
 #include "hw/i386/ich9.h"
 #include "hw/i386/amd_iommu.h"
 #include "hw/i386/intel_iommu.h"
diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index f5f3f466b0..6762a6b453 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -33,6 +33,7 @@
 #include "hw/sysbus.h"
 #include "hw/i386/x86.h"
 #include "hw/i386/pc.h"
+#include "hw/i386/pc_internal.h"
 #include "hw/loader.h"
 #include "hw/qdev-properties.h"
 #include "sysemu/sysemu.h"
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 82ece6b9e7..26e6d013d0 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -14,6 +14,7 @@
 #include "hw/pci/pci.h"
 #include "hw/pci/pci_host.h"
 #include "hw/i386/pc.h"
+#include "hw/i386/pc_internal.h"
 #include "hw/southbridge/piix.h"
 #include "hw/irq.h"
 #include "hw/hw.h"
-- 
2.21.0

Re: [PATCH 12/12] hw/i386/pc: Move PC-machine specific declarations to 'pc_internal.h'

[Philippe Mathieu-Daudé]

On 12/13/19 5:17 PM, Philippe Mathieu-Daudé wrote:
> Historically, QEMU started with only one X86 machine: the PC.
> The 'hw/i386/pc.h' header was used to store all X86 and PC
> declarations. Since we have now multiple machines based on the
> X86 architecture, move the PC-specific declarations in a new
> header.
> We use 'internal' in the name to explicit this header is restricted
> to the X86 architecture. Other architecture can not access it.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Maybe name it 'pc_machine.h'?

I forgot to describe here (and in the cover), what's follow after this 
patch.

Patch #13 moves PCMachineClass to

If you ignore PCMachineState, "hw/i386/pc.h" now only contains 76 lines, 
and it is easier to see what is PC machine specific, what is X86 
specific, and what is device generic (not X86 related at all):

- GSI is common to X86 (Paolo sent [3], [6])
- IOAPIC is common to X86
- i8259 is multiarch (Paolo [2])
- PCI_HOST definitions and pc_pci_hole64_start() are X86
- pc_machine_is_smm_enabled() is X86 (Paolo sent [5])
- hpet
- tsc (Paolo sent [3])
- 3 more functions

So we can move half of this file to "pc_internal.h" and the other to

One problem is the Q35 MCH north bridge which directly sets the PCI 
PCMachineState->bus in q35_host_realize(). This seems a QOM violation 
and is probably easily fixable.

Maybe I can apply Paolo's patches instead of this #12, move X86-generic 
declarations to "hw/i386/x86.h", and directly git-move what's left of 
"hw/i386/pc.h" to "pc_internal.h".

[3] https://www.mail-archive.com/qemu-devel@nongnu.org/msg664627.html
[2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg664765.html
[5] https://www.mail-archive.com/qemu-devel@nongnu.org/msg664754.html
[6] https://www.mail-archive.com/qemu-devel@nongnu.org/msg664766.html

> ---
>   hw/i386/pc_internal.h | 144 ++++++++++++++++++++++++++++++++++++++++++
>   include/hw/i386/pc.h  | 128 -------------------------------------
>   hw/i386/acpi-build.c  |   1 +
>   hw/i386/pc.c          |   1 +
>   hw/i386/pc_piix.c     |   1 +
>   hw/i386/pc_q35.c      |   1 +
>   hw/i386/pc_sysfw.c    |   1 +
>   hw/i386/xen/xen-hvm.c |   1 +
>   8 files changed, 150 insertions(+), 128 deletions(-)
>   create mode 100644 hw/i386/pc_internal.h

Re: [PATCH 12/12] hw/i386/pc: Move PC-machine specific declarations to 'pc_internal.h'

[Michael S. Tsirkin]

On Fri, Dec 13, 2019 at 05:47:28PM +0100, Philippe Mathieu-Daudé wrote:
> On 12/13/19 5:17 PM, Philippe Mathieu-Daudé wrote:
> > Historically, QEMU started with only one X86 machine: the PC.
> > The 'hw/i386/pc.h' header was used to store all X86 and PC
> > declarations. Since we have now multiple machines based on the
> > X86 architecture, move the PC-specific declarations in a new
> > header.
> > We use 'internal' in the name to explicit this header is restricted
> > to the X86 architecture. Other architecture can not access it.
> > 
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > ---
> > Maybe name it 'pc_machine.h'?
> 
> I forgot to describe here (and in the cover), what's follow after this
> patch.
> 
> Patch #13 moves PCMachineClass to
> 
> If you ignore PCMachineState, "hw/i386/pc.h" now only contains 76 lines, and
> it is easier to see what is PC machine specific, what is X86 specific, and
> what is device generic (not X86 related at all):
> 
> - GSI is common to X86 (Paolo sent [3], [6])
> - IOAPIC is common to X86
> - i8259 is multiarch (Paolo [2])
> - PCI_HOST definitions and pc_pci_hole64_start() are X86
> - pc_machine_is_smm_enabled() is X86 (Paolo sent [5])
> - hpet
> - tsc (Paolo sent [3])
> - 3 more functions
> 
> So we can move half of this file to "pc_internal.h" and the other to
> 
> One problem is the Q35 MCH north bridge which directly sets the PCI
> PCMachineState->bus in q35_host_realize(). This seems a QOM violation and is
> probably easily fixable.
> 
> Maybe I can apply Paolo's patches instead of this #12, move X86-generic
> declarations to "hw/i386/x86.h", and directly git-move what's left of
> "hw/i386/pc.h" to "pc_internal.h".

Yea that sounds a bit better.

> [3] https://www.mail-archive.com/qemu-devel@nongnu.org/msg664627.html
> [2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg664765.html
> [5] https://www.mail-archive.com/qemu-devel@nongnu.org/msg664754.html
> [6] https://www.mail-archive.com/qemu-devel@nongnu.org/msg664766.html
> 
> > ---
> >   hw/i386/pc_internal.h | 144 ++++++++++++++++++++++++++++++++++++++++++
> >   include/hw/i386/pc.h  | 128 -------------------------------------
> >   hw/i386/acpi-build.c  |   1 +
> >   hw/i386/pc.c          |   1 +
> >   hw/i386/pc_piix.c     |   1 +
> >   hw/i386/pc_q35.c      |   1 +
> >   hw/i386/pc_sysfw.c    |   1 +
> >   hw/i386/xen/xen-hvm.c |   1 +
> >   8 files changed, 150 insertions(+), 128 deletions(-)
> >   create mode 100644 hw/i386/pc_internal.h

Re: [PATCH 03/12] hw/i386/pc: Remove obsolete pc_pci_device_init() declaration

[Paolo Bonzini]

On 13/12/19 17:17, Philippe Mathieu-Daudé wrote:
> In commit 1454509726 we removed the pc_pci_device_init()
> deprecated function and its calls, but we forgot to remove
> its prototype. Do that now.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  include/hw/i386/pc.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 9866dfbd60..bc7d855aaa 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -211,7 +211,6 @@ void pc_cmos_init(PCMachineState *pcms,
>                    BusState *ide0, BusState *ide1,
>                    ISADevice *s);
>  void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus);
> -void pc_pci_device_init(PCIBus *pci_bus);
>  
>  typedef void (*cpu_set_smm_t)(int smm, void *arg);
>  
> 

Queued, thanks.

Paolo

Re: [PATCH 04/12] hw/i386/pc: Remove obsolete cpu_set_smm_t typedef

[Paolo Bonzini]

On 13/12/19 17:17, Philippe Mathieu-Daudé wrote:
> In commit f809c6051 we replaced the use of cpu_set_smm_t callbacks
> by using a Notifier to modify the MemoryRegion. This prototype is
> now not used anymore, we can safely remove it.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  include/hw/i386/pc.h | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index bc7d855aaa..743141e107 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -212,8 +212,6 @@ void pc_cmos_init(PCMachineState *pcms,
>                    ISADevice *s);
>  void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus);
>  
> -typedef void (*cpu_set_smm_t)(int smm, void *arg);
> -
>  void pc_i8259_create(ISABus *isa_bus, qemu_irq *i8259_irqs);
>  void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name);
>  
> 

Queued, thanks.

Paolo

Re: [PATCH 05/12] hw/i386/ich9: Remove unused include

[Paolo Bonzini]

On 13/12/19 17:17, Philippe Mathieu-Daudé wrote:
> The "pcie_host.h" header is used by devices providing a PCI-e bus,
> usually North Bridges. The ICH9 is a South Bridge.
> Since we don't need this header, do not include it.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  include/hw/i386/ich9.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
> index 72e803f6e2..eeb79ac1fe 100644
> --- a/include/hw/i386/ich9.h
> +++ b/include/hw/i386/ich9.h
> @@ -7,7 +7,6 @@
>  #include "hw/isa/apm.h"
>  #include "hw/i386/ioapic.h"
>  #include "hw/pci/pci.h"
> -#include "hw/pci/pcie_host.h"
>  #include "hw/pci/pci_bridge.h"
>  #include "hw/acpi/acpi.h"
>  #include "hw/acpi/ich9.h"
> 

Queued, thanks.

Paolo

Re: [PATCH 06/12] hw/i386/ich9: Move unnecessary "pci_bridge.h" include

[Paolo Bonzini]

On 13/12/19 17:17, Philippe Mathieu-Daudé wrote:
> While the ICH9 chipset is a 'South Bridge', it is not a PCI bridge.
> Nothing in "hw/i386/ich9.h" requires definitions from "pci_bridge.h"
> so move its inclusion where it is required.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  include/hw/i386/ich9.h    | 1 -
>  hw/i386/acpi-build.c      | 1 +
>  hw/pci-bridge/i82801b11.c | 1 +
>  3 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
> index eeb79ac1fe..369bc64671 100644
> --- a/include/hw/i386/ich9.h
> +++ b/include/hw/i386/ich9.h
> @@ -7,7 +7,6 @@
>  #include "hw/isa/apm.h"
>  #include "hw/i386/ioapic.h"
>  #include "hw/pci/pci.h"
> -#include "hw/pci/pci_bridge.h"
>  #include "hw/acpi/acpi.h"
>  #include "hw/acpi/ich9.h"
>  #include "hw/pci/pci_bus.h"
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 12ff55fcfb..291909fa05 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -27,6 +27,7 @@
>  #include "qemu/bitmap.h"
>  #include "qemu/error-report.h"
>  #include "hw/pci/pci.h"
> +#include "hw/pci/pci_bridge.h"
>  #include "hw/core/cpu.h"
>  #include "target/i386/cpu.h"
>  #include "hw/misc/pvpanic.h"
> diff --git a/hw/pci-bridge/i82801b11.c b/hw/pci-bridge/i82801b11.c
> index 2b3907655b..033b3c43c4 100644
> --- a/hw/pci-bridge/i82801b11.c
> +++ b/hw/pci-bridge/i82801b11.c
> @@ -43,6 +43,7 @@
>  
>  #include "qemu/osdep.h"
>  #include "hw/pci/pci.h"
> +#include "hw/pci/pci_bridge.h"
>  #include "migration/vmstate.h"
>  #include "qemu/module.h"
>  #include "hw/i386/ich9.h"
> 

Queued, thanks.

Paolo

Re: [PATCH 07/12] hw/ide/piix: Remove superfluous DEVICE() cast

[Paolo Bonzini]

On 13/12/19 17:17, Philippe Mathieu-Daudé wrote:
> Commit 02a9594b4f0 already converted 'dev' to DeviceState.
> Since the cast is superfluous, remove it.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/ide/piix.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> index db313dd3b1..ffeff4e095 100644
> --- a/hw/ide/piix.c
> +++ b/hw/ide/piix.c
> @@ -193,7 +193,8 @@ int pci_piix3_xen_ide_unplug(DeviceState *dev, bool aux)
>              blk_unref(blk);
>          }
>      }
> -    qdev_reset_all(DEVICE(dev));
> +    qdev_reset_all(dev);
> +
>      return 0;
>  }
>  
> 

Queued, thanks.

Paolo

Re: [PATCH 08/12] hw/ide/piix: Use ARRAY_SIZE() instead of magic numbers

[Paolo Bonzini]

On 13/12/19 17:17, Philippe Mathieu-Daudé wrote:
> Using magic numbers is dangerous because the structures PCIIDEState
> might be modified and this source file consuming the "ide/pci.h"
> header would be out of sync, eventually accessing out of bound
> array members.
> Use the ARRAY_SIZE() to keep the source file sync.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/ide/piix.c | 26 +++++++++++---------------
>  1 file changed, 11 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> index ffeff4e095..ab23613a44 100644
> --- a/hw/ide/piix.c
> +++ b/hw/ide/piix.c
> @@ -87,10 +87,9 @@ static const MemoryRegionOps piix_bmdma_ops = {
>  
>  static void bmdma_setup_bar(PCIIDEState *d)
>  {
> -    int i;
> -
>      memory_region_init(&d->bmdma_bar, OBJECT(d), "piix-bmdma-container", 16);
> -    for(i = 0;i < 2; i++) {
> +
> +    for (size_t i = 0; i < ARRAY_SIZE(d->bmdma); i++) {
>          BMDMAState *bm = &d->bmdma[i];
>  
>          memory_region_init_io(&bm->extra_io, OBJECT(d), &piix_bmdma_ops, bm,
> @@ -107,9 +106,8 @@ static void piix_ide_reset(DeviceState *dev)
>      PCIIDEState *d = PCI_IDE(dev);
>      PCIDevice *pd = PCI_DEVICE(d);
>      uint8_t *pci_conf = pd->config;
> -    int i;
>  
> -    for (i = 0; i < 2; i++) {
> +    for (size_t i = 0; i < ARRAY_SIZE(d->bus); i++) {
>          ide_bus_reset(&d->bus[i]);
>      }
>  
> @@ -132,10 +130,10 @@ static void pci_piix_init_ports(PCIIDEState *d) {
>          {0x1f0, 0x3f6, 14},
>          {0x170, 0x376, 15},
>      };
> -    int i;
>  
> -    for (i = 0; i < 2; i++) {
> -        ide_bus_new(&d->bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
> +    for (size_t i = 0; i < ARRAY_SIZE(d->bus); i++) {
> +        ide_bus_new(&d->bus[i], sizeof(d->bus[i]), DEVICE(d), i,
> +                    ARRAY_SIZE(d->bus[0].ifs));
>          ide_init_ioport(&d->bus[i], NULL, port_info[i].iobase,
>                          port_info[i].iobase2);
>          ide_init2(&d->bus[i], isa_get_irq(NULL, port_info[i].isairq));
> @@ -163,14 +161,13 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp)
>  
>  int pci_piix3_xen_ide_unplug(DeviceState *dev, bool aux)
>  {
> -    PCIIDEState *pci_ide;
> +    PCIIDEState *pci_ide = PCI_IDE(dev);
>      DriveInfo *di;
> -    int i;
>      IDEDevice *idedev;
> +    const size_t idedev_max = ARRAY_SIZE(pci_ide->bus)
> +                            * ARRAY_SIZE(pci_ide->bus[0].ifs);
>  
> -    pci_ide = PCI_IDE(dev);
> -
> -    for (i = aux ? 1 : 0; i < 4; i++) {
> +    for (size_t i = aux ? 1 : 0; i < idedev_max; i++) {
>          di = drive_get_by_index(IF_IDE, i);
>          if (di != NULL && !di->media_cd) {
>              BlockBackend *blk = blk_by_legacy_dinfo(di);
> @@ -210,9 +207,8 @@ PCIDevice *pci_piix3_xen_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
>  static void pci_piix_ide_exitfn(PCIDevice *dev)
>  {
>      PCIIDEState *d = PCI_IDE(dev);
> -    unsigned i;
>  
> -    for (i = 0; i < 2; ++i) {
> +    for (size_t i = 0; i < ARRAY_SIZE(d->bmdma); ++i) {
>          memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io);
>          memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport);
>      }
> 

Queued, thanks.

Paolo

Re: [PATCH 09/12] hw/intc/ioapic: Make ioapic_print_redtbl() static

[Paolo Bonzini]

On 13/12/19 17:17, Philippe Mathieu-Daudé wrote:
> Since commit 0c8465440 the ioapic_print_redtbl() function is not
> used outside of ioapic_common.c, make it static, and remove its
> prototype declaration in "ioapic_internal.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  include/hw/i386/ioapic_internal.h | 1 -
>  hw/intc/ioapic_common.c           | 2 +-
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/include/hw/i386/ioapic_internal.h b/include/hw/i386/ioapic_internal.h
> index d46c87c510..8b743aeed0 100644
> --- a/include/hw/i386/ioapic_internal.h
> +++ b/include/hw/i386/ioapic_internal.h
> @@ -118,7 +118,6 @@ struct IOAPICCommonState {
>  
>  void ioapic_reset_common(DeviceState *dev);
>  
> -void ioapic_print_redtbl(Monitor *mon, IOAPICCommonState *s);
>  void ioapic_stat_update_irq(IOAPICCommonState *s, int irq, int level);
>  
>  #endif /* QEMU_IOAPIC_INTERNAL_H */
> diff --git a/hw/intc/ioapic_common.c b/hw/intc/ioapic_common.c
> index 5538b5b86e..72ea945377 100644
> --- a/hw/intc/ioapic_common.c
> +++ b/hw/intc/ioapic_common.c
> @@ -76,7 +76,7 @@ static void ioapic_irr_dump(Monitor *mon, const char *name, uint32_t bitmap)
>      monitor_printf(mon, "\n");
>  }
>  
> -void ioapic_print_redtbl(Monitor *mon, IOAPICCommonState *s)
> +static void ioapic_print_redtbl(Monitor *mon, IOAPICCommonState *s)
>  {
>      static const char *delm_str[] = {
>          "fixed", "lowest", "SMI", "...", "NMI", "INIT", "...", "extINT"};
> 

Queued, thanks.

Paolo

Re: [PATCH 12/12] hw/i386/pc: Move PC-machine specific declarations to 'pc_internal.h'

[Philippe Mathieu-Daudé]

On 12/15/19 10:58 AM, Michael S. Tsirkin wrote:
> On Fri, Dec 13, 2019 at 05:47:28PM +0100, Philippe Mathieu-Daudé wrote:
>> On 12/13/19 5:17 PM, Philippe Mathieu-Daudé wrote:
>>> Historically, QEMU started with only one X86 machine: the PC.
>>> The 'hw/i386/pc.h' header was used to store all X86 and PC
>>> declarations. Since we have now multiple machines based on the
>>> X86 architecture, move the PC-specific declarations in a new
>>> header.
>>> We use 'internal' in the name to explicit this header is restricted
>>> to the X86 architecture. Other architecture can not access it.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
>>> Maybe name it 'pc_machine.h'?
>>
>> I forgot to describe here (and in the cover), what's follow after this
>> patch.
>>
>> Patch #13 moves PCMachineClass to
>>
>> If you ignore PCMachineState, "hw/i386/pc.h" now only contains 76 lines, and
>> it is easier to see what is PC machine specific, what is X86 specific, and
>> what is device generic (not X86 related at all):
>>
>> - GSI is common to X86 (Paolo sent [3], [6])
>> - IOAPIC is common to X86
>> - i8259 is multiarch (Paolo [2])
>> - PCI_HOST definitions and pc_pci_hole64_start() are X86
>> - pc_machine_is_smm_enabled() is X86 (Paolo sent [5])
>> - hpet
>> - tsc (Paolo sent [3])
>> - 3 more functions
>>
>> So we can move half of this file to "pc_internal.h" and the other to
>>
>> One problem is the Q35 MCH north bridge which directly sets the PCI
>> PCMachineState->bus in q35_host_realize(). This seems a QOM violation and is
>> probably easily fixable.
>>
>> Maybe I can apply Paolo's patches instead of this #12, move X86-generic
>> declarations to "hw/i386/x86.h", and directly git-move what's left of
>> "hw/i386/pc.h" to "pc_internal.h".
> 
> Yea that sounds a bit better.

OK, I'll wait for Paolo's next pull get in, then continue based on that, 
including Paolo's "x86: allow building without PC machine types" series.

(Thanks Paolo for picking most of this series!)

Re: [PATCH 12/12] hw/i386/pc: Move PC-machine specific declarations to 'pc_internal.h'

[Paolo Bonzini]

On 16/12/19 16:37, Philippe Mathieu-Daudé wrote:
> On 12/15/19 10:58 AM, Michael S. Tsirkin wrote:
>> On Fri, Dec 13, 2019 at 05:47:28PM +0100, Philippe Mathieu-Daudé wrote:
>>> On 12/13/19 5:17 PM, Philippe Mathieu-Daudé wrote:
>>>> Historically, QEMU started with only one X86 machine: the PC.
>>>> The 'hw/i386/pc.h' header was used to store all X86 and PC
>>>> declarations. Since we have now multiple machines based on the
>>>> X86 architecture, move the PC-specific declarations in a new
>>>> header.
>>>> We use 'internal' in the name to explicit this header is restricted
>>>> to the X86 architecture. Other architecture can not access it.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>> ---
>>>> Maybe name it 'pc_machine.h'?
>>>
>>> I forgot to describe here (and in the cover), what's follow after this
>>> patch.
>>>
>>> Patch #13 moves PCMachineClass to
>>>
>>> If you ignore PCMachineState, "hw/i386/pc.h" now only contains 76
>>> lines, and
>>> it is easier to see what is PC machine specific, what is X86
>>> specific, and
>>> what is device generic (not X86 related at all):
>>>
>>> - GSI is common to X86 (Paolo sent [3], [6])
>>> - IOAPIC is common to X86
>>> - i8259 is multiarch (Paolo [2])
>>> - PCI_HOST definitions and pc_pci_hole64_start() are X86
>>> - pc_machine_is_smm_enabled() is X86 (Paolo sent [5])
>>> - hpet
>>> - tsc (Paolo sent [3])
>>> - 3 more functions
>>>
>>> So we can move half of this file to "pc_internal.h" and the other to
>>>
>>> One problem is the Q35 MCH north bridge which directly sets the PCI
>>> PCMachineState->bus in q35_host_realize(). This seems a QOM violation
>>> and is
>>> probably easily fixable.
>>>
>>> Maybe I can apply Paolo's patches instead of this #12, move X86-generic
>>> declarations to "hw/i386/x86.h", and directly git-move what's left of
>>> "hw/i386/pc.h" to "pc_internal.h".
>>
>> Yea that sounds a bit better.
> 
> OK, I'll wait for Paolo's next pull get in, then continue based on that,
> including Paolo's "x86: allow building without PC machine types" series.
> 
> (Thanks Paolo for picking most of this series!)

FWIW I don't think kvm_i8259_init should be in sysemu/kvm.h, since it's
x86-specific and that would be something like the same mistake already
done with hw/i386/pc.h.

Paolo

Re: [PATCH 12/12] hw/i386/pc: Move PC-machine specific declarations to 'pc_internal.h'

[Philippe Mathieu-Daudé]

On 12/16/19 4:41 PM, Paolo Bonzini wrote:
> On 16/12/19 16:37, Philippe Mathieu-Daudé wrote:
>> On 12/15/19 10:58 AM, Michael S. Tsirkin wrote:
>>> On Fri, Dec 13, 2019 at 05:47:28PM +0100, Philippe Mathieu-Daudé wrote:
>>>> On 12/13/19 5:17 PM, Philippe Mathieu-Daudé wrote:
>>>>> Historically, QEMU started with only one X86 machine: the PC.
>>>>> The 'hw/i386/pc.h' header was used to store all X86 and PC
>>>>> declarations. Since we have now multiple machines based on the
>>>>> X86 architecture, move the PC-specific declarations in a new
>>>>> header.
>>>>> We use 'internal' in the name to explicit this header is restricted
>>>>> to the X86 architecture. Other architecture can not access it.
>>>>>
>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>> ---
>>>>> Maybe name it 'pc_machine.h'?
>>>>
>>>> I forgot to describe here (and in the cover), what's follow after this
>>>> patch.
>>>>
>>>> Patch #13 moves PCMachineClass to
>>>>
>>>> If you ignore PCMachineState, "hw/i386/pc.h" now only contains 76
>>>> lines, and
>>>> it is easier to see what is PC machine specific, what is X86
>>>> specific, and
>>>> what is device generic (not X86 related at all):
>>>>
>>>> - GSI is common to X86 (Paolo sent [3], [6])
>>>> - IOAPIC is common to X86
>>>> - i8259 is multiarch (Paolo [2])
>>>> - PCI_HOST definitions and pc_pci_hole64_start() are X86
>>>> - pc_machine_is_smm_enabled() is X86 (Paolo sent [5])
>>>> - hpet
>>>> - tsc (Paolo sent [3])
>>>> - 3 more functions
>>>>
>>>> So we can move half of this file to "pc_internal.h" and the other to
>>>>
>>>> One problem is the Q35 MCH north bridge which directly sets the PCI
>>>> PCMachineState->bus in q35_host_realize(). This seems a QOM violation
>>>> and is
>>>> probably easily fixable.
>>>>
>>>> Maybe I can apply Paolo's patches instead of this #12, move X86-generic
>>>> declarations to "hw/i386/x86.h", and directly git-move what's left of
>>>> "hw/i386/pc.h" to "pc_internal.h".
>>>
>>> Yea that sounds a bit better.
>>
>> OK, I'll wait for Paolo's next pull get in, then continue based on that,
>> including Paolo's "x86: allow building without PC machine types" series.
>>
>> (Thanks Paolo for picking most of this series!)
> 
> FWIW I don't think kvm_i8259_init should be in sysemu/kvm.h, since it's
> x86-specific and that would be something like the same mistake already
> done with hw/i386/pc.h.

Hmm OK.
So to follow your reasoning, 
kvm_pc_gsi_handler/kvm_pc_setup_irq_routing() are x86-specific and could 
be moved out.
I'll figure that out when I rework the last patches.