| Message ID | 20180629001538.11415-7-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm SVE updates | expand |
On 06/28/2018 09:15 PM, Richard Henderson wrote: > For the supported extensions, fill in the appropriate bits in > ID_ISAR5, ID_ISAR6, ID_AA64ISAR0, ID_AA64ISAR1. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/arm/cpu.c | 24 +++++++++++++++++------- > target/arm/cpu64.c | 36 ++++++++++++++++++++++++++++-------- > 2 files changed, 45 insertions(+), 15 deletions(-) > > diff --git a/target/arm/cpu.c b/target/arm/cpu.c > index de1a07a9f1..943c589445 100644 > --- a/target/arm/cpu.c > +++ b/target/arm/cpu.c > @@ -1795,19 +1795,29 @@ static void arm_max_initfn(Object *obj) > kvm_arm_set_cpu_features_from_host(cpu); > } else { > cortex_a15_initfn(obj); > + > + set_feature(&cpu->env, ARM_FEATURE_V8_AES); > + cpu->id_isar5 = deposit32(cpu->id_isar5, 4, 4, 2); > + set_feature(&cpu->env, ARM_FEATURE_V8_SHA1); > + cpu->id_isar5 = deposit32(cpu->id_isar5, 8, 4, 1); > + set_feature(&cpu->env, ARM_FEATURE_V8_SHA256); > + cpu->id_isar5 = deposit32(cpu->id_isar5, 12, 4, 1); > + set_feature(&cpu->env, ARM_FEATURE_CRC); > + cpu->id_isar5 = deposit32(cpu->id_isar5, 16, 4, 1); > + set_feature(&cpu->env, ARM_FEATURE_V8_RDM); > + cpu->id_isar5 = deposit32(cpu->id_isar5, 24, 4, 1); > + set_feature(&cpu->env, ARM_FEATURE_V8_FCMA); > + cpu->id_isar5 = deposit32(cpu->id_isar5, 28, 4, 1); > + > + set_feature(&cpu->env, ARM_FEATURE_V8_DOTPROD); > + cpu->id_isar6 = deposit32(cpu->id_isar6, 4, 4, 1); Since I don't have ARM_FEATURE_V8_DOTPROD, I now realize this series goes on top of your "target/arm SVE patches" v6: http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg07698.html > + > #ifdef CONFIG_USER_ONLY > /* We don't set these in system emulation mode for the moment, > * since we don't correctly set the ID registers to advertise them, > */ > set_feature(&cpu->env, ARM_FEATURE_V8); > - set_feature(&cpu->env, ARM_FEATURE_V8_AES); > - set_feature(&cpu->env, ARM_FEATURE_V8_SHA1); > - set_feature(&cpu->env, ARM_FEATURE_V8_SHA256); > set_feature(&cpu->env, ARM_FEATURE_V8_PMULL); > - set_feature(&cpu->env, ARM_FEATURE_CRC); > - set_feature(&cpu->env, ARM_FEATURE_V8_RDM); > - set_feature(&cpu->env, ARM_FEATURE_V8_DOTPROD); > - set_feature(&cpu->env, ARM_FEATURE_V8_FCMA); > #endif > } > } > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c > index d0581d59d8..b24fee45e3 100644 > --- a/target/arm/cpu64.c > +++ b/target/arm/cpu64.c > @@ -230,6 +230,34 @@ static void aarch64_max_initfn(Object *obj) > kvm_arm_set_cpu_features_from_host(cpu); > } else { > aarch64_a57_initfn(obj); > + > + set_feature(&cpu->env, ARM_FEATURE_V8_SHA512); > + cpu->id_aa64isar0 = deposit64(cpu->id_aa64isar0, 12, 4, 2); > + > + set_feature(&cpu->env, ARM_FEATURE_V8_ATOMICS); > + cpu->id_aa64isar0 = deposit64(cpu->id_aa64isar0, 20, 4, 2); > + > + set_feature(&cpu->env, ARM_FEATURE_V8_RDM); > + cpu->id_aa64isar0 = deposit64(cpu->id_aa64isar0, 28, 4, 1); > + cpu->id_isar5 = deposit32(cpu->id_isar5, 24, 4, 1); > + > + set_feature(&cpu->env, ARM_FEATURE_V8_SHA3); > + cpu->id_aa64isar0 = deposit64(cpu->id_aa64isar0, 32, 4, 1); > + > + set_feature(&cpu->env, ARM_FEATURE_V8_SM3); > + cpu->id_aa64isar0 = deposit64(cpu->id_aa64isar0, 36, 4, 1); > + > + set_feature(&cpu->env, ARM_FEATURE_V8_SM4); > + cpu->id_aa64isar0 = deposit64(cpu->id_aa64isar0, 40, 4, 1); > + > + set_feature(&cpu->env, ARM_FEATURE_V8_DOTPROD); > + cpu->id_aa64isar0 = deposit64(cpu->id_aa64isar0, 44, 4, 1); > + cpu->id_isar6 = deposit32(cpu->id_isar6, 4, 4, 1); > + > + set_feature(&cpu->env, ARM_FEATURE_V8_FCMA); > + cpu->id_aa64isar1 = deposit64(cpu->id_aa64isar1, 16, 4, 1); > + cpu->id_isar5 = deposit32(cpu->id_isar5, 28, 4, 1); > + > #ifdef CONFIG_USER_ONLY > /* We don't set these in system emulation mode for the moment, > * since we don't correctly set the ID registers to advertise them, > @@ -237,15 +265,7 @@ static void aarch64_max_initfn(Object *obj) > * whereas the architecture requires them to be present in both if > * present in either. > */ > - set_feature(&cpu->env, ARM_FEATURE_V8_SHA512); > - set_feature(&cpu->env, ARM_FEATURE_V8_SHA3); > - set_feature(&cpu->env, ARM_FEATURE_V8_SM3); > - set_feature(&cpu->env, ARM_FEATURE_V8_SM4); > - set_feature(&cpu->env, ARM_FEATURE_V8_ATOMICS); > - set_feature(&cpu->env, ARM_FEATURE_V8_RDM); > - set_feature(&cpu->env, ARM_FEATURE_V8_DOTPROD); > set_feature(&cpu->env, ARM_FEATURE_V8_FP16); > - set_feature(&cpu->env, ARM_FEATURE_V8_FCMA); > set_feature(&cpu->env, ARM_FEATURE_SVE); > /* For usermode -cpu max we can use a larger and more efficient DCZ > * blocksize since we don't have to follow what the hardware does. >
On 29 June 2018 at 01:15, Richard Henderson <richard.henderson@linaro.org> wrote: > For the supported extensions, fill in the appropriate bits in > ID_ISAR5, ID_ISAR6, ID_AA64ISAR0, ID_AA64ISAR1. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- This makes sense, but I'd rather have a bit of time to think about how exactly we want to handle feature bits vs ID register values (the current codebase is not entirely coherent on the topic), so I'd rather not put this in for softfreeze unless there's a strong reason we should... thanks -- PMM
On 06/29/2018 01:42 AM, Peter Maydell wrote: > On 29 June 2018 at 01:15, Richard Henderson > <richard.henderson@linaro.org> wrote: >> For the supported extensions, fill in the appropriate bits in >> ID_ISAR5, ID_ISAR6, ID_AA64ISAR0, ID_AA64ISAR1. >> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- > > This makes sense, but I'd rather have a bit of time to think > about how exactly we want to handle feature bits vs ID > register values (the current codebase is not entirely > coherent on the topic), so I'd rather not put this in > for softfreeze unless there's a strong reason we should... Fair. I was wondering if we'd post-process the feature bits to initialize the id registers, so that we don't have different places with the same knowledge. The clearing of EL3 bits from id_pfr1 is an example of that already. r~
On 29 June 2018 at 15:54, Richard Henderson <richard.henderson@linaro.org> wrote: > On 06/29/2018 01:42 AM, Peter Maydell wrote: >> On 29 June 2018 at 01:15, Richard Henderson >> <richard.henderson@linaro.org> wrote: >>> For the supported extensions, fill in the appropriate bits in >>> ID_ISAR5, ID_ISAR6, ID_AA64ISAR0, ID_AA64ISAR1. >> This makes sense, but I'd rather have a bit of time to think >> about how exactly we want to handle feature bits vs ID >> register values (the current codebase is not entirely >> coherent on the topic), so I'd rather not put this in >> for softfreeze unless there's a strong reason we should... > > Fair. > > I was wondering if we'd post-process the feature bits to initialize the id > registers, so that we don't have different places with the same knowledge. > > The clearing of EL3 bits from id_pfr1 is an example of that already. Yes, exactly. We have a few bits that are odd like that, where we've kind of ad-hoc tweaked stuff to make things work. I'd rather we decided on a coherent design and then moved the code to do that. (Another example is Aaron's PMUv3 patchset, which wants to tweak the PMU related ID registers to match what the emulation code implements.) The original QEMU design I think was more or less "we report fixed ID regs which don't necessarily match our actual capabilities" (which is also how a KVM vCPU will behave). "Feature bits are the primary source of truth" is also a coherent design, just not the one we started with... thanks -- PMM
diff --git a/target/arm/cpu.c b/target/arm/cpu.c index de1a07a9f1..943c589445 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -1795,19 +1795,29 @@ static void arm_max_initfn(Object *obj) kvm_arm_set_cpu_features_from_host(cpu); } else { cortex_a15_initfn(obj); + + set_feature(&cpu->env, ARM_FEATURE_V8_AES); + cpu->id_isar5 = deposit32(cpu->id_isar5, 4, 4, 2); + set_feature(&cpu->env, ARM_FEATURE_V8_SHA1); + cpu->id_isar5 = deposit32(cpu->id_isar5, 8, 4, 1); + set_feature(&cpu->env, ARM_FEATURE_V8_SHA256); + cpu->id_isar5 = deposit32(cpu->id_isar5, 12, 4, 1); + set_feature(&cpu->env, ARM_FEATURE_CRC); + cpu->id_isar5 = deposit32(cpu->id_isar5, 16, 4, 1); + set_feature(&cpu->env, ARM_FEATURE_V8_RDM); + cpu->id_isar5 = deposit32(cpu->id_isar5, 24, 4, 1); + set_feature(&cpu->env, ARM_FEATURE_V8_FCMA); + cpu->id_isar5 = deposit32(cpu->id_isar5, 28, 4, 1); + + set_feature(&cpu->env, ARM_FEATURE_V8_DOTPROD); + cpu->id_isar6 = deposit32(cpu->id_isar6, 4, 4, 1); + #ifdef CONFIG_USER_ONLY /* We don't set these in system emulation mode for the moment, * since we don't correctly set the ID registers to advertise them, */ set_feature(&cpu->env, ARM_FEATURE_V8); - set_feature(&cpu->env, ARM_FEATURE_V8_AES); - set_feature(&cpu->env, ARM_FEATURE_V8_SHA1); - set_feature(&cpu->env, ARM_FEATURE_V8_SHA256); set_feature(&cpu->env, ARM_FEATURE_V8_PMULL); - set_feature(&cpu->env, ARM_FEATURE_CRC); - set_feature(&cpu->env, ARM_FEATURE_V8_RDM); - set_feature(&cpu->env, ARM_FEATURE_V8_DOTPROD); - set_feature(&cpu->env, ARM_FEATURE_V8_FCMA); #endif } } diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index d0581d59d8..b24fee45e3 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -230,6 +230,34 @@ static void aarch64_max_initfn(Object *obj) kvm_arm_set_cpu_features_from_host(cpu); } else { aarch64_a57_initfn(obj); + + set_feature(&cpu->env, ARM_FEATURE_V8_SHA512); + cpu->id_aa64isar0 = deposit64(cpu->id_aa64isar0, 12, 4, 2); + + set_feature(&cpu->env, ARM_FEATURE_V8_ATOMICS); + cpu->id_aa64isar0 = deposit64(cpu->id_aa64isar0, 20, 4, 2); + + set_feature(&cpu->env, ARM_FEATURE_V8_RDM); + cpu->id_aa64isar0 = deposit64(cpu->id_aa64isar0, 28, 4, 1); + cpu->id_isar5 = deposit32(cpu->id_isar5, 24, 4, 1); + + set_feature(&cpu->env, ARM_FEATURE_V8_SHA3); + cpu->id_aa64isar0 = deposit64(cpu->id_aa64isar0, 32, 4, 1); + + set_feature(&cpu->env, ARM_FEATURE_V8_SM3); + cpu->id_aa64isar0 = deposit64(cpu->id_aa64isar0, 36, 4, 1); + + set_feature(&cpu->env, ARM_FEATURE_V8_SM4); + cpu->id_aa64isar0 = deposit64(cpu->id_aa64isar0, 40, 4, 1); + + set_feature(&cpu->env, ARM_FEATURE_V8_DOTPROD); + cpu->id_aa64isar0 = deposit64(cpu->id_aa64isar0, 44, 4, 1); + cpu->id_isar6 = deposit32(cpu->id_isar6, 4, 4, 1); + + set_feature(&cpu->env, ARM_FEATURE_V8_FCMA); + cpu->id_aa64isar1 = deposit64(cpu->id_aa64isar1, 16, 4, 1); + cpu->id_isar5 = deposit32(cpu->id_isar5, 28, 4, 1); + #ifdef CONFIG_USER_ONLY /* We don't set these in system emulation mode for the moment, * since we don't correctly set the ID registers to advertise them, @@ -237,15 +265,7 @@ static void aarch64_max_initfn(Object *obj) * whereas the architecture requires them to be present in both if * present in either. */ - set_feature(&cpu->env, ARM_FEATURE_V8_SHA512); - set_feature(&cpu->env, ARM_FEATURE_V8_SHA3); - set_feature(&cpu->env, ARM_FEATURE_V8_SM3); - set_feature(&cpu->env, ARM_FEATURE_V8_SM4); - set_feature(&cpu->env, ARM_FEATURE_V8_ATOMICS); - set_feature(&cpu->env, ARM_FEATURE_V8_RDM); - set_feature(&cpu->env, ARM_FEATURE_V8_DOTPROD); set_feature(&cpu->env, ARM_FEATURE_V8_FP16); - set_feature(&cpu->env, ARM_FEATURE_V8_FCMA); set_feature(&cpu->env, ARM_FEATURE_SVE); /* For usermode -cpu max we can use a larger and more efficient DCZ * blocksize since we don't have to follow what the hardware does.
For the supported extensions, fill in the appropriate bits in ID_ISAR5, ID_ISAR6, ID_AA64ISAR0, ID_AA64ISAR1. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/cpu.c | 24 +++++++++++++++++------- target/arm/cpu64.c | 36 ++++++++++++++++++++++++++++-------- 2 files changed, 45 insertions(+), 15 deletions(-) -- 2.17.1