Hi Oliver,
On Tue, Feb 15, 2022 at 10:53 AM Oliver Upton <oupton@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > 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
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
