Re: [PATCH 4/6] KVM: arm64: Add a visibility bit to ignore user writes

Reiji Watanabe
2022-08-31T03:30:01Z

Hi Oliver,

On Wed, Aug 17, 2022 at 2:48 PM Oliver Upton <oliver.upton@linux.dev> wrote:
>
> We're about to ignore writes to AArch32 ID registers on AArch64-only
> systems. Add a bit to indicate a register is handled as write ignore
> when accessed from userspace.
>
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
> arch/arm64/kvm/sys_regs.c | 3 +++
> arch/arm64/kvm/sys_regs.h | 7 +++++++
> 2 files changed, 10 insertions(+)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 26210f3a0b27..9f06c85f26b8 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1232,6 +1232,9 @@ static int set_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> {
> bool raz = sysreg_visible_as_raz(vcpu, rd);
>
> + if (sysreg_user_write_ignore(vcpu, rd))
> + return 0;

Since the visibility flags are not ID register specific,
have you considered checking REG_USER_WI from kvm_sys_reg_set_user()
rather than the ID register specific function ?

This patch made me reconsider my comment for the patch-2.
Perhaps it might be more appropriate to check RAZ visibility from
kvm_sys_reg_get_user() rather than the ID register specific function ?

REG_HIDDEN is already checked from kvm_sys_reg_{s,g}et_user() (indirectly).

Thank you,
Reiji

> +
> /* This is what we mean by invariant: you can't change it. */
> if (val != read_id_reg(vcpu, rd, raz))
> return -EINVAL;
> diff --git a/arch/arm64/kvm/sys_regs.h b/arch/arm64/kvm/sys_regs.h
> index e78b51059622..e4ebb3a379fd 100644
> --- a/arch/arm64/kvm/sys_regs.h
> +++ b/arch/arm64/kvm/sys_regs.h
> @@ -86,6 +86,7 @@ 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_USER_WI (1 << 2) /* WI from userspace only */
>
> static __printf(2, 3)
> inline void print_sys_reg_msg(const struct sys_reg_params *p,
> @@ -157,6 +158,12 @@ static inline bool sysreg_visible_as_raz(const struct kvm_vcpu *vcpu,
> return sysreg_visibility(vcpu, r) & REG_RAZ;
> }
>
> +static inline bool sysreg_user_write_ignore(const struct kvm_vcpu *vcpu,
> + const struct sys_reg_desc *r)
> +{
> + return sysreg_visibility(vcpu, r) & REG_USER_WI;
> +}
> +
> static inline int cmp_sys_reg(const struct sys_reg_desc *i1,
> const struct sys_reg_desc *i2)
> {
> --
> 2.37.1.595.g718a3a8f04-goog
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm