[PATCH v5 09/27] KVM: arm64: Make ID_AA64MMFR1_EL1 writable

[Reiji Watanabe]

Hi Oliver,

On Tue, Feb 15, 2022 at 10:53 AM Oliver Upton <oupton at google.com> wrote:
>
> Hi Reiji,
>
> On Sun, Feb 13, 2022 at 10:57:28PM -0800, Reiji Watanabe wrote:
> > This patch adds id_reg_info for ID_AA64MMFR1_EL1 to make it
> > writable by userspace.
> >
> > Hardware update of Access flag and/or Dirty state in translation
> > table needs to be disabled for the guest to let userspace set
> > ID_AA64MMFR1_EL1.HAFDBS field to a lower value. It requires trapping
> > the guest's accessing TCR_EL1, which KVM doesn't always do (in order
> > to trap it without FEAT_FGT, HCR_EL2.TVM needs to be set to 1, which
> > also traps many other virtual memory control registers).
> > So, userspace won't be allowed to modify the value of the HAFDBS field.
> >
> > Signed-off-by: Reiji Watanabe <reijiw at google.com>
> > ---
> >  arch/arm64/kvm/sys_regs.c | 30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> >
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index 4ed15ae7f160..1c137f8c236f 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -570,6 +570,30 @@ static int validate_id_aa64mmfr0_el1(struct kvm_vcpu *vcpu,
> >       return 0;
> >  }
> >
> > +static int validate_id_aa64mmfr1_el1(struct kvm_vcpu *vcpu,
> > +                                  const struct id_reg_info *id_reg, u64 val)
> > +{
> > +     u64 limit = id_reg->vcpu_limit_val;
> > +     unsigned int hafdbs, lim_hafdbs;
> > +
> > +     hafdbs = cpuid_feature_extract_unsigned_field(val, ID_AA64MMFR1_HADBS_SHIFT);
> > +     lim_hafdbs = cpuid_feature_extract_unsigned_field(limit, ID_AA64MMFR1_HADBS_SHIFT);
> > +
> > +     /*
> > +      * Don't allow userspace to modify the value of HAFDBS.
> > +      * Hardware update of Access flag and/or Dirty state in translation
> > +      * table needs to be disabled for the guest to let userspace set
> > +      * HAFDBS field to a lower value. It requires trapping the guest's
> > +      * accessing TCR_EL1, which KVM doesn't always do (in order to trap
> > +      * it without FEAT_FGT, HCR_EL2.TVM needs to be set to 1, which also
> > +      * traps many other virtual memory control registers).
> > +      */
> > +     if (hafdbs != lim_hafdbs)
> > +             return -EINVAL;
>
> Are we going to require that any hidden feature be trappable going
> forward? It doesn't seem to me like there's much risk to userspace
> hiding any arbitrary feature so long as identity remains architectural.

That wasn't my intention, and I will drop this patch.
(I wonder why I thought this was needed...)

Thank you for catching this !

Regards,
Reiji

>
> Another example of this is AArch32 at EL0. Without FGT, there is no
> precise trap for KVM to intervene and prevent an AArch32 EL0.
> Nonetheless, userspace might still want to hide this from its guests
> even if a misbehaved guest could still use it.
>
> --
> Thanks,
> Oliver