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

[Peter Maydell <peter.maydell@linaro.org>]

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?

thanks
-- PMM