hw/i386/pc: Move PC-machine specific declarations to 'pc_internal.h'
cover hw/i386/pc: Move PC-machine specific declarations to 'pc_internal.h'
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/[email protected]/msg664760.html
[2] https://www.mail-archive.com/[email protected]/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
Signature · 1 line
2.21.0
PATCH 1/12 hw/i386/pc: Convert DPRINTF() to trace events
Convert the deprecated DPRINTF() macro to trace events.
---
v2: rename pc_pic -> x86_pic
--- hw/i386/pc.c | 19 +++++-------------- hw/i386/trace-events | 6 ++++++ 2 files changed, 11 insertions(+), 14 deletions(-)
hw/i386/pc.c +5 -14
@@ -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; }
hw/i386/trace-events +6 -0
@@ -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"
Signature · 1 line
2.21.0
PATCH 2/12 hw/i386/pc: Move kvm_i8259_init() declaration to sysemu/kvm.h
In reply to Philippe Mathieu-DaudéMove the KVM-related call to "sysemu/kvm.h".
--- include/hw/i386/pc.h | 1 - include/sysemu/kvm.h | 1 + 2 files changed, 1 insertion(+), 1 deletion(-)
include/hw/i386/pc.h +0 -1
@@ -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);
include/sysemu/kvm.h +1 -0
@@ -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:
Signature · 1 line
2.21.0
PATCH 3/12 hw/i386/pc: Remove obsolete pc_pci_device_init() declaration
In reply to 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.
--- include/hw/i386/pc.h | 1 - 1 file changed, 1 deletion(-)
include/hw/i386/pc.h +0 -1
@@ -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);
Signature · 1 line
2.21.0
Re: [PATCH 03/12] hw/i386/pc: Remove obsolete pc_pci_device_init() declaration
Quoted 15 lines · 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é <[email protected]>--- include/hw/i386/pc.h | 1 - 1 file changed, 1 deletion(-)include/hw/i386/pc.h +0
-0
include/hw/i386/pc.h void pc_cmos_init(PCMachineState *pcms, · line 216
Quoted 7 lines
include/hw/i386/pc.h +0
-1@@ -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
PATCH 4/12 hw/i386/pc: Remove obsolete cpu_set_smm_t typedef
In reply to 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.
--- include/hw/i386/pc.h | 2 -- 1 file changed, 2 deletions(-)
include/hw/i386/pc.h +0 -2
@@ -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);
Signature · 1 line
2.21.0
Re: [PATCH 04/12] hw/i386/pc: Remove obsolete cpu_set_smm_t typedef
Quoted 15 lines · Philippe Mathieu-Daudé
In commit
f809c6051we replaced the use ofcpu_set_smm_tcallbacks 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é <[email protected]>--- include/hw/i386/pc.h | 2 -- 1 file changed, 2 deletions(-)include/hw/i386/pc.h +0
-0
include/hw/i386/pc.h void pc_cmos_init(PCMachineState *pcms, · line 217
Quoted 8 lines
include/hw/i386/pc.h +0
-2@@ -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
PATCH 5/12 hw/i386/ich9: Remove unused include
In reply to 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.
--- include/hw/i386/ich9.h | 1 - 1 file changed, 1 deletion(-)
include/hw/i386/ich9.h +0 -1
@@ -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"
Signature · 1 line
2.21.0
Re: [PATCH 05/12] hw/i386/ich9: Remove unused include
Quoted 15 lines · 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é <[email protected]>--- include/hw/i386/ich9.h | 1 - 1 file changed, 1 deletion(-)include/hw/i386/ich9.h +0
-0
include/hw/i386/ich9.h line 12
Quoted 7 lines
include/hw/i386/ich9.h +0
-1@@ -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
PATCH 6/12 hw/i386/ich9: Move unnecessary "pci_bridge.h" include
In reply to 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.
--- 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(-)
include/hw/i386/ich9.h +0 -1
@@ -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"
hw/i386/acpi-build.c +1 -0
@@ -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"
hw/pci-bridge/i82801b11.c +1 -0
@@ -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"
Signature · 1 line
2.21.0
Re: [PATCH 06/12] hw/i386/ich9: Move unnecessary "pci_bridge.h" include
Quoted 41 lines · 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é <[email protected]>--- 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(-)include/hw/i386/ich9.h +0
-1@@ -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" hw/i386/acpi-build.c +1
-0@@ -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" hw/pci-bridge/i82801b11.c +0
-0
hw/pci-bridge/i82801b11.c line 49
Quoted 7 lines
hw/pci-bridge/i82801b11.c +1
-0@@ -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
PATCH 7/12 hw/ide/piix: Remove superfluous DEVICE() cast
In reply to Philippe Mathieu-DaudéCommit 02a9594b4f0 already converted 'dev' to DeviceState. Since the cast is superfluous, remove it.
--- hw/ide/piix.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
hw/ide/piix.c +2 -1
@@ -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 ; }
Signature · 1 line
2.21.0
Re: [PATCH 07/12] hw/ide/piix: Remove superfluous DEVICE() cast
Quoted 14 lines · Philippe Mathieu-Daudé
Commit
02a9594b4f0already converted 'dev' to DeviceState. Since the cast is superfluous, remove it.Signed-off-by Philippe Mathieu-Daudé <[email protected]>--- hw/ide/piix.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)hw/ide/piix.c +0
-0
hw/ide/piix.c int pci_piix3_xen_ide_unplug(DeviceState *dev, bool aux) · line 200
Quoted 9 lines
hw/ide/piix.c +2
-1@@ -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
PATCH 8/12 hw/ide/piix: Use ARRAY_SIZE() instead of magic numbers
In reply to 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.
--- hw/ide/piix.c | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-)
hw/ide/piix.c +11 -15
@@ -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); }
Signature · 1 line
2.21.0
Re: [PATCH 08/12] hw/ide/piix: Use ARRAY_SIZE() instead of magic numbers
Quoted 74 lines · 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 theARRAY_SIZE()to keep the source file sync.Signed-off-by Philippe Mathieu-Daudé <[email protected]>--- hw/ide/piix.c | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-)hw/ide/piix.c +10
-13@@ -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);
hw/ide/piix.c PCIDevice *pci_piix3_xen_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn) · line 214
Quoted 10 lines
hw/ide/piix.c +1
-2@@ -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
PATCH 9/12 hw/intc/ioapic: Make ioapic_print_redtbl() static
In reply to 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".
--- include/hw/i386/ioapic_internal.h | 1 - hw/intc/ioapic_common.c | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-)
include/hw/i386/ioapic_internal.h +0 -1
@@ -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 */
hw/intc/ioapic_common.c +1 -1
@@ -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" };
Signature · 1 line
2.21.0
Re: [PATCH 09/12] hw/intc/ioapic: Make ioapic_print_redtbl() static
Quoted 28 lines · Philippe Mathieu-Daudé
Since commit
0c8465440theioapic_print_redtbl()function is not used outside ofioapic_common.c, make it static, and remove its prototype declaration in "ioapic_internal.h".Signed-off-by Philippe Mathieu-Daudé <[email protected]>--- include/hw/i386/ioapic_internal.h | 1 - hw/intc/ioapic_common.c | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-)include/hw/i386/ioapic_internal.h +0
-1@@ -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 */ hw/intc/ioapic_common.c +0
-0
hw/intc/ioapic_common.c static void ioapic_irr_dump(Monitor *mon, const char *name, uint32_t bitmap) · line 82
Quoted 8 lines
hw/intc/ioapic_common.c +1
-1@@ -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
PATCH 10/12 hw/i386/pc: Rename allocate_cpu_irq from 'pc' to 'x86_machine'
In reply to Philippe Mathieu-DaudéAll the X86 machines use an interrupt controller.
Rename the function to something more generic.
--- include/hw/i386/pc.h | 2 +- hw/i386/microvm.c | 2 +- hw/i386/pc.c | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-)
include/hw/i386/pc.h +1 -1
@@ -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,
hw/i386/microvm.c +1 -1
@@ -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]; }
hw/i386/pc.c +2 -2
@@ -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++) {
Signature · 1 line
2.21.0
PATCH 11/12 hw/i386/pc: Move x86_machine_allocate_cpu_irq() to 'hw/i386/x86.c'
In reply to Philippe Mathieu-DaudéKeep 'pc.c' for PC-machine specific code, and use 'x86.c' for code used by all the X86-based machines.
--- 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(-)
include/hw/i386/pc.h +0 -1
@@ -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,
include/hw/i386/x86.h +2 -0
@@ -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
hw/i386/pc.c +0 -27
@@ -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 ;
hw/i386/x86.c +30 -0
@@ -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;
Signature · 1 line
2.21.0
PATCH 12/12 hw/i386/pc: Move PC-machine specific declarations to 'pc_internal.h'
In reply to 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.
---
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
hw/i386/pc_internal.h new file +144 -0
@@ -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
include/hw/i386/pc.h +0 -128
168 lines not on this page · raw message
hw/i386/acpi-build.c +1 -0
@@ -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"
hw/i386/pc.c +1 -0
@@ -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"
hw/i386/pc_piix.c +1 -0
@@ -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"
hw/i386/pc_q35.c +1 -0
@@ -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"
hw/i386/pc_sysfw.c +1 -0
@@ -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"
hw/i386/xen/xen-hvm.c +1 -0
@@ -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"
Signature · 1 line
2.21.0
Quoted 11 lines · 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é <[email protected]>---
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/[email protected]/msg664627.html
[2] https://www.mail-archive.com/[email protected]/msg664765.html
[5] https://www.mail-archive.com/[email protected]/msg664754.html
[6] https://www.mail-archive.com/[email protected]/msg664766.html
Quoted 11 lines
--- 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
Quoted 33 lines · Philippe Mathieu-Daudé
Quoted 11 lines · 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é <[email protected]>---
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_HOSTdefinitions andpc_pci_hole64_start()are X86
-pc_machine_is_smm_enabled()is X86 (Paolo sent [5])
- hpet
- tsc (Paolo sent [3])
- 3 more functionsSo we can move half of this file to "
pc_internal.h" and the other to
Quoted 7 lines
One problem is the Q35 MCH north bridge which directly sets the PCI
PCMachineState->businq35_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.
Quoted 16 lines
[3] https://www.mail-archive.com/[email protected]/msg664627.html
[2] https://www.mail-archive.com/[email protected]/msg664765.html
[5] https://www.mail-archive.com/[email protected]/msg664754.html
[6] https://www.mail-archive.com/[email protected]/msg664766.htmlQuoted 11 lines
--- 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
Quoted 38 lines · Michael S. Tsirkin
Quoted 37 lines · Philippe Mathieu-Daudé
Quoted 11 lines · 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é <[email protected]>---
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_HOSTdefinitions andpc_pci_hole64_start()are X86
-pc_machine_is_smm_enabled()is X86 (Paolo sent [5])
- hpet
- tsc (Paolo sent [3])
- 3 more functionsSo we can move half of this file to "
pc_internal.h" and the other toOne problem is the Q35 MCH north bridge which directly sets the PCI
PCMachineState->businq35_host_realize(). This seems a QOM violation and is probably easily fixable.
Quoted 5 lines
Quoted 3 lines
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!)
Paolo Bonzini
maintainerQuoted 46 lines · Philippe Mathieu-Daudé
Quoted 45 lines · Michael S. Tsirkin
Quoted 43 lines · Philippe Mathieu-Daudé
Quoted 11 lines · 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é <[email protected]>---
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_HOSTdefinitions andpc_pci_hole64_start()are X86
-pc_machine_is_smm_enabled()is X86 (Paolo sent [5])
- hpet
- tsc (Paolo sent [3])
- 3 more functionsSo we can move half of this file to "
pc_internal.h" and the other toOne problem is the Q35 MCH north bridge which directly sets the PCI
PCMachineState->businq35_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".
Quoted 6 lines
Quoted 1 line
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
Quoted 49 lines · Paolo Bonzini
Quoted 48 lines · Philippe Mathieu-Daudé
Quoted 46 lines · Michael S. Tsirkin
Quoted 35 lines · Philippe Mathieu-Daudé
Quoted 11 lines · 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é <[email protected]>---
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_HOSTdefinitions andpc_pci_hole64_start()are X86
-pc_machine_is_smm_enabled()is X86 (Paolo sent [5])
- hpet
- tsc (Paolo sent [3])
- 3 more functionsSo we can move half of this file to "
pc_internal.h" and the other toQuoted 8 lines
One problem is the Q35 MCH north bridge which directly sets the PCI
PCMachineState->businq35_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.
Quoted 8 lines
Quoted 4 lines
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_initshould be insysemu/kvm.h, since it's x86-specific and that would be something like the same mistake already done withhw/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.