Re: [Qemu-devel] [PATCH v3 10/10] target/arm: Convert v8.2-fp16 from feature bit to aa64pfr0 test

[Richard Henderson]

On 10/16/18 3:36 AM, Peter Maydell wrote:
> On 8 October 2018 at 22:22, Richard Henderson
> <address@hidden> wrote:
>> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
>> Signed-off-by: Richard Henderson <address@hidden>
>> ---
>>  target/arm/cpu.h           | 17 +++++++++++++++-
>>  target/arm/translate-a64.h |  1 +
>>  target/arm/translate.h     |  1 +
>>  linux-user/elfload.c       |  6 +-----
>>  target/arm/cpu64.c         |  9 ++-------
>>  target/arm/helper.c        |  2 +-
>>  target/arm/translate-a64.c | 40 +++++++++++++++++++-------------------
>>  target/arm/translate.c     |  6 +++---
>>  8 files changed, 45 insertions(+), 37 deletions(-)
>> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
>> index ee2c04a627..38e9afef3b 100644
>> --- a/target/arm/cpu64.c
>> +++ b/target/arm/cpu64.c
>> @@ -266,6 +266,8 @@ static void aarch64_max_initfn(Object *obj)
>>
>>          t = cpu->id_aa64pfr0;
>>          t = FIELD_DP64(t, ID_AA64PFR0, SVE, 1);
>> +        t = FIELD_DP64(t, ID_AA64PFR0, FP, 1);
>> +        t = FIELD_DP64(t, ID_AA64PFR0, ADVSIMD, 1);
>>          cpu->id_aa64pfr0 = t;
>>
>>          /* Replicate the same data to the 32-bit id registers.  */
>> @@ -283,13 +285,6 @@ static void aarch64_max_initfn(Object *obj)
>>          cpu->id_isar6 = u;
>>
>>  #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,
>> -         * and in some cases they're only available in AArch64 and not 
>> AArch32,
>> -         * whereas the architecture requires them to be present in both if
>> -         * present in either.
>> -         */
>> -        set_feature(&cpu->env, ARM_FEATURE_V8_FP16);
> 
> FP16 is the feature that this comment refers to about not having the
> AArch32 support present yet. So previously we only set that feature
> bit in the user-only mode. Doesn't that mean we need to only
> set the equivalent PFR0 bits in the ID register in user-only mode now?

If we do that, then we violate the SVE rule that FP16 must be present.  I think
it's more valuable to have SVE available in system mode than the more obscure
AArch64 <-> AArch32 feature correspondence.


r~