[PATCH 0/2] KVM/arm64 fixes for NV PAC support

[PATCH 0/2] KVM/arm64 fixes for NV PAC support

[Marc Zyngier]

Here's a small series of fixes following the introduction of NV PAuth
support in 6.10:

- address the relative priorities of Instruction abort, Illegal
  Execution state, and PAC failure (already posted)

- Expose BTI to a NV guest now that we have PAC up and running

Unless someone shouts, I'll take these patches in the next batch of
fixes.

	M.

Marc Zyngier (2):
  KVM: arm64: nv: Fix relative priorities of exceptions generated by
    ERETAx
  KVM: arm64: nv: Expose BTI and CSV_frac to a guest hypervisor

 arch/arm64/kvm/emulate-nested.c | 21 +++++++++++----------
 arch/arm64/kvm/nested.c         |  6 ++++--
 2 files changed, 15 insertions(+), 12 deletions(-)

-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/2] KVM: arm64: nv: Fix relative priorities of exceptions generated by ERETAx

[Marc Zyngier]

ERETAx can fail in multiple ways:

(1) ELR_EL2 points lalaland
(2) we get a PAC failure
(3) SPSR_EL2 has the wrong mode

(1) is easy, as we just let the CPU do its thing and deliver an
Instruction Abort. However, (2) and (3) are interesting, because
the PAC failure priority is way below that of the Illegal Execution
State exception.

Which means that if we have detected a PAC failure (and that we have
FPACCOMBINE), we must be careful to give priority to the Illegal
Execution State exception, should one be pending.

Solving this involves hoisting the SPSR calculation earlier and
testing for the IL bit before injecting the FPAC exception.

In the extreme case of a ERETAx returning to an invalid mode *and*
failing its PAC check, we end up with an Instruction Abort (due
to the new PC being mangled by the failed Auth) *and* PSTATE.IL
being set. Which matches the requirements of the architecture.

Whilst we're at it, remove a stale comment that states the obvious
and only confuses the reader.

Fixes: 213b3d1ea161 ("KVM: arm64: nv: Handle ERETA[AB] instructions")
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/emulate-nested.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
index 72d733c74a38..54090967a335 100644
--- a/arch/arm64/kvm/emulate-nested.c
+++ b/arch/arm64/kvm/emulate-nested.c
@@ -2181,16 +2181,23 @@ void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu)
 	if (forward_traps(vcpu, HCR_NV))
 		return;
 
+	spsr = vcpu_read_sys_reg(vcpu, SPSR_EL2);
+	spsr = kvm_check_illegal_exception_return(vcpu, spsr);
+
 	/* Check for an ERETAx */
 	esr = kvm_vcpu_get_esr(vcpu);
 	if (esr_iss_is_eretax(esr) && !kvm_auth_eretax(vcpu, &elr)) {
 		/*
-		 * Oh no, ERETAx failed to authenticate.  If we have
-		 * FPACCOMBINE, deliver an exception right away.  If we
-		 * don't, then let the mangled ELR value trickle down the
+		 * Oh no, ERETAx failed to authenticate.
+		 *
+		 * If we have FPACCOMBINE and we don't have a pending
+		 * Illegal Execution State exception (which has priority
+		 * over FPAC), deliver an exception right away.
+		 *
+		 * Otherwise, let the mangled ELR value trickle down the
 		 * ERET handling, and the guest will have a little surprise.
 		 */
-		if (kvm_has_pauth(vcpu->kvm, FPACCOMBINE)) {
+		if (kvm_has_pauth(vcpu->kvm, FPACCOMBINE) && !(spsr & PSR_IL_BIT)) {
 			esr &= ESR_ELx_ERET_ISS_ERETA;
 			esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_FPAC);
 			kvm_inject_nested_sync(vcpu, esr);
@@ -2201,17 +2208,11 @@ void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu)
 	preempt_disable();
 	kvm_arch_vcpu_put(vcpu);
 
-	spsr = __vcpu_sys_reg(vcpu, SPSR_EL2);
-	spsr = kvm_check_illegal_exception_return(vcpu, spsr);
 	if (!esr_iss_is_eretax(esr))
 		elr = __vcpu_sys_reg(vcpu, ELR_EL2);
 
 	trace_kvm_nested_eret(vcpu, elr, spsr);
 
-	/*
-	 * Note that the current exception level is always the virtual EL2,
-	 * since we set HCR_EL2.NV bit only when entering the virtual EL2.
-	 */
 	*vcpu_pc(vcpu) = elr;
 	*vcpu_cpsr(vcpu) = spsr;
 
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] KVM: arm64: nv: Fix relative priorities of exceptions generated by ERETAx

[Joey Gouly]

Hi Marc,

These references are from ARM DDI 0487 J.a.

On Tue, May 28, 2024 at 11:06:31AM +0100, Marc Zyngier wrote:
> ERETAx can fail in multiple ways:
> 
> (1) ELR_EL2 points lalaland
> (2) we get a PAC failure
> (3) SPSR_EL2 has the wrong mode
> 
> (1) is easy, as we just let the CPU do its thing and deliver an
> Instruction Abort. However, (2) and (3) are interesting, because
> the PAC failure priority is way below that of the Illegal Execution
> State exception.
> 
> Which means that if we have detected a PAC failure (and that we have
> FPACCOMBINE), we must be careful to give priority to the Illegal
> Execution State exception, should one be pending.

This is IZFGJP Prioritization of Synchronous exceptions taken to AArch64 state.

> 
> Solving this involves hoisting the SPSR calculation earlier and
> testing for the IL bit before injecting the FPAC exception.
> 
> In the extreme case of a ERETAx returning to an invalid mode *and*
> failing its PAC check, we end up with an Instruction Abort (due
> to the new PC being mangled by the failed Auth) *and* PSTATE.IL
> being set. Which matches the requirements of the architecture.

And this is IGPPXQ, which says "which causes the next instruction
to generate an Illegal State exception. The exception return instruction does not generate the exception."

Which matches since Instruction Abort has a higher priority than Illegal Exception.

> 
> Whilst we're at it, remove a stale comment that states the obvious
> and only confuses the reader.
> 
> Fixes: 213b3d1ea161 ("KVM: arm64: nv: Handle ERETA[AB] instructions")
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/emulate-nested.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> index 72d733c74a38..54090967a335 100644
> --- a/arch/arm64/kvm/emulate-nested.c
> +++ b/arch/arm64/kvm/emulate-nested.c
> @@ -2181,16 +2181,23 @@ void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu)
>  	if (forward_traps(vcpu, HCR_NV))
>  		return;
>  
> +	spsr = vcpu_read_sys_reg(vcpu, SPSR_EL2);
> +	spsr = kvm_check_illegal_exception_return(vcpu, spsr);
> +
>  	/* Check for an ERETAx */
>  	esr = kvm_vcpu_get_esr(vcpu);
>  	if (esr_iss_is_eretax(esr) && !kvm_auth_eretax(vcpu, &elr)) {
>  		/*
> -		 * Oh no, ERETAx failed to authenticate.  If we have
> -		 * FPACCOMBINE, deliver an exception right away.  If we
> -		 * don't, then let the mangled ELR value trickle down the
> +		 * Oh no, ERETAx failed to authenticate.
> +		 *
> +		 * If we have FPACCOMBINE and we don't have a pending
> +		 * Illegal Execution State exception (which has priority
> +		 * over FPAC), deliver an exception right away.
> +		 *
> +		 * Otherwise, let the mangled ELR value trickle down the
>  		 * ERET handling, and the guest will have a little surprise.
>  		 */
> -		if (kvm_has_pauth(vcpu->kvm, FPACCOMBINE)) {
> +		if (kvm_has_pauth(vcpu->kvm, FPACCOMBINE) && !(spsr & PSR_IL_BIT)) {
>  			esr &= ESR_ELx_ERET_ISS_ERETA;
>  			esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_FPAC);
>  			kvm_inject_nested_sync(vcpu, esr);
> @@ -2201,17 +2208,11 @@ void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu)
>  	preempt_disable();
>  	kvm_arch_vcpu_put(vcpu);
>  
> -	spsr = __vcpu_sys_reg(vcpu, SPSR_EL2);
> -	spsr = kvm_check_illegal_exception_return(vcpu, spsr);
>  	if (!esr_iss_is_eretax(esr))
>  		elr = __vcpu_sys_reg(vcpu, ELR_EL2);
>  
>  	trace_kvm_nested_eret(vcpu, elr, spsr);
>  
> -	/*
> -	 * Note that the current exception level is always the virtual EL2,
> -	 * since we set HCR_EL2.NV bit only when entering the virtual EL2.
> -	 */
>  	*vcpu_pc(vcpu) = elr;
>  	*vcpu_cpsr(vcpu) = spsr;
>  

Those references were just me checking it, not suggesting you edit the commit message.

Reviewed-by: Joey Gouly <joey.gouly@arm.com>

Thanks,
Joey

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] KVM: arm64: nv: Fix relative priorities of exceptions generated by ERETAx

[Marc Zyngier]

Hi Joey,

On Wed, 29 May 2024 15:27:27 +0100,
Joey Gouly <joey.gouly@arm.com> wrote:
> 
> Hi Marc,
> 
> These references are from ARM DDI 0487 J.a.

Ah, you have an upgrade pending! ;-)

> 
> On Tue, May 28, 2024 at 11:06:31AM +0100, Marc Zyngier wrote:
> > ERETAx can fail in multiple ways:
> > 
> > (1) ELR_EL2 points lalaland
> > (2) we get a PAC failure
> > (3) SPSR_EL2 has the wrong mode
> > 
> > (1) is easy, as we just let the CPU do its thing and deliver an
> > Instruction Abort. However, (2) and (3) are interesting, because
> > the PAC failure priority is way below that of the Illegal Execution
> > State exception.
> > 
> > Which means that if we have detected a PAC failure (and that we have
> > FPACCOMBINE), we must be careful to give priority to the Illegal
> > Execution State exception, should one be pending.
> 
> This is IZFGJP Prioritization of Synchronous exceptions taken to
> AArch64 state.
>

Indeed.

> > 
> > Solving this involves hoisting the SPSR calculation earlier and
> > testing for the IL bit before injecting the FPAC exception.
> > 
> > In the extreme case of a ERETAx returning to an invalid mode *and*
> > failing its PAC check, we end up with an Instruction Abort (due
> > to the new PC being mangled by the failed Auth) *and* PSTATE.IL
> > being set. Which matches the requirements of the architecture.
> 
> And this is IGPPXQ, which says "which causes the next instruction to
> generate an Illegal State exception. The exception return
> instruction does not generate the exception."
> 
> Which matches since Instruction Abort has a higher priority than
> Illegal Exception.

Spot on. Although it is a bit surprising that in all cases, the CPU
has to attempt fetching the next instruction before delivering the IL
exception so that it can prioritise the Abort. I guess it simplifies
some HW implementations, but makes SW suffer a bit.

> 
> > 
> > Whilst we're at it, remove a stale comment that states the obvious
> > and only confuses the reader.
> > 
> > Fixes: 213b3d1ea161 ("KVM: arm64: nv: Handle ERETA[AB] instructions")
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arch/arm64/kvm/emulate-nested.c | 21 +++++++++++----------
> >  1 file changed, 11 insertions(+), 10 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> > index 72d733c74a38..54090967a335 100644
> > --- a/arch/arm64/kvm/emulate-nested.c
> > +++ b/arch/arm64/kvm/emulate-nested.c
> > @@ -2181,16 +2181,23 @@ void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu)
> >  	if (forward_traps(vcpu, HCR_NV))
> >  		return;
> >  
> > +	spsr = vcpu_read_sys_reg(vcpu, SPSR_EL2);
> > +	spsr = kvm_check_illegal_exception_return(vcpu, spsr);
> > +
> >  	/* Check for an ERETAx */
> >  	esr = kvm_vcpu_get_esr(vcpu);
> >  	if (esr_iss_is_eretax(esr) && !kvm_auth_eretax(vcpu, &elr)) {
> >  		/*
> > -		 * Oh no, ERETAx failed to authenticate.  If we have
> > -		 * FPACCOMBINE, deliver an exception right away.  If we
> > -		 * don't, then let the mangled ELR value trickle down the
> > +		 * Oh no, ERETAx failed to authenticate.
> > +		 *
> > +		 * If we have FPACCOMBINE and we don't have a pending
> > +		 * Illegal Execution State exception (which has priority
> > +		 * over FPAC), deliver an exception right away.
> > +		 *
> > +		 * Otherwise, let the mangled ELR value trickle down the
> >  		 * ERET handling, and the guest will have a little surprise.
> >  		 */
> > -		if (kvm_has_pauth(vcpu->kvm, FPACCOMBINE)) {
> > +		if (kvm_has_pauth(vcpu->kvm, FPACCOMBINE) && !(spsr & PSR_IL_BIT)) {
> >  			esr &= ESR_ELx_ERET_ISS_ERETA;
> >  			esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_FPAC);
> >  			kvm_inject_nested_sync(vcpu, esr);
> > @@ -2201,17 +2208,11 @@ void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu)
> >  	preempt_disable();
> >  	kvm_arch_vcpu_put(vcpu);
> >  
> > -	spsr = __vcpu_sys_reg(vcpu, SPSR_EL2);
> > -	spsr = kvm_check_illegal_exception_return(vcpu, spsr);
> >  	if (!esr_iss_is_eretax(esr))
> >  		elr = __vcpu_sys_reg(vcpu, ELR_EL2);
> >  
> >  	trace_kvm_nested_eret(vcpu, elr, spsr);
> >  
> > -	/*
> > -	 * Note that the current exception level is always the virtual EL2,
> > -	 * since we set HCR_EL2.NV bit only when entering the virtual EL2.
> > -	 */
> >  	*vcpu_pc(vcpu) = elr;
> >  	*vcpu_cpsr(vcpu) = spsr;
> >  
> 
> Those references were just me checking it, not suggesting you edit
> the commit message.

Well, that's excellent detective work!

>
> Reviewed-by: Joey Gouly <joey.gouly@arm.com>

Thanks again,

	M.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] KVM: arm64: nv: Expose BTI and CSV_frac to a guest hypervisor

[Marc Zyngier]

Now that we expose PAC to NV guests, we can also expose BTI (as
the two as joined at the hip, due to some of the PAC instructions
being landing pads).

While we're at it, also propagate CSV_frac, which requires no
particular emulation.

Fixes: f4f6a95bac49 ("KVM: arm64: nv: Advertise support for PAuth")
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/nested.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 6813c7c7f00a..bae8536cbf00 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -58,8 +58,10 @@ static u64 limit_nv_id_reg(u32 id, u64 val)
 		break;
 
 	case SYS_ID_AA64PFR1_EL1:
-		/* Only support SSBS */
-		val &= NV_FTR(PFR1, SSBS);
+		/* Only support BTI, SSBS, CSV2_frac */
+		val &= (NV_FTR(PFR1, BT)	|
+			NV_FTR(PFR1, SSBS)	|
+			NV_FTR(PFR1, CSV2_frac));
 		break;
 
 	case SYS_ID_AA64MMFR0_EL1:
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] KVM/arm64 fixes for NV PAC support

[Oliver Upton]

On Tue, May 28, 2024 at 11:06:30AM +0100, Marc Zyngier wrote:
> Here's a small series of fixes following the introduction of NV PAuth
> support in 6.10:
> 
> - address the relative priorities of Instruction abort, Illegal
>   Execution state, and PAC failure (already posted)
> 
> - Expose BTI to a NV guest now that we have PAC up and running
> 
> Unless someone shouts, I'll take these patches in the next batch of
> fixes.
> 

Reviewed-by: Oliver Upton <oliver.upton@linux.dev>

-- 
Thanks,
Oliver

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] KVM/arm64 fixes for NV PAC support

[Marc Zyngier]

On Tue, 28 May 2024 11:06:30 +0100, Marc Zyngier wrote:
> Here's a small series of fixes following the introduction of NV PAuth
> support in 6.10:
> 
> - address the relative priorities of Instruction abort, Illegal
>   Execution state, and PAC failure (already posted)
> 
> - Expose BTI to a NV guest now that we have PAC up and running
> 
> [...]

Applied to fixes, thanks!

[1/2] KVM: arm64: nv: Fix relative priorities of exceptions generated by ERETAx
      commit: 41011e2de3480f9adb6420b35b67628b2d903355
[2/2] KVM: arm64: nv: Expose BTI and CSV_frac to a guest hypervisor
      commit: 47eb2d68d10208e6a9e89d10b66018e8d6ca0623

Cheers,

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



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel