Re: [Qemu-devel] [PATCH v3 2/5] target/arm: relax permission checks for HWCAP_CPUID registers

["Alex Bennée" <alex.bennee@linaro.org>]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 25 June 2018 at 17:00, Alex Bennée <alex.bennee@linaro.org> wrote:
>> Although technically not visible to userspace the kernel does make
>> them visible via trap and emulate. For user mode we can provide the
>> value directly but we need to relax our permission checks to do this.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  target/arm/helper.c | 14 +++++++++++++-
>>  1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index 6e6b1762e8..9d81feb124 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -5813,7 +5813,19 @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu,
>>      if (r->state != ARM_CP_STATE_AA32) {
>>          int mask = 0;
>>          switch (r->opc1) {
>> -        case 0: case 1: case 2:
>> +        case 0:
>> +#ifdef CONFIG_USER_ONLY
>> +            /* Some AArch64 CPU ID/feature are exported to userspace
>> +             * by the kernel (see HWCAP_CPUID) */
>> +            if (r->opc0 == 3 && r->crn == 0 &&
>> +                (r->crm == 0 ||
>> +                 (r->crm >= 4 && r->crm <= 7))) {
>> +                mask = PL0_R;
>> +                break;
>> +            }
>> +#endif
>> +            /* fall-through */
>> +        case 1: case 2:
>>              /* min_EL EL1 */
>>              mask = PL1_RW;
>>              break;
>
> This looks like a rather inelegant place to shove a CONFIG_USER_ONLY
> special case. Isn't there a cleaner way to do whatever this is trying
> to achieve?

Well technically those registers aren't accessible to user space and
this is a sanity check to ensure we don't accidentally make them
accessible. But it does get in the way of emulating the traps for
USER_ONLY.


>
> thanks
> -- PMM


--
Alex Bennée