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

[Marc Zyngier <maz@kernel.org>]

On Fri, 06 Dec 2024 17:43:42 +0000,
Mark Brown <broonie@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.

	M.

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