[PATCH v6 2/5] KVM: arm64: Add newly allocated ID registers to register descriptions

[Marc Zyngier]

On Tue, 02 Apr 2024 18:21:55 +0100,
Mark Brown <broonie at kernel.org> wrote:
> 
> On Sun, Mar 31, 2024 at 11:59:06AM +0100, Marc Zyngier wrote:
> > Mark Brown <broonie at kernel.org> wrote:
> 
> > > The 2023 architecture extensions have allocated some new ID registers, add
> > > them to the KVM system register descriptions so that they are visible to
> > > guests.
> 
> > > We make the newly introduced dpISA features writeable, as well as
> > > allowing writes to ID_AA64ISAR3_EL1.CPA for FEAT_CPA which only
> > > introduces straigforward new instructions with no additional
> > > architectural state or traps.
> 
> > FPMR actively gets trapped by HCRX_EL2.
> 
> Sure, I'm not clear what you're trying to say here?

I'm saying (and not trying to say) that there are traps implied by the
features that you are adding.

> The "no additional" bit is referring to FEAT_CPA.

Well, that wasn't clear to me.

And when it comes to CPA, there are additional controls in SCTLR2_ELx,
which doesn't even gets context switched for EL1. What could possibly
go wrong?

> 
> > > -	ID_UNALLOCATED(6,3),
> > > +	ID_WRITABLE(ID_AA64ISAR3_EL1, ~(ID_AA64ISAR2_EL1_RES0 |
> > > +					ID_AA64ISAR3_EL1_PACM |
> > > +					ID_AA64ISAR3_EL1_TLBIW)),
> > >  	ID_UNALLOCATED(6,4),
> > >  	ID_UNALLOCATED(6,5),
> > >  	ID_UNALLOCATED(6,6),
> 
> > Where is the code that enforces the lack of support for MTEFAR,
> > MTESTOREONLY, and MTEPERM for SCTLR_ELx, EnPACM and EnFPM in HCRX_EL2?
> 
> Could you please be more explicit regarding what you're expecting to see
> here?

I'm expecting you to add all the required masking and fine-grained
disabling of features that are not explicitly advertised to the guest.

This should translate into additional init code in kvm_init_sysreg(),
kvm_init_nv_sysregs() and limit_nv_id_reg(). You also should update
the exception triaging infrastructure in emulate-nested.c.

> Other than the writeability mask for the ID register I would have
> expected to need explicit code to enable new features rather than
> explicit code to keep currently unsupported features unsupported.  I'm
> sure what you're referencing will be obvious once I see it but I'm
> drawing a blank.
> 
> > And I haven't checked whether TLBI VMALLWS2 can be trapped.
> 
> I didn't see anything but I might not be aware of where to look, there
> doesn't seem to be anything for that specifically in HFGITR_EL2 or
> HFGITR2_EL2 which would be the main places I'd expect to find
> something.

That's a really odd place to look. This is a S2 invalidation
primitive, which by definition is under the sole control of EL2, and
therefore cannot be trapped by any of the FGT registers, as they only
affect lesser-privileged ELs.

The instruction is described in the XML:

https://developer.arm.com/documentation/ddi0601/2024-03/AArch64-Instructions/TLBI-VMALLWS2E1--TLB-Invalidate-stage-2-dirty-state-by-VMID--EL1-0

	M.

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