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

2019-12-16 Thread 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é 
---
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.


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

2019-12-16 Thread 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é 
 ---
 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


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

2019-12-16 Thread 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é 
---
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!)


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

2019-12-15 Thread 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é 
> > ---
> > 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


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

2019-12-13 Thread 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é 
---
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



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

2019-12-13 Thread 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é 
---
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 00..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,