[RFC PATCH v3 18/28] target/arm: Move common cpu code into cpu.c

Fabiano Rosas posted 28 patches 2 years, 3 months ago
There is a newer version of this series
[RFC PATCH v3 18/28] target/arm: Move common cpu code into cpu.c
Posted by Fabiano Rosas 2 years, 3 months ago
The cpu_tcg.c file about to be moved into the tcg directory. Move the
code that is needed for cpus that also work with KVM into cpu.c.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 target/arm/cpu.c     | 76 +++++++++++++++++++++++++++++++++++++++++++
 target/arm/cpu_tcg.c | 77 --------------------------------------------
 2 files changed, 76 insertions(+), 77 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index ce1a425e10..3a1fa3b20c 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -45,6 +45,75 @@
 #include "fpu/softfloat.h"
 #include "cpregs.h"
 
+/* Share AArch32 -cpu max features with AArch64. */
+void aa32_max_features(ARMCPU *cpu)
+{
+    uint32_t t;
+
+    /* Add additional features supported by QEMU */
+    t = cpu->isar.id_isar5;
+    t = FIELD_DP32(t, ID_ISAR5, AES, 2);          /* FEAT_PMULL */
+    t = FIELD_DP32(t, ID_ISAR5, SHA1, 1);         /* FEAT_SHA1 */
+    t = FIELD_DP32(t, ID_ISAR5, SHA2, 1);         /* FEAT_SHA256 */
+    t = FIELD_DP32(t, ID_ISAR5, CRC32, 1);
+    t = FIELD_DP32(t, ID_ISAR5, RDM, 1);          /* FEAT_RDM */
+    t = FIELD_DP32(t, ID_ISAR5, VCMA, 1);         /* FEAT_FCMA */
+    cpu->isar.id_isar5 = t;
+
+    t = cpu->isar.id_isar6;
+    t = FIELD_DP32(t, ID_ISAR6, JSCVT, 1);        /* FEAT_JSCVT */
+    t = FIELD_DP32(t, ID_ISAR6, DP, 1);           /* Feat_DotProd */
+    t = FIELD_DP32(t, ID_ISAR6, FHM, 1);          /* FEAT_FHM */
+    t = FIELD_DP32(t, ID_ISAR6, SB, 1);           /* FEAT_SB */
+    t = FIELD_DP32(t, ID_ISAR6, SPECRES, 1);      /* FEAT_SPECRES */
+    t = FIELD_DP32(t, ID_ISAR6, BF16, 1);         /* FEAT_AA32BF16 */
+    t = FIELD_DP32(t, ID_ISAR6, I8MM, 1);         /* FEAT_AA32I8MM */
+    cpu->isar.id_isar6 = t;
+
+    t = cpu->isar.mvfr1;
+    t = FIELD_DP32(t, MVFR1, FPHP, 3);            /* FEAT_FP16 */
+    t = FIELD_DP32(t, MVFR1, SIMDHP, 2);          /* FEAT_FP16 */
+    cpu->isar.mvfr1 = t;
+
+    t = cpu->isar.mvfr2;
+    t = FIELD_DP32(t, MVFR2, SIMDMISC, 3);        /* SIMD MaxNum */
+    t = FIELD_DP32(t, MVFR2, FPMISC, 4);          /* FP MaxNum */
+    cpu->isar.mvfr2 = t;
+
+    t = cpu->isar.id_mmfr3;
+    t = FIELD_DP32(t, ID_MMFR3, PAN, 2);          /* FEAT_PAN2 */
+    cpu->isar.id_mmfr3 = t;
+
+    t = cpu->isar.id_mmfr4;
+    t = FIELD_DP32(t, ID_MMFR4, HPDS, 1);         /* FEAT_AA32HPD */
+    t = FIELD_DP32(t, ID_MMFR4, AC2, 1);          /* ACTLR2, HACTLR2 */
+    t = FIELD_DP32(t, ID_MMFR4, CNP, 1);          /* FEAT_TTCNP */
+    t = FIELD_DP32(t, ID_MMFR4, XNX, 1);          /* FEAT_XNX */
+    t = FIELD_DP32(t, ID_MMFR4, EVT, 2);          /* FEAT_EVT */
+    cpu->isar.id_mmfr4 = t;
+
+    t = cpu->isar.id_mmfr5;
+    t = FIELD_DP32(t, ID_MMFR5, ETS, 1);          /* FEAT_ETS */
+    cpu->isar.id_mmfr5 = t;
+
+    t = cpu->isar.id_pfr0;
+    t = FIELD_DP32(t, ID_PFR0, CSV2, 2);          /* FEAT_CVS2 */
+    t = FIELD_DP32(t, ID_PFR0, DIT, 1);           /* FEAT_DIT */
+    t = FIELD_DP32(t, ID_PFR0, RAS, 1);           /* FEAT_RAS */
+    cpu->isar.id_pfr0 = t;
+
+    t = cpu->isar.id_pfr2;
+    t = FIELD_DP32(t, ID_PFR2, CSV3, 1);          /* FEAT_CSV3 */
+    t = FIELD_DP32(t, ID_PFR2, SSBS, 1);          /* FEAT_SSBS */
+    cpu->isar.id_pfr2 = t;
+
+    t = cpu->isar.id_dfr0;
+    t = FIELD_DP32(t, ID_DFR0, COPDBG, 9);        /* FEAT_Debugv8p4 */
+    t = FIELD_DP32(t, ID_DFR0, COPSDBG, 9);       /* FEAT_Debugv8p4 */
+    t = FIELD_DP32(t, ID_DFR0, PERFMON, 6);       /* FEAT_PMUv3p5 */
+    cpu->isar.id_dfr0 = t;
+}
+
 static void arm_cpu_set_pc(CPUState *cs, vaddr value)
 {
     ARMCPU *cpu = ARM_CPU(cs);
@@ -2315,8 +2384,15 @@ static const TypeInfo arm_cpu_type_info = {
     .class_init = arm_cpu_class_init,
 };
 
+static const TypeInfo idau_interface_type_info = {
+    .name = TYPE_IDAU_INTERFACE,
+    .parent = TYPE_INTERFACE,
+    .class_size = sizeof(IDAUInterfaceClass),
+};
+
 static void arm_cpu_register_types(void)
 {
+    type_register_static(&idau_interface_type_info);
     type_register_static(&arm_cpu_type_info);
 }
 
diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c
index 64d5a785c1..571e29bc16 100644
--- a/target/arm/cpu_tcg.c
+++ b/target/arm/cpu_tcg.c
@@ -14,82 +14,12 @@
 #include "hw/core/tcg-cpu-ops.h"
 #endif /* CONFIG_TCG */
 #include "internals.h"
-#include "target/arm/idau.h"
 #if !defined(CONFIG_USER_ONLY)
 #include "hw/boards.h"
 #endif
 #include "cpregs.h"
 
 
-/* Share AArch32 -cpu max features with AArch64. */
-void aa32_max_features(ARMCPU *cpu)
-{
-    uint32_t t;
-
-    /* Add additional features supported by QEMU */
-    t = cpu->isar.id_isar5;
-    t = FIELD_DP32(t, ID_ISAR5, AES, 2);          /* FEAT_PMULL */
-    t = FIELD_DP32(t, ID_ISAR5, SHA1, 1);         /* FEAT_SHA1 */
-    t = FIELD_DP32(t, ID_ISAR5, SHA2, 1);         /* FEAT_SHA256 */
-    t = FIELD_DP32(t, ID_ISAR5, CRC32, 1);
-    t = FIELD_DP32(t, ID_ISAR5, RDM, 1);          /* FEAT_RDM */
-    t = FIELD_DP32(t, ID_ISAR5, VCMA, 1);         /* FEAT_FCMA */
-    cpu->isar.id_isar5 = t;
-
-    t = cpu->isar.id_isar6;
-    t = FIELD_DP32(t, ID_ISAR6, JSCVT, 1);        /* FEAT_JSCVT */
-    t = FIELD_DP32(t, ID_ISAR6, DP, 1);           /* Feat_DotProd */
-    t = FIELD_DP32(t, ID_ISAR6, FHM, 1);          /* FEAT_FHM */
-    t = FIELD_DP32(t, ID_ISAR6, SB, 1);           /* FEAT_SB */
-    t = FIELD_DP32(t, ID_ISAR6, SPECRES, 1);      /* FEAT_SPECRES */
-    t = FIELD_DP32(t, ID_ISAR6, BF16, 1);         /* FEAT_AA32BF16 */
-    t = FIELD_DP32(t, ID_ISAR6, I8MM, 1);         /* FEAT_AA32I8MM */
-    cpu->isar.id_isar6 = t;
-
-    t = cpu->isar.mvfr1;
-    t = FIELD_DP32(t, MVFR1, FPHP, 3);            /* FEAT_FP16 */
-    t = FIELD_DP32(t, MVFR1, SIMDHP, 2);          /* FEAT_FP16 */
-    cpu->isar.mvfr1 = t;
-
-    t = cpu->isar.mvfr2;
-    t = FIELD_DP32(t, MVFR2, SIMDMISC, 3);        /* SIMD MaxNum */
-    t = FIELD_DP32(t, MVFR2, FPMISC, 4);          /* FP MaxNum */
-    cpu->isar.mvfr2 = t;
-
-    t = cpu->isar.id_mmfr3;
-    t = FIELD_DP32(t, ID_MMFR3, PAN, 2);          /* FEAT_PAN2 */
-    cpu->isar.id_mmfr3 = t;
-
-    t = cpu->isar.id_mmfr4;
-    t = FIELD_DP32(t, ID_MMFR4, HPDS, 1);         /* FEAT_AA32HPD */
-    t = FIELD_DP32(t, ID_MMFR4, AC2, 1);          /* ACTLR2, HACTLR2 */
-    t = FIELD_DP32(t, ID_MMFR4, CNP, 1);          /* FEAT_TTCNP */
-    t = FIELD_DP32(t, ID_MMFR4, XNX, 1);          /* FEAT_XNX */
-    t = FIELD_DP32(t, ID_MMFR4, EVT, 2);          /* FEAT_EVT */
-    cpu->isar.id_mmfr4 = t;
-
-    t = cpu->isar.id_mmfr5;
-    t = FIELD_DP32(t, ID_MMFR5, ETS, 1);          /* FEAT_ETS */
-    cpu->isar.id_mmfr5 = t;
-
-    t = cpu->isar.id_pfr0;
-    t = FIELD_DP32(t, ID_PFR0, CSV2, 2);          /* FEAT_CVS2 */
-    t = FIELD_DP32(t, ID_PFR0, DIT, 1);           /* FEAT_DIT */
-    t = FIELD_DP32(t, ID_PFR0, RAS, 1);           /* FEAT_RAS */
-    cpu->isar.id_pfr0 = t;
-
-    t = cpu->isar.id_pfr2;
-    t = FIELD_DP32(t, ID_PFR2, CSV3, 1);          /* FEAT_CSV3 */
-    t = FIELD_DP32(t, ID_PFR2, SSBS, 1);          /* FEAT_SSBS */
-    cpu->isar.id_pfr2 = t;
-
-    t = cpu->isar.id_dfr0;
-    t = FIELD_DP32(t, ID_DFR0, COPDBG, 9);        /* FEAT_Debugv8p4 */
-    t = FIELD_DP32(t, ID_DFR0, COPSDBG, 9);       /* FEAT_Debugv8p4 */
-    t = FIELD_DP32(t, ID_DFR0, PERFMON, 6);       /* FEAT_PMUv3p5 */
-    cpu->isar.id_dfr0 = t;
-}
-
 /* CPU models. These are not needed for the AArch64 linux-user build. */
 #if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64)
 
@@ -1170,17 +1100,10 @@ static const ARMCPUInfo arm_tcg_cpus[] = {
 #endif
 };
 
-static const TypeInfo idau_interface_type_info = {
-    .name = TYPE_IDAU_INTERFACE,
-    .parent = TYPE_INTERFACE,
-    .class_size = sizeof(IDAUInterfaceClass),
-};
-
 static void arm_tcg_cpu_register_types(void)
 {
     size_t i;
 
-    type_register_static(&idau_interface_type_info);
     for (i = 0; i < ARRAY_SIZE(arm_tcg_cpus); ++i) {
         arm_cpu_register(&arm_tcg_cpus[i]);
     }
-- 
2.35.3
Re: [RFC PATCH v3 18/28] target/arm: Move common cpu code into cpu.c
Posted by Philippe Mathieu-Daudé 2 years, 3 months ago
On 13/1/23 15:04, Fabiano Rosas wrote:
> The cpu_tcg.c file about to be moved into the tcg directory. Move the
> code that is needed for cpus that also work with KVM into cpu.c.
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>   target/arm/cpu.c     | 76 +++++++++++++++++++++++++++++++++++++++++++
>   target/arm/cpu_tcg.c | 77 --------------------------------------------
>   2 files changed, 76 insertions(+), 77 deletions(-)
> 
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
[...]

TYPE_IDAU_INTERFACE is ARMv8-M specific, so TCG AFAIU.

> +static const TypeInfo idau_interface_type_info = {
> +    .name = TYPE_IDAU_INTERFACE,
> +    .parent = TYPE_INTERFACE,
> +    .class_size = sizeof(IDAUInterfaceClass),
> +};
> +
>   static void arm_cpu_register_types(void)
>   {
> +    type_register_static(&idau_interface_type_info);
>       type_register_static(&arm_cpu_type_info);
>   }
>   
> diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c

> -static const TypeInfo idau_interface_type_info = {
> -    .name = TYPE_IDAU_INTERFACE,
> -    .parent = TYPE_INTERFACE,
> -    .class_size = sizeof(IDAUInterfaceClass),
> -};
> -
>   static void arm_tcg_cpu_register_types(void)
>   {
>       size_t i;
>   
> -    type_register_static(&idau_interface_type_info);
>       for (i = 0; i < ARRAY_SIZE(arm_tcg_cpus); ++i) {
>           arm_cpu_register(&arm_tcg_cpus[i]);
>       }
Re: [RFC PATCH v3 18/28] target/arm: Move common cpu code into cpu.c
Posted by Fabiano Rosas 2 years, 3 months ago
Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> On 13/1/23 15:04, Fabiano Rosas wrote:
>> The cpu_tcg.c file about to be moved into the tcg directory. Move the
>> code that is needed for cpus that also work with KVM into cpu.c.
>> 
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> ---
>>   target/arm/cpu.c     | 76 +++++++++++++++++++++++++++++++++++++++++++
>>   target/arm/cpu_tcg.c | 77 --------------------------------------------
>>   2 files changed, 76 insertions(+), 77 deletions(-)
>> 
>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> [...]
>
> TYPE_IDAU_INTERFACE is ARMv8-M specific, so TCG AFAIU.

Hm.. QEMU doesn't start without it. There might be some implicit
dependency. I'll check.
Re: [RFC PATCH v3 18/28] target/arm: Move common cpu code into cpu.c
Posted by Philippe Mathieu-Daudé 2 years, 3 months ago
On 17/1/23 20:01, Fabiano Rosas wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> On 13/1/23 15:04, Fabiano Rosas wrote:
>>> The cpu_tcg.c file about to be moved into the tcg directory. Move the
>>> code that is needed for cpus that also work with KVM into cpu.c.
>>>
>>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>>> ---
>>>    target/arm/cpu.c     | 76 +++++++++++++++++++++++++++++++++++++++++++
>>>    target/arm/cpu_tcg.c | 77 --------------------------------------------
>>>    2 files changed, 76 insertions(+), 77 deletions(-)
>>>
>>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>> [...]
>>
>> TYPE_IDAU_INTERFACE is ARMv8-M specific, so TCG AFAIU.
> 
> Hm.. QEMU doesn't start without it. There might be some implicit
> dependency. I'll check.

Likely some M-profile code (note this type is a QOM *interface*).

I checked the uses (git-grep -W IDAU_INTERFACE) and none should be
reachable in a non-TCG build.

Re: [RFC PATCH v3 18/28] target/arm: Move common cpu code into cpu.c
Posted by Claudio Fontana 2 years, 3 months ago
On 1/18/23 11:45, Philippe Mathieu-Daudé wrote:
> On 17/1/23 20:01, Fabiano Rosas wrote:
>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>>
>>> On 13/1/23 15:04, Fabiano Rosas wrote:
>>>> The cpu_tcg.c file about to be moved into the tcg directory. Move the
>>>> code that is needed for cpus that also work with KVM into cpu.c.
>>>>
>>>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>>>> ---
>>>>    target/arm/cpu.c     | 76 +++++++++++++++++++++++++++++++++++++++++++
>>>>    target/arm/cpu_tcg.c | 77 --------------------------------------------
>>>>    2 files changed, 76 insertions(+), 77 deletions(-)
>>>>
>>>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>>> [...]
>>>
>>> TYPE_IDAU_INTERFACE is ARMv8-M specific, so TCG AFAIU.
>>
>> Hm.. QEMU doesn't start without it. There might be some implicit
>> dependency. I'll check.
> 
> Likely some M-profile code (note this type is a QOM *interface*).
> 
> I checked the uses (git-grep -W IDAU_INTERFACE) and none should be
> reachable in a non-TCG build.

crossing fingers, I remember getting in trouble there, but maybe that is now solved by the KConfig thing..

https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg02958.html

My understanding is probably obsolete now, if so sorry for the noise.

Claudio


Re: [RFC PATCH v3 18/28] target/arm: Move common cpu code into cpu.c
Posted by Fabiano Rosas 2 years, 3 months ago
Claudio Fontana <cfontana@suse.de> writes:

> On 1/18/23 11:45, Philippe Mathieu-Daudé wrote:
>> On 17/1/23 20:01, Fabiano Rosas wrote:
>>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>>>
>>>> On 13/1/23 15:04, Fabiano Rosas wrote:
>>>>> The cpu_tcg.c file about to be moved into the tcg directory. Move the
>>>>> code that is needed for cpus that also work with KVM into cpu.c.
>>>>>
>>>>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>>>>> ---
>>>>>    target/arm/cpu.c     | 76 +++++++++++++++++++++++++++++++++++++++++++
>>>>>    target/arm/cpu_tcg.c | 77 --------------------------------------------
>>>>>    2 files changed, 76 insertions(+), 77 deletions(-)
>>>>>
>>>>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>>>> [...]
>>>>
>>>> TYPE_IDAU_INTERFACE is ARMv8-M specific, so TCG AFAIU.
>>>
>>> Hm.. QEMU doesn't start without it. There might be some implicit
>>> dependency. I'll check.
>> 
>> Likely some M-profile code (note this type is a QOM *interface*).
>> 
>> I checked the uses (git-grep -W IDAU_INTERFACE) and none should be
>> reachable in a non-TCG build.
>
> crossing fingers, I remember getting in trouble there, but maybe that is now solved by the KConfig thing..
>
> https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg02958.html
>
> My understanding is probably obsolete now, if so sorry for the noise.

I guess this is what I was remembering. But after the Kconfig and qtest
changes everything looks good.
Re: [RFC PATCH v3 18/28] target/arm: Move common cpu code into cpu.c
Posted by Richard Henderson 2 years, 3 months ago
On 1/13/23 06:04, Fabiano Rosas wrote:
> The cpu_tcg.c file about to be moved into the tcg directory. Move the
> code that is needed for cpus that also work with KVM into cpu.c.
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>   target/arm/cpu.c     | 76 +++++++++++++++++++++++++++++++++++++++++++
>   target/arm/cpu_tcg.c | 77 --------------------------------------------
>   2 files changed, 76 insertions(+), 77 deletions(-)

Actually, not true.  This is tcg-only.  As is the bulk of aarch64_max_initfn from which 
this is called -- note the first 4 lines of that function:

     if (kvm_enabled() || hvf_enabled()) {
         /* With KVM or HVF, '-cpu max' is identical to '-cpu host' */
         aarch64_host_initfn(obj);
         return;
     }

Thus the rest of the function is only reachable for tcg.


r~
Re: [RFC PATCH v3 18/28] target/arm: Move common cpu code into cpu.c
Posted by Fabiano Rosas 2 years, 3 months ago
Richard Henderson <richard.henderson@linaro.org> writes:

> On 1/13/23 06:04, Fabiano Rosas wrote:
>> The cpu_tcg.c file about to be moved into the tcg directory. Move the
>> code that is needed for cpus that also work with KVM into cpu.c.
>> 
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> ---
>>   target/arm/cpu.c     | 76 +++++++++++++++++++++++++++++++++++++++++++
>>   target/arm/cpu_tcg.c | 77 --------------------------------------------
>>   2 files changed, 76 insertions(+), 77 deletions(-)
>
> Actually, not true.  This is tcg-only.  As is the bulk of aarch64_max_initfn from which 
> this is called -- note the first 4 lines of that function:
>
>      if (kvm_enabled() || hvf_enabled()) {
>          /* With KVM or HVF, '-cpu max' is identical to '-cpu host' */
>          aarch64_host_initfn(obj);
>          return;
>      }
>
> Thus the rest of the function is only reachable for tcg.

Sigh... It seems it's not that simple:

We can currently have a QEMU invocation with "-accel qtest -cpu max" and
no other accelerator. Currently, it falls into this implicit else
branch. So this is actually "else if (tcg_enabled() || qtest_enabled())".

If I move the "TCG-only" code under CONFIG_TCG, the qtests that use -cpu
max will break.

So I have chosen to move the code which depends on aa32_max_features as
you suggest into tcg/ but kept the cortex-a57 as a baseline for
qtest. This has the effect of causing "-cpu max" for the tests to be a
slightly different CPU depending on whether TCG is built in (which
perhaps is ok because if the tests depended on cpu features they should
specify an accel?).