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

Mark Brown posted 5 patches 1 year, 10 months ago
[PATCH v6 2/5] KVM: arm64: Add newly allocated ID registers to register descriptions
Posted by Mark Brown 1 year, 10 months ago
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.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/kvm/sys_regs.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index c9f4f387155f..a3c20d1a36aa 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -2293,12 +2293,15 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 		   ID_AA64PFR0_EL1_AdvSIMD |
 		   ID_AA64PFR0_EL1_FP), },
 	ID_SANITISED(ID_AA64PFR1_EL1),
-	ID_UNALLOCATED(4,2),
+	ID_WRITABLE(ID_AA64PFR2_EL1, ~(ID_AA64PFR2_EL1_RES0 |
+				       ID_AA64PFR2_EL1_MTEFAR |
+				       ID_AA64PFR2_EL1_MTESTOREONLY |
+				       ID_AA64PFR2_EL1_MTEPERM)),
 	ID_UNALLOCATED(4,3),
 	ID_WRITABLE(ID_AA64ZFR0_EL1, ~ID_AA64ZFR0_EL1_RES0),
 	ID_HIDDEN(ID_AA64SMFR0_EL1),
 	ID_UNALLOCATED(4,6),
-	ID_UNALLOCATED(4,7),
+	ID_WRITABLE(ID_AA64FPFR0_EL1, ~ID_AA64FPFR0_EL1_RES0),
 
 	/* CRm=5 */
 	{ SYS_DESC(SYS_ID_AA64DFR0_EL1),
@@ -2325,7 +2328,9 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	ID_WRITABLE(ID_AA64ISAR2_EL1, ~(ID_AA64ISAR2_EL1_RES0 |
 					ID_AA64ISAR2_EL1_APA3 |
 					ID_AA64ISAR2_EL1_GPA3)),
-	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),

-- 
2.30.2
Re: [PATCH v6 2/5] KVM: arm64: Add newly allocated ID registers to register descriptions
Posted by Marc Zyngier 1 year, 10 months ago
On Fri, 29 Mar 2024 00:13:43 +0000,
Mark Brown <broonie@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.

> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/kvm/sys_regs.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index c9f4f387155f..a3c20d1a36aa 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -2293,12 +2293,15 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>  		   ID_AA64PFR0_EL1_AdvSIMD |
>  		   ID_AA64PFR0_EL1_FP), },
>  	ID_SANITISED(ID_AA64PFR1_EL1),
> -	ID_UNALLOCATED(4,2),
> +	ID_WRITABLE(ID_AA64PFR2_EL1, ~(ID_AA64PFR2_EL1_RES0 |
> +				       ID_AA64PFR2_EL1_MTEFAR |
> +				       ID_AA64PFR2_EL1_MTESTOREONLY |
> +				       ID_AA64PFR2_EL1_MTEPERM)),
>  	ID_UNALLOCATED(4,3),
>  	ID_WRITABLE(ID_AA64ZFR0_EL1, ~ID_AA64ZFR0_EL1_RES0),
>  	ID_HIDDEN(ID_AA64SMFR0_EL1),
>  	ID_UNALLOCATED(4,6),
> -	ID_UNALLOCATED(4,7),
> +	ID_WRITABLE(ID_AA64FPFR0_EL1, ~ID_AA64FPFR0_EL1_RES0),
>  
>  	/* CRm=5 */
>  	{ SYS_DESC(SYS_ID_AA64DFR0_EL1),
> @@ -2325,7 +2328,9 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>  	ID_WRITABLE(ID_AA64ISAR2_EL1, ~(ID_AA64ISAR2_EL1_RES0 |
>  					ID_AA64ISAR2_EL1_APA3 |
>  					ID_AA64ISAR2_EL1_GPA3)),
> -	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?
And I haven't checked whether TLBI VMALLWS2 can be trapped.

	M.

-- 
Without deviation from the norm, progress is not possible.
Re: [PATCH v6 2/5] KVM: arm64: Add newly allocated ID registers to register descriptions
Posted by Mark Brown 1 year, 10 months ago
On Sun, Mar 31, 2024 at 11:59:06AM +0100, Marc Zyngier wrote:
> Mark Brown <broonie@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?  The "no additional"
bit is referring to FEAT_CPA.

> > -	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?  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.
Re: [PATCH v6 2/5] KVM: arm64: Add newly allocated ID registers to register descriptions
Posted by Marc Zyngier 1 year, 10 months ago
On Tue, 02 Apr 2024 18:21:55 +0100,
Mark Brown <broonie@kernel.org> wrote:
> 
> On Sun, Mar 31, 2024 at 11:59:06AM +0100, Marc Zyngier wrote:
> > Mark Brown <broonie@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.
Re: [PATCH v6 2/5] KVM: arm64: Add newly allocated ID registers to register descriptions
Posted by Mark Brown 1 year, 10 months ago
On Wed, Apr 10, 2024 at 11:32:02AM +0100, Marc Zyngier wrote:
> Mark Brown <broonie@kernel.org> wrote:
> > On Sun, Mar 31, 2024 at 11:59:06AM +0100, Marc Zyngier wrote:
> > > Mark Brown <broonie@kernel.org> wrote:

> 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?

Yes, I'd missed those - they were rather buried in the XML
unfortunately.  I'd already disabled CPA in my local code, and I've also
refactored the exposure of FPMR to be in the patch that context switches
it.

> > > > -	ID_UNALLOCATED(6,3),
> > > > +	ID_WRITABLE(ID_AA64ISAR3_EL1, ~(ID_AA64ISAR2_EL1_RES0 |
> > > > +					ID_AA64ISAR3_EL1_PACM |

> > > >  	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

I see that in limit_nv_id_reg() I am missing updates to expose the new
dpISA features to nested guests.  However from a first pass it looks
like kvm_init_nv_sysregs() already handles everything I'd expect it to,
AFAICT it's handling all known trap bits?  For kvm_init_sysreg() with
HCRX AFAICT we default to having all bits 0 with explicit relaxations
for supported features (currently FEAT_MOPS, also FEAT_FPMR with this
series) meaning that I'm still unclear what exactly the updates you're
looking for are.  For SCTLR unless I'm misunderstanding things we've got
an existing issue with not initialising the res0 and res1 fields in
kvm_init_nv_sysregs() but that doesn't seem right...  

> the exception triaging infrastructure in emulate-nested.c.

Again I am really struggling to identify which specific updates you are
looking for here.

> > > 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

That's TLBI VMALLWSE1 which is a more specific instruction.  TBH I can't
remember exactly what I was looking for, I did go into the instruction
pseudocode a bit (I was going in via SYS at one point) but didn't find
anything so was also trawling sysregs looking for something.  If I'm
reading this right there are no traps?