| Message ID | 20230113140419.4013-19-farosas@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/arm: Allow CONFIG_TCG=n builds | expand |
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~
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]); > }
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.
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.
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
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.
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?).
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]); }
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(-)