Re: [PATCH v6 27/64] KVM: arm64: nv: Allow a sysreg to be hidden from userspace only

Alexandru Elisei
2022-02-08T14:36:19Z

Hi Marc,

On Fri, Jan 28, 2022 at 12:18:35PM +0000, Marc Zyngier wrote:
> So far, we never needed to distinguish between registers hidden
> from userspace and being hidden from a guest (they are always
> either visible to both, or hidden from both).
>
> With NV, we have the ugly case of the EL{0,1}2 registers, which
^^^^^^^^
That might be easier to parse if it is rephrased to "EL02 and EL12
registers".

> are only a view on the EL{0,1} registers. It makes absolutely no
> sense to expose them to userspace, since it already has the
> canonical view.
>
> Add a new visibility flag (REG_HIDDEN_USER) and a new helper that
> checks for it and REG_HIDDEN when checking whether to expose
> a sysreg to userspace. Subsequent patches will make use of it.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>

The patch makes sense to me, the _EL02 and _EL12 are just aliases for the
EL0 and EL1 registers, no sense in exposing both to userspace. And these
registers also only make sense when the VCPU has the EL2 feature, which is
not always the case:

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,
Alex

> ---
> arch/arm64/kvm/sys_regs.c | 6 +++---
> arch/arm64/kvm/sys_regs.h | 12 +++++++++++-
> 2 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 31d739d59f67..0c9bbe5eee5e 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -3031,7 +3031,7 @@ int kvm_arm_sys_reg_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg
> return get_invariant_sys_reg(reg->id, uaddr);
>
> /* Check for regs disabled by runtime config */
> - if (sysreg_hidden(vcpu, r))
> + if (sysreg_hidden_user(vcpu, r))
> return -ENOENT;
>
> if (r->get_user)
> @@ -3056,7 +3056,7 @@ int kvm_arm_sys_reg_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg
> return set_invariant_sys_reg(reg->id, uaddr);
>
> /* Check for regs disabled by runtime config */
> - if (sysreg_hidden(vcpu, r))
> + if (sysreg_hidden_user(vcpu, r))
> return -ENOENT;
>
> if (r->set_user)
> @@ -3127,7 +3127,7 @@ static int walk_one_sys_reg(const struct kvm_vcpu *vcpu,
> if (!(rd->reg || rd->get_user))
> return 0;
>
> - if (sysreg_hidden(vcpu, rd))
> + if (sysreg_hidden_user(vcpu, rd))
> return 0;
>
> if (!copy_reg_to_user(rd, uind))
> diff --git a/arch/arm64/kvm/sys_regs.h b/arch/arm64/kvm/sys_regs.h
> index cc0cc95a0280..850629f083a3 100644
> --- a/arch/arm64/kvm/sys_regs.h
> +++ b/arch/arm64/kvm/sys_regs.h
> @@ -78,7 +78,8 @@ struct sys_reg_desc {
> };
>
> #define REG_HIDDEN (1 << 0) /* hidden from userspace and guest */
> -#define REG_RAZ (1 << 1) /* RAZ from userspace and guest */
> +#define REG_HIDDEN_USER (1 << 1) /* hidden from userspace only */
> +#define REG_RAZ (1 << 2) /* RAZ from userspace and guest */
>
> static __printf(2, 3)
> inline void print_sys_reg_msg(const struct sys_reg_params *p,
> @@ -138,6 +139,15 @@ static inline bool sysreg_hidden(const struct kvm_vcpu *vcpu,
> return r->visibility(vcpu, r) & REG_HIDDEN;
> }
>
> +static inline bool sysreg_hidden_user(const struct kvm_vcpu *vcpu,
> + const struct sys_reg_desc *r)
> +{
> + if (likely(!r->visibility))
> + return false;
> +
> + return r->visibility(vcpu, r) & (REG_HIDDEN | REG_HIDDEN_USER);
> +}
> +
> static inline bool sysreg_visible_as_raz(const struct kvm_vcpu *vcpu,
> const struct sys_reg_desc *r)
> {
> --
> 2.30.2
>