[PATCH v4 06/14] KVM: arm64: Set protected VM traps based on its view of feature registers

[Fuad Tabba]

Hi,

On Fri, 6 Dec 2024 at 18:22, Marc Zyngier <maz at kernel.org> wrote:
>
> On Fri, 06 Dec 2024 17:43:42 +0000,
> Mark Brown <broonie at kernel.org> wrote:
> >
> > On Fri, Dec 06, 2024 at 05:31:57PM +0000, Quentin Perret wrote:
> > > On Monday 02 Dec 2024 at 15:47:33 (+0000), Fuad Tabba wrote:
> >
> > > > - if (FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_RAS), id_aa64pfr0) <
> > > > -     ID_AA64PFR0_EL1_RAS_V1P1) {
> > > > + if (!kvm_has_feat(kvm, ID_AA64PFR0_EL1, RAS, IMP)) {
> >
> > > I'm admittedly getting a bit lost in the kvm_has_feat() macro maze, but
> > > should this be:
> >
> > >     if (!kvm_has_feat(kvm, ID_AA64PFR0_EL1, RAS, V1P1))
> >
> > > to replicate the existing check?
> >
> > kvm_has_feat() checks for a value <= the supplied constant while the
>
> Not quite:
>
> #define kvm_has_feat(kvm, id, fld, limit)                               \
>         kvm_cmp_feat(kvm, id, fld, >=, limit)
>
> > existing check is just < and the values for the field are:
> >
> > UnsignedEnum  31:28   RAS
> >       0b0000  NI
> >       0b0001  IMP
> >       0b0010  V1P1
> > EndEnum
> >
> > so they're both checking for the same thing.
>
> Evidently not. The original code is checking for RASv1p1, while the
> change turns that into a check for the original RAS spec.

Marc is right. This change was intentional, since the goal is to trap
if RAS isn't implemented/supported by the guest. I should comment on
this change in the commit message.

Thanks,
/fuad


>         M.
>
> --
> Without deviation from the norm, progress is not possible.