diff mbox series

[05/18] target/arm: Move CPU QOM type definitions to "hw/arm/cpu.h"

Message ID 20230110164406.94366-6-philmd@linaro.org
State New
Headers show
Series hw/arm: Move various objects to softmmu_ss to build them once (part 1) | expand

Commit Message

Philippe Mathieu-Daudé Jan. 10, 2023, 4:43 p.m. UTC 
Units including "target/arm/cpu.h" can't be built once via meson's
softmmu_ss[] source set. Since this header depends on specific
definitions such the word size (32 or 64-bit), for ARM such units
must go to the per-target arm_ss[].

We want to expose few architectural definitions to hardware models.

Start by exposing the ARM CPU QOM types to files under hw/
via the new "hw/arm/cpu.h" header. Doing so, less HW models
will require access to "target/arm/cpu.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/arm/cpu.h | 28 ++++++++++++++++++++++++++++
 target/arm/cpu-qom.h | 13 +------------
 target/arm/cpu.h     |  8 ++++----
 3 files changed, 33 insertions(+), 16 deletions(-)
 create mode 100644 include/hw/arm/cpu.h

Comments

Richard Henderson Jan. 11, 2023, 8:02 p.m. UTC | #1 
On 1/10/23 08:43, Philippe Mathieu-Daudé wrote:
> +++ b/target/arm/cpu.h
> @@ -26,6 +26,7 @@
>   #include "cpu-qom.h"
>   #include "exec/cpu-defs.h"
>   #include "qapi/qapi-types-common.h"
> +#include "hw/arm/cpu.h"

I'm not a fan of this.

If you want a smaller version of cpu-qom.h here in target/arm/, for use by hw/, that's one 
thing.  But target/ should not be reaching back into hw/, IMO.


r~
Philippe Mathieu-Daudé Jan. 12, 2023, 7:17 a.m. UTC | #2 
On 11/1/23 21:02, Richard Henderson wrote:
> On 1/10/23 08:43, Philippe Mathieu-Daudé wrote:
>> +++ b/target/arm/cpu.h
>> @@ -26,6 +26,7 @@
>>   #include "cpu-qom.h"
>>   #include "exec/cpu-defs.h"
>>   #include "qapi/qapi-types-common.h"
>> +#include "hw/arm/cpu.h"
> 
> I'm not a fan of this.
> 
> If you want a smaller version of cpu-qom.h here in target/arm/, for use 
> by hw/, that's one thing.  But target/ should not be reaching back into 
> hw/, IMO.

I concur, but currently we have:

$ git grep '#include "hw' target | wc -l
      220

$ git grep -h '#include "hw' target | sort | uniq -c
    1 #include "hw/acpi/acpi.h"
    1 #include "hw/acpi/ghes.h"
    1 #include "hw/arm/boot.h"
    1 #include "hw/arm/virt.h"
   19 #include "hw/boards.h"
    2 #include "hw/clock.h"
    3 #include "hw/core/accel-cpu.h"
   24 #include "hw/core/cpu.h"
   20 #include "hw/core/sysemu-cpu-ops.h"
   24 #include "hw/core/tcg-cpu-ops.h"
    1 #include "hw/hppa/hppa_hardware.h"
    3 #include "hw/hw.h"
    1 #include "hw/hyperv/hyperv-proto.h"
    2 #include "hw/hyperv/hyperv.h"
    2 #include "hw/i386/apic-msidef.h"
    2 #include "hw/i386/apic.h"
    8 #include "hw/i386/apic_internal.h"
    1 #include "hw/i386/e820_memory_layout.h"
    1 #include "hw/i386/intel_iommu.h"
    1 #include "hw/i386/ioapic.h"
    2 #include "hw/i386/pc.h"
    1 #include "hw/i386/sgx-epc.h"
    1 #include "hw/i386/topology.h"
    1 #include "hw/i386/x86-iommu.h"
    2 #include "hw/i386/x86.h"
    1 #include "hw/intc/riscv_aclint.h"
    8 #include "hw/irq.h"
    1 #include "hw/isa/isa.h"
    5 #include "hw/loader.h"
    1 #include "hw/loongarch/virt.h"
    2 #include "hw/mips/cpudevs.h"
    2 #include "hw/pci/msi.h"
    1 #include "hw/pci/msix.h"
    3 #include "hw/pci/pci.h"
    1 #include "hw/ppc/openpic_kvm.h"
    5 #include "hw/ppc/ppc.h"
    2 #include "hw/ppc/spapr.h"
    1 #include "hw/ppc/spapr_cpu_core.h"
    2 #include "hw/qdev-clock.h"
   12 #include "hw/qdev-properties.h"
   11 #include "hw/registerfields.h"
    2 #include "hw/s390x/ebcdic.h"
    5 #include "hw/s390x/ioinst.h"
    2 #include "hw/s390x/ipl.h"
    8 #include "hw/s390x/pv.h"
    2 #include "hw/s390x/s390-pci-bus.h"
    2 #include "hw/s390x/s390-pci-inst.h"
    2 #include "hw/s390x/s390-virtio-ccw.h"
    2 #include "hw/s390x/s390-virtio-hcall.h"
    3 #include "hw/s390x/s390_flic.h"
    1 #include "hw/s390x/sclp.h"
    2 #include "hw/s390x/storage-keys.h"
    1 #include "hw/s390x/tod.h"
    1 #include "hw/sh4/sh_intc.h"
    2 #include "hw/sysbus.h"
    1 #include "hw/watchdog/wdt_diag288.h"
    1 #include "hw/xtensa/xtensa-isa.h"

Assuming we want to have a self-contained libtarget$arch, how can we
deal with HW tied to the arch such CPU timers or NVIC?
Philippe Mathieu-Daudé April 2, 2025, 4:06 a.m. UTC | #3 
Cc'ing Pierrick

On 12/1/23 08:17, Philippe Mathieu-Daudé wrote:
> On 11/1/23 21:02, Richard Henderson wrote:
>> On 1/10/23 08:43, Philippe Mathieu-Daudé wrote:
>>> +++ b/target/arm/cpu.h
>>> @@ -26,6 +26,7 @@
>>>   #include "cpu-qom.h"
>>>   #include "exec/cpu-defs.h"
>>>   #include "qapi/qapi-types-common.h"
>>> +#include "hw/arm/cpu.h"
>>
>> I'm not a fan of this.
>>
>> If you want a smaller version of cpu-qom.h here in target/arm/, for 
>> use by hw/, that's one thing.  But target/ should not be reaching back 
>> into hw/, IMO.
> 
> I concur, but currently we have:
> 
> $ git grep '#include "hw' target | wc -l
>       220
> 
> $ git grep -h '#include "hw' target | sort | uniq -c
>     1 #include "hw/acpi/acpi.h"
>     1 #include "hw/acpi/ghes.h"
>     1 #include "hw/arm/boot.h"
>     1 #include "hw/arm/virt.h"
>    19 #include "hw/boards.h"
>     2 #include "hw/clock.h"
>     3 #include "hw/core/accel-cpu.h"
>    24 #include "hw/core/cpu.h"
>    20 #include "hw/core/sysemu-cpu-ops.h"
>    24 #include "hw/core/tcg-cpu-ops.h"
>     1 #include "hw/hppa/hppa_hardware.h"
>     3 #include "hw/hw.h"
>     1 #include "hw/hyperv/hyperv-proto.h"
>     2 #include "hw/hyperv/hyperv.h"
>     2 #include "hw/i386/apic-msidef.h"
>     2 #include "hw/i386/apic.h"
>     8 #include "hw/i386/apic_internal.h"
>     1 #include "hw/i386/e820_memory_layout.h"
>     1 #include "hw/i386/intel_iommu.h"
>     1 #include "hw/i386/ioapic.h"
>     2 #include "hw/i386/pc.h"
>     1 #include "hw/i386/sgx-epc.h"
>     1 #include "hw/i386/topology.h"
>     1 #include "hw/i386/x86-iommu.h"
>     2 #include "hw/i386/x86.h"
>     1 #include "hw/intc/riscv_aclint.h"
>     8 #include "hw/irq.h"
>     1 #include "hw/isa/isa.h"
>     5 #include "hw/loader.h"
>     1 #include "hw/loongarch/virt.h"
>     2 #include "hw/mips/cpudevs.h"
>     2 #include "hw/pci/msi.h"
>     1 #include "hw/pci/msix.h"
>     3 #include "hw/pci/pci.h"
>     1 #include "hw/ppc/openpic_kvm.h"
>     5 #include "hw/ppc/ppc.h"
>     2 #include "hw/ppc/spapr.h"
>     1 #include "hw/ppc/spapr_cpu_core.h"
>     2 #include "hw/qdev-clock.h"
>    12 #include "hw/qdev-properties.h"
>    11 #include "hw/registerfields.h"
>     2 #include "hw/s390x/ebcdic.h"
>     5 #include "hw/s390x/ioinst.h"
>     2 #include "hw/s390x/ipl.h"
>     8 #include "hw/s390x/pv.h"
>     2 #include "hw/s390x/s390-pci-bus.h"
>     2 #include "hw/s390x/s390-pci-inst.h"
>     2 #include "hw/s390x/s390-virtio-ccw.h"
>     2 #include "hw/s390x/s390-virtio-hcall.h"
>     3 #include "hw/s390x/s390_flic.h"
>     1 #include "hw/s390x/sclp.h"
>     2 #include "hw/s390x/storage-keys.h"
>     1 #include "hw/s390x/tod.h"
>     1 #include "hw/sh4/sh_intc.h"
>     2 #include "hw/sysbus.h"
>     1 #include "hw/watchdog/wdt_diag288.h"
>     1 #include "hw/xtensa/xtensa-isa.h"
> 
> Assuming we want to have a self-contained libtarget$arch, how can we
> deal with HW tied to the arch such CPU timers or NVIC?
Philippe Mathieu-Daudé April 2, 2025, 4:59 a.m. UTC | #4 
On 2/4/25 06:06, Philippe Mathieu-Daudé wrote:
> Cc'ing Pierrick
> 
> On 12/1/23 08:17, Philippe Mathieu-Daudé wrote:
>> On 11/1/23 21:02, Richard Henderson wrote:
>>> On 1/10/23 08:43, Philippe Mathieu-Daudé wrote:
>>>> +++ b/target/arm/cpu.h
>>>> @@ -26,6 +26,7 @@
>>>>   #include "cpu-qom.h"
>>>>   #include "exec/cpu-defs.h"
>>>>   #include "qapi/qapi-types-common.h"
>>>> +#include "hw/arm/cpu.h"
>>>
>>> I'm not a fan of this.
>>>
>>> If you want a smaller version of cpu-qom.h here in target/arm/, for 
>>> use by hw/, that's one thing.  But target/ should not be reaching 
>>> back into hw/, IMO.

Giving it more thought, we should keep "target/foo/cpu.h" private to
target/foo/, and only access its fields from hw/ via clearly defined
methods/constants.

>>
>> I concur, but currently we have:
>>
>> $ git grep '#include "hw' target | wc -l
>>       220
>>
>> $ git grep -h '#include "hw' target | sort | uniq -c
>>     1 #include "hw/acpi/acpi.h"
>>     1 #include "hw/acpi/ghes.h"
>>     1 #include "hw/arm/boot.h"
>>     1 #include "hw/arm/virt.h"
>>    19 #include "hw/boards.h"
>>     2 #include "hw/clock.h"
>>     3 #include "hw/core/accel-cpu.h"
>>    24 #include "hw/core/cpu.h"
>>    20 #include "hw/core/sysemu-cpu-ops.h"
>>    24 #include "hw/core/tcg-cpu-ops.h"
>>     1 #include "hw/hppa/hppa_hardware.h"
>>     3 #include "hw/hw.h"
>>     1 #include "hw/hyperv/hyperv-proto.h"
>>     2 #include "hw/hyperv/hyperv.h"
>>     2 #include "hw/i386/apic-msidef.h"
>>     2 #include "hw/i386/apic.h"
>>     8 #include "hw/i386/apic_internal.h"
>>     1 #include "hw/i386/e820_memory_layout.h"
>>     1 #include "hw/i386/intel_iommu.h"
>>     1 #include "hw/i386/ioapic.h"
>>     2 #include "hw/i386/pc.h"
>>     1 #include "hw/i386/sgx-epc.h"
>>     1 #include "hw/i386/topology.h"
>>     1 #include "hw/i386/x86-iommu.h"
>>     2 #include "hw/i386/x86.h"
>>     1 #include "hw/intc/riscv_aclint.h"
>>     8 #include "hw/irq.h"
>>     1 #include "hw/isa/isa.h"
>>     5 #include "hw/loader.h"
>>     1 #include "hw/loongarch/virt.h"
>>     2 #include "hw/mips/cpudevs.h"
>>     2 #include "hw/pci/msi.h"
>>     1 #include "hw/pci/msix.h"
>>     3 #include "hw/pci/pci.h"
>>     1 #include "hw/ppc/openpic_kvm.h"
>>     5 #include "hw/ppc/ppc.h"
>>     2 #include "hw/ppc/spapr.h"
>>     1 #include "hw/ppc/spapr_cpu_core.h"
>>     2 #include "hw/qdev-clock.h"
>>    12 #include "hw/qdev-properties.h"
>>    11 #include "hw/registerfields.h"
>>     2 #include "hw/s390x/ebcdic.h"
>>     5 #include "hw/s390x/ioinst.h"
>>     2 #include "hw/s390x/ipl.h"
>>     8 #include "hw/s390x/pv.h"
>>     2 #include "hw/s390x/s390-pci-bus.h"
>>     2 #include "hw/s390x/s390-pci-inst.h"
>>     2 #include "hw/s390x/s390-virtio-ccw.h"
>>     2 #include "hw/s390x/s390-virtio-hcall.h"
>>     3 #include "hw/s390x/s390_flic.h"
>>     1 #include "hw/s390x/sclp.h"
>>     2 #include "hw/s390x/storage-keys.h"
>>     1 #include "hw/s390x/tod.h"
>>     1 #include "hw/sh4/sh_intc.h"
>>     2 #include "hw/sysbus.h"
>>     1 #include "hw/watchdog/wdt_diag288.h"
>>     1 #include "hw/xtensa/xtensa-isa.h"
>>
>> Assuming we want to have a self-contained libtarget$arch, how can we
>> deal with HW tied to the arch such CPU timers or NVIC?
>
Pierrick Bouvier April 2, 2025, 3:29 p.m. UTC | #5 
On 4/1/25 21:06, Philippe Mathieu-Daudé wrote:
> Cc'ing Pierrick
> 
> On 12/1/23 08:17, Philippe Mathieu-Daudé wrote:
>> On 11/1/23 21:02, Richard Henderson wrote:
>>> On 1/10/23 08:43, Philippe Mathieu-Daudé wrote:
>>>> +++ b/target/arm/cpu.h
>>>> @@ -26,6 +26,7 @@
>>>>    #include "cpu-qom.h"
>>>>    #include "exec/cpu-defs.h"
>>>>    #include "qapi/qapi-types-common.h"
>>>> +#include "hw/arm/cpu.h"
>>>
>>> I'm not a fan of this.
>>>
>>> If you want a smaller version of cpu-qom.h here in target/arm/, for
>>> use by hw/, that's one thing.  But target/ should not be reaching back
>>> into hw/, IMO.
>>
>> I concur, but currently we have:
>>
>> $ git grep '#include "hw' target | wc -l
>>        220
>>
>> $ git grep -h '#include "hw' target | sort | uniq -c
>>      1 #include "hw/acpi/acpi.h"
>>      1 #include "hw/acpi/ghes.h"
>>      1 #include "hw/arm/boot.h"
>>      1 #include "hw/arm/virt.h"
>>     19 #include "hw/boards.h"
>>      2 #include "hw/clock.h"
>>      3 #include "hw/core/accel-cpu.h"
>>     24 #include "hw/core/cpu.h"
>>     20 #include "hw/core/sysemu-cpu-ops.h"
>>     24 #include "hw/core/tcg-cpu-ops.h"
>>      1 #include "hw/hppa/hppa_hardware.h"
>>      3 #include "hw/hw.h"
>>      1 #include "hw/hyperv/hyperv-proto.h"
>>      2 #include "hw/hyperv/hyperv.h"
>>      2 #include "hw/i386/apic-msidef.h"
>>      2 #include "hw/i386/apic.h"
>>      8 #include "hw/i386/apic_internal.h"
>>      1 #include "hw/i386/e820_memory_layout.h"
>>      1 #include "hw/i386/intel_iommu.h"
>>      1 #include "hw/i386/ioapic.h"
>>      2 #include "hw/i386/pc.h"
>>      1 #include "hw/i386/sgx-epc.h"
>>      1 #include "hw/i386/topology.h"
>>      1 #include "hw/i386/x86-iommu.h"
>>      2 #include "hw/i386/x86.h"
>>      1 #include "hw/intc/riscv_aclint.h"
>>      8 #include "hw/irq.h"
>>      1 #include "hw/isa/isa.h"
>>      5 #include "hw/loader.h"
>>      1 #include "hw/loongarch/virt.h"
>>      2 #include "hw/mips/cpudevs.h"
>>      2 #include "hw/pci/msi.h"
>>      1 #include "hw/pci/msix.h"
>>      3 #include "hw/pci/pci.h"
>>      1 #include "hw/ppc/openpic_kvm.h"
>>      5 #include "hw/ppc/ppc.h"
>>      2 #include "hw/ppc/spapr.h"
>>      1 #include "hw/ppc/spapr_cpu_core.h"
>>      2 #include "hw/qdev-clock.h"
>>     12 #include "hw/qdev-properties.h"
>>     11 #include "hw/registerfields.h"
>>      2 #include "hw/s390x/ebcdic.h"
>>      5 #include "hw/s390x/ioinst.h"
>>      2 #include "hw/s390x/ipl.h"
>>      8 #include "hw/s390x/pv.h"
>>      2 #include "hw/s390x/s390-pci-bus.h"
>>      2 #include "hw/s390x/s390-pci-inst.h"
>>      2 #include "hw/s390x/s390-virtio-ccw.h"
>>      2 #include "hw/s390x/s390-virtio-hcall.h"
>>      3 #include "hw/s390x/s390_flic.h"
>>      1 #include "hw/s390x/sclp.h"
>>      2 #include "hw/s390x/storage-keys.h"
>>      1 #include "hw/s390x/tod.h"
>>      1 #include "hw/sh4/sh_intc.h"
>>      2 #include "hw/sysbus.h"
>>      1 #include "hw/watchdog/wdt_diag288.h"
>>      1 #include "hw/xtensa/xtensa-isa.h"
>>
>> Assuming we want to have a self-contained libtarget$arch, how can we
>> deal with HW tied to the arch such CPU timers or NVIC?

It's not what we'll achieve.
Right now, what we call libX in meson.build is simply a selection of 
object files, without any guarantee that they don't have external 
dependency on symbols from other libraries. We use that to apply 
specific compilation flags to a set of files, and this is what we really 
achieve here.

Yes, ideally, it would be better if hw was a leaf library in our 
dependency graph.
However, it's not needed to solve this now, and it's not unblocking 
anything to be able to build a single binary, so I don't see it as a 
priority.

>
diff mbox series

Patch

diff --git a/include/hw/arm/cpu.h b/include/hw/arm/cpu.h
new file mode 100644
index 0000000000..0c5d6ca2a8
--- /dev/null
+++ b/include/hw/arm/cpu.h
@@ -0,0 +1,28 @@ 
+/*
+ * ARM / Aarch64 CPU definitions
+ *
+ * This file contains architectural definitions consumed by hardware models
+ * implementations (files under hw/).
+ * Definitions not required to be exposed to hardware has to go in the
+ * architecture specific "target/arm/cpu.h" header.
+ *
+ * Copyright (c) 2003 Fabrice Bellard
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+#ifndef HW_ARM_CPU_H
+#define HW_ARM_CPU_H
+
+#include "hw/core/cpu.h"
+
+#define TYPE_ARM_CPU "arm-cpu"
+OBJECT_DECLARE_CPU_TYPE(ARMCPU, ARMCPUClass, ARM_CPU)
+
+#define TYPE_AARCH64_CPU "aarch64-cpu"
+typedef struct AArch64CPUClass AArch64CPUClass;
+DECLARE_CLASS_CHECKERS(AArch64CPUClass, AARCH64_CPU, TYPE_AARCH64_CPU)
+
+#define ARM_CPU_TYPE_SUFFIX "-" TYPE_ARM_CPU
+#define ARM_CPU_TYPE_NAME(name) (name ARM_CPU_TYPE_SUFFIX)
+
+#endif
diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h
index 514c22ced9..b98904b6bc 100644
--- a/target/arm/cpu-qom.h
+++ b/target/arm/cpu-qom.h
@@ -21,16 +21,11 @@ 
 #define QEMU_ARM_CPU_QOM_H
 
 #include "hw/core/cpu.h"
+#include "hw/arm/cpu.h"
 #include "qom/object.h"
 
 struct arm_boot_info;
 
-#define TYPE_ARM_CPU "arm-cpu"
-
-OBJECT_DECLARE_CPU_TYPE(ARMCPU, ARMCPUClass, ARM_CPU)
-
-#define TYPE_ARM_MAX_CPU "max-" TYPE_ARM_CPU
-
 typedef struct ARMCPUInfo {
     const char *name;
     void (*initfn)(Object *obj);
@@ -57,12 +52,6 @@  struct ARMCPUClass {
     ResettablePhases parent_phases;
 };
 
-
-#define TYPE_AARCH64_CPU "aarch64-cpu"
-typedef struct AArch64CPUClass AArch64CPUClass;
-DECLARE_CLASS_CHECKERS(AArch64CPUClass, AARCH64_CPU,
-                       TYPE_AARCH64_CPU)
-
 struct AArch64CPUClass {
     /*< private >*/
     ARMCPUClass parent_class;
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index bf2bce046d..52ac99cad3 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -17,8 +17,8 @@ 
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
 
-#ifndef ARM_CPU_H
-#define ARM_CPU_H
+#ifndef TARGET_ARM_CPU_H
+#define TARGET_ARM_CPU_H
 
 #include "kvm-consts.h"
 #include "qemu/cpu-float.h"
@@ -26,6 +26,7 @@ 
 #include "cpu-qom.h"
 #include "exec/cpu-defs.h"
 #include "qapi/qapi-types-common.h"
+#include "hw/arm/cpu.h"
 
 /* ARM processors have a weak memory model */
 #define TCG_GUEST_DEFAULT_MO      (0)
@@ -2853,11 +2854,10 @@  bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync);
 #define ARM_CPUID_TI915T      0x54029152
 #define ARM_CPUID_TI925T      0x54029252
 
-#define ARM_CPU_TYPE_SUFFIX "-" TYPE_ARM_CPU
-#define ARM_CPU_TYPE_NAME(name) (name ARM_CPU_TYPE_SUFFIX)
 #define CPU_RESOLVING_TYPE TYPE_ARM_CPU
 
 #define TYPE_ARM_HOST_CPU "host-" TYPE_ARM_CPU
+#define TYPE_ARM_MAX_CPU "max-" TYPE_ARM_CPU
 
 #define cpu_list arm_cpu_list