[PATCH v4 3/6] KVM: arm64: Allow guest to set the OSLK bit

[Oliver Upton]

Hi Mark,

Sorry for the delay on my end..

On Wed, Dec 15, 2021 at 4:15 AM Mark Rutland <mark.rutland at arm.com> wrote:
> > +static bool trap_oslar_el1(struct kvm_vcpu *vcpu,
> > +                        struct sys_reg_params *p,
> > +                        const struct sys_reg_desc *r)
> > +{
> > +     u64 oslsr;
> > +
> > +     if (!p->is_write)
> > +             return read_from_write_only(vcpu, p, r);
> > +
> > +     /* Forward the OSLK bit to OSLSR */
> > +     oslsr = __vcpu_sys_reg(vcpu, OSLSR_EL1) & ~SYS_OSLSR_OSLK;
> > +     if (p->regval & SYS_OSLAR_OSLK)
> > +             oslsr |= SYS_OSLSR_OSLK;
> > +
> > +     __vcpu_sys_reg(vcpu, OSLSR_EL1) = oslsr;
> > +     return true;
> > +}
>
> Does changing this affect existing userspace? Previosuly it could read
> OSLAR_EL1 as 0, whereas now that should be rejected.
>
> That might be fine, and if so, it would be good to call that out in the commit
> message.

I do not believe we expose OSLAR_EL1 to userspace. Attempts to read it
return -ENOENT. The access will go through get_invariant_sys_reg(),
which cannot find a corresponding entry in the invariant_sys_regs
array.

[...]

> > @@ -309,9 +331,14 @@ static int set_oslsr_el1(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> >       if (err)
> >               return err;
> >
> > -     if (val != rd->val)
> > +     /*
> > +      * The only modifiable bit is the OSLK bit. Refuse the write if
> > +      * userspace attempts to change any other bit in the register.
> > +      */
> > +     if ((val & ~SYS_OSLSR_OSLK) != SYS_OSLSR_OSLM)
> >               return -EINVAL;
>
> How about:
>
>         if ((val ^ rd->val) & ~SYS_OSLSR_OSLK)
>                 return -EINVAL;
>
> ... so that we don't need to hard-code the expected value here, and can more
> easily change it in future?

Nice and clean. Thanks!

--
Best,
Oliver