All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Zyngier <maz@kernel.org>
To: James Morse <james.morse@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Anshuman Khandual <anshuman.khandual@arm.com>,
	Will Deacon <will@kernel.org>,
	kvmarm@lists.cs.columbia.edu,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 4/4] KVM: arm64: Workaround Cortex-A510's single-step and PAC trap errata
Date: Tue, 25 Jan 2022 16:51:45 +0000	[thread overview]
Message-ID: <87mtjj7o7y.wl-maz@kernel.org> (raw)
In-Reply-To: <20220125153803.549084-5-james.morse@arm.com>

Hi James,

Thanks for this. I guess.

On Tue, 25 Jan 2022 15:38:03 +0000,
James Morse <james.morse@arm.com> wrote:
> 
> Cortex-A510's erratum #2077057 causes SPSR_EL2 to be corrupted when
> single-stepping authenticated ERET instructions. A single step is
> expected, but a pointer authentication trap is taken instead. The
> erratum causes SPSR_EL1 to be copied to SPSR_EL2, which could allow
> EL1 to cause a return to EL2 with a guest controlled ELR_EL2.
> 
> Because the conditions require an ERET into active-not-pending state,
> this is only a problem for the EL2 when EL2 is stepping EL1. In this case
> the previous SPSR_EL2 value is preserved in struct kvm_vcpu, and can be
> restored.

Urgh. That's pretty nasty :-(.

> 
> Cc: stable@vger.kernel.org # ${GITHASHHERE}: arm64: Add Cortex-A510 CPU part definition
> Cc: stable@vger.kernel.org
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
>  Documentation/arm64/silicon-errata.rst  |  2 ++
>  arch/arm64/Kconfig                      | 16 ++++++++++++++++
>  arch/arm64/kernel/cpu_errata.c          |  8 ++++++++
>  arch/arm64/kvm/hyp/include/hyp/switch.h | 24 +++++++++++++++++++++---
>  arch/arm64/tools/cpucaps                |  1 +
>  5 files changed, 48 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
> index 5342e895fb60..ac1ae34564c9 100644
> --- a/Documentation/arm64/silicon-errata.rst
> +++ b/Documentation/arm64/silicon-errata.rst
> @@ -92,6 +92,8 @@ stable kernels.
>  +----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Cortex-A77      | #1508412        | ARM64_ERRATUM_1508412       |
>  +----------------+-----------------+-----------------+-----------------------------+
> +| ARM            | Cortex-A510     | #2077057        | ARM64_ERRATUM_2077057       |
> ++----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Cortex-A710     | #2119858        | ARM64_ERRATUM_2119858       |
>  +----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Cortex-A710     | #2054223        | ARM64_ERRATUM_2054223       |
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 6978140edfa4..02b542ec18c8 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -670,6 +670,22 @@ config ARM64_ERRATUM_1508412
>  config ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>  	bool
>  
> +config ARM64_ERRATUM_2077057
> +	bool "Cortex-A510: 2077057: workaround software-step corrupting SPSR_EL2"
> +	help
> +	  This option adds the workaround for ARM Cortex-A510 erratum 2077057.
> +	  Affected Cortex-A510 may corrupt SPSR_EL2 when the a step exception is
> +	  expected, but a Pointer Authentication trap is taken instead. The
> +	  erratum causes SPSR_EL1 to be copied to SPSR_EL2, which could allow
> +	  EL1 to cause a return to EL2 with a guest controlled ELR_EL2.
> +
> +	  This can only happen when EL2 is stepping EL1.
> +
> +	  When these conditions occur, the SPSR_EL2 value is unchanged from the
> +	  previous guest entry, and can be restored from the in-memory copy.
> +
> +	  If unsure, say Y.
> +
>  config ARM64_ERRATUM_2119858
>  	bool "Cortex-A710: 2119858: workaround TRBE overwriting trace data in FILL mode"
>  	default y
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index 9e1c1aef9ebd..04a014c63251 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -597,6 +597,14 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>  		.type = ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE,
>  		CAP_MIDR_RANGE_LIST(trbe_write_out_of_range_cpus),
>  	},
> +#endif
> +#ifdef CONFIG_ARM64_ERRATUM_2077057
> +	{
> +		.desc = "ARM erratum 2077057",
> +		.capability = ARM64_WORKAROUND_2077057,
> +		.type = ARM64_CPUCAP_LOCAL_CPU_ERRATUM,
> +		ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2),
> +	},
>  #endif
>  	{
>  	}
> diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
> index 331dd10821df..93bf140cc972 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/switch.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
> @@ -409,6 +409,8 @@ static inline bool kvm_hyp_handle_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
>   */
>  static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
>  {
> +	u8 esr_ec;
> +
>  	/*
>  	 * Save PSTATE early so that we can evaluate the vcpu mode
>  	 * early on.
> @@ -421,13 +423,13 @@ static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
>  	 */
>  	early_exit_filter(vcpu, exit_code);
>  
> -	if (ARM_EXCEPTION_CODE(*exit_code) != ARM_EXCEPTION_IRQ)
> +	if (ARM_EXCEPTION_CODE(*exit_code) != ARM_EXCEPTION_IRQ) {
>  		vcpu->arch.fault.esr_el2 = read_sysreg_el2(SYS_ESR);
> +		esr_ec = kvm_vcpu_trap_get_class(vcpu);
> +	}
>  
>  	if (ARM_SERROR_PENDING(*exit_code) &&
>  	    ARM_EXCEPTION_CODE(*exit_code) != ARM_EXCEPTION_IRQ) {
> -		u8 esr_ec = kvm_vcpu_trap_get_class(vcpu);
> -
>  		/*
>  		 * HVC already have an adjusted PC, which we need to
>  		 * correct in order to return to after having injected
> @@ -440,6 +442,22 @@ static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
>  			write_sysreg_el2(read_sysreg_el2(SYS_ELR) - 4, SYS_ELR);
>  	}
>  
> +	/*
> +	 * Check for the conditions of Cortex-A510's #2077057. When these occur
> +	 * SPSR_EL2 can't be trusted, but isn't needed either as it is
> +	 * unchanged from the value in vcpu_gp_regs(vcpu)->pstate.
> +	 * Did we just take a PAC exception when a step exception was expected?
> +	 */
> +	if (IS_ENABLED(CONFIG_ARM64_ERRATUM_2077057) &&

nit: we can drop this IS_ENABLED()...

> +	    cpus_have_const_cap(ARM64_WORKAROUND_2077057) &&

and make this a final cap. Being a ARM64_CPUCAP_LOCAL_CPU_ERRATUM, we
won't accept late CPUs on a system that wasn't affected until then.

> +	    ARM_EXCEPTION_CODE(*exit_code) != ARM_EXCEPTION_IRQ &&
> +	    esr_ec == ESR_ELx_EC_PAC &&
> +	    vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) {
> +		/* Active-not-pending? */
> +		if (*vcpu_cpsr(vcpu) & DBG_SPSR_SS)
> +			write_sysreg_el2(*vcpu_cpsr(vcpu), SYS_SPSR);

Err... Isn't this way too late? The function starts with:

	vcpu->arch.ctxt.regs.pstate = read_sysreg_el2(SYS_SPSR);

which is just another way to write:

	*vcpu_cpsr(vcpu) = read_sysreg_el2(SYS_SPSR);

By that time, the vcpu's PSTATE is terminally corrupted.

I think you need to hoist this workaround way up, before we call into
early_exit_filter() as it will assume that the guest pstate is correct
(this is used by both pKVM and the NV code).

Something like this (untested):

diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index 93bf140cc972..a8a1502db237 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -402,6 +402,26 @@ static inline bool kvm_hyp_handle_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
 	return false;
 }
 
+static inline void synchronize_vcpu_pstate(struct kvm_vcpu *vcpu,
+					   u64 *exit_code)
+{
+	/*
+	 * Check for the conditions of Cortex-A510's #2077057. When these occur
+	 * SPSR_EL2 can't be trusted, but isn't needed either as it is
+	 * unchanged from the value in vcpu_gp_regs(vcpu)->pstate.
+	 * Did we just take a PAC exception when a step exception (being in the
+	 * Active-not-pending state) was expected?
+	 */
+	if (cpus_have_final_cap(ARM64_WORKAROUND_2077057)	&&
+	    ARM_EXCEPTION_CODE(*exit_code) != ARM_EXCEPTION_IRQ	&&
+	    kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_PAC	&&
+	    vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP		&&
+	    *vcpu_cpsr(vcpu) & DBG_SPSR_SS)
+		write_sysreg_el2(*vcpu_cpsr(vcpu), SYS_SPSR);
+
+	*vcpu_cpsr(vcpu) = read_sysreg_el2(SYS_SPSR);
+}
+
 /*
  * Return true when we were able to fixup the guest exit and should return to
  * the guest, false when we should restore the host state and return to the
@@ -415,7 +435,7 @@ static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
 	 * Save PSTATE early so that we can evaluate the vcpu mode
 	 * early on.
 	 */
-	vcpu->arch.ctxt.regs.pstate = read_sysreg_el2(SYS_SPSR);
+	synchronize_vcpu_pstate(vcpu, exit_code);
 
 	/*
 	 * Check whether we want to repaint the state one way or
@@ -442,22 +462,6 @@ static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
 			write_sysreg_el2(read_sysreg_el2(SYS_ELR) - 4, SYS_ELR);
 	}
 
-	/*
-	 * Check for the conditions of Cortex-A510's #2077057. When these occur
-	 * SPSR_EL2 can't be trusted, but isn't needed either as it is
-	 * unchanged from the value in vcpu_gp_regs(vcpu)->pstate.
-	 * Did we just take a PAC exception when a step exception was expected?
-	 */
-	if (IS_ENABLED(CONFIG_ARM64_ERRATUM_2077057) &&
-	    cpus_have_const_cap(ARM64_WORKAROUND_2077057) &&
-	    ARM_EXCEPTION_CODE(*exit_code) != ARM_EXCEPTION_IRQ &&
-	    esr_ec == ESR_ELx_EC_PAC &&
-	    vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) {
-		/* Active-not-pending? */
-		if (*vcpu_cpsr(vcpu) & DBG_SPSR_SS)
-			write_sysreg_el2(*vcpu_cpsr(vcpu), SYS_SPSR);
-	}
-
 	/*
 	 * We're using the raw exception code in order to only process
 	 * the trap if no SError is pending. We will come back to the

> +	}
> +
>  	/*
>  	 * We're using the raw exception code in order to only process
>  	 * the trap if no SError is pending. We will come back to the
> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
> index 870c39537dd0..2e7cd3fecca6 100644
> --- a/arch/arm64/tools/cpucaps
> +++ b/arch/arm64/tools/cpucaps
> @@ -55,6 +55,7 @@ WORKAROUND_1418040
>  WORKAROUND_1463225
>  WORKAROUND_1508412
>  WORKAROUND_1542419
> +WORKAROUND_2077057
>  WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>  WORKAROUND_TSB_FLUSH_FAILURE
>  WORKAROUND_TRBE_WRITE_OUT_OF_RANGE

Other than that, I'm happy to take the series as a whole ASAP, if only
for the two pretty embarrassing bug fixes. If you can respin it
shortly and address the comments above, I'd like it into -rc2.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org>
To: James Morse <james.morse@arm.com>
Cc: kvmarm@lists.cs.columbia.edu,
	linux-arm-kernel@lists.infradead.org,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Anshuman Khandual <anshuman.khandual@arm.com>
Subject: Re: [PATCH 4/4] KVM: arm64: Workaround Cortex-A510's single-step and PAC trap errata
Date: Tue, 25 Jan 2022 16:51:45 +0000	[thread overview]
Message-ID: <87mtjj7o7y.wl-maz@kernel.org> (raw)
In-Reply-To: <20220125153803.549084-5-james.morse@arm.com>

Hi James,

Thanks for this. I guess.

On Tue, 25 Jan 2022 15:38:03 +0000,
James Morse <james.morse@arm.com> wrote:
> 
> Cortex-A510's erratum #2077057 causes SPSR_EL2 to be corrupted when
> single-stepping authenticated ERET instructions. A single step is
> expected, but a pointer authentication trap is taken instead. The
> erratum causes SPSR_EL1 to be copied to SPSR_EL2, which could allow
> EL1 to cause a return to EL2 with a guest controlled ELR_EL2.
> 
> Because the conditions require an ERET into active-not-pending state,
> this is only a problem for the EL2 when EL2 is stepping EL1. In this case
> the previous SPSR_EL2 value is preserved in struct kvm_vcpu, and can be
> restored.

Urgh. That's pretty nasty :-(.

> 
> Cc: stable@vger.kernel.org # ${GITHASHHERE}: arm64: Add Cortex-A510 CPU part definition
> Cc: stable@vger.kernel.org
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
>  Documentation/arm64/silicon-errata.rst  |  2 ++
>  arch/arm64/Kconfig                      | 16 ++++++++++++++++
>  arch/arm64/kernel/cpu_errata.c          |  8 ++++++++
>  arch/arm64/kvm/hyp/include/hyp/switch.h | 24 +++++++++++++++++++++---
>  arch/arm64/tools/cpucaps                |  1 +
>  5 files changed, 48 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
> index 5342e895fb60..ac1ae34564c9 100644
> --- a/Documentation/arm64/silicon-errata.rst
> +++ b/Documentation/arm64/silicon-errata.rst
> @@ -92,6 +92,8 @@ stable kernels.
>  +----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Cortex-A77      | #1508412        | ARM64_ERRATUM_1508412       |
>  +----------------+-----------------+-----------------+-----------------------------+
> +| ARM            | Cortex-A510     | #2077057        | ARM64_ERRATUM_2077057       |
> ++----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Cortex-A710     | #2119858        | ARM64_ERRATUM_2119858       |
>  +----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Cortex-A710     | #2054223        | ARM64_ERRATUM_2054223       |
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 6978140edfa4..02b542ec18c8 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -670,6 +670,22 @@ config ARM64_ERRATUM_1508412
>  config ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>  	bool
>  
> +config ARM64_ERRATUM_2077057
> +	bool "Cortex-A510: 2077057: workaround software-step corrupting SPSR_EL2"
> +	help
> +	  This option adds the workaround for ARM Cortex-A510 erratum 2077057.
> +	  Affected Cortex-A510 may corrupt SPSR_EL2 when the a step exception is
> +	  expected, but a Pointer Authentication trap is taken instead. The
> +	  erratum causes SPSR_EL1 to be copied to SPSR_EL2, which could allow
> +	  EL1 to cause a return to EL2 with a guest controlled ELR_EL2.
> +
> +	  This can only happen when EL2 is stepping EL1.
> +
> +	  When these conditions occur, the SPSR_EL2 value is unchanged from the
> +	  previous guest entry, and can be restored from the in-memory copy.
> +
> +	  If unsure, say Y.
> +
>  config ARM64_ERRATUM_2119858
>  	bool "Cortex-A710: 2119858: workaround TRBE overwriting trace data in FILL mode"
>  	default y
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index 9e1c1aef9ebd..04a014c63251 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -597,6 +597,14 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>  		.type = ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE,
>  		CAP_MIDR_RANGE_LIST(trbe_write_out_of_range_cpus),
>  	},
> +#endif
> +#ifdef CONFIG_ARM64_ERRATUM_2077057
> +	{
> +		.desc = "ARM erratum 2077057",
> +		.capability = ARM64_WORKAROUND_2077057,
> +		.type = ARM64_CPUCAP_LOCAL_CPU_ERRATUM,
> +		ERRATA_MIDR_REV_RANGE(MIDR_CORTEX_A510, 0, 0, 2),
> +	},
>  #endif
>  	{
>  	}
> diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
> index 331dd10821df..93bf140cc972 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/switch.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
> @@ -409,6 +409,8 @@ static inline bool kvm_hyp_handle_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
>   */
>  static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
>  {
> +	u8 esr_ec;
> +
>  	/*
>  	 * Save PSTATE early so that we can evaluate the vcpu mode
>  	 * early on.
> @@ -421,13 +423,13 @@ static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
>  	 */
>  	early_exit_filter(vcpu, exit_code);
>  
> -	if (ARM_EXCEPTION_CODE(*exit_code) != ARM_EXCEPTION_IRQ)
> +	if (ARM_EXCEPTION_CODE(*exit_code) != ARM_EXCEPTION_IRQ) {
>  		vcpu->arch.fault.esr_el2 = read_sysreg_el2(SYS_ESR);
> +		esr_ec = kvm_vcpu_trap_get_class(vcpu);
> +	}
>  
>  	if (ARM_SERROR_PENDING(*exit_code) &&
>  	    ARM_EXCEPTION_CODE(*exit_code) != ARM_EXCEPTION_IRQ) {
> -		u8 esr_ec = kvm_vcpu_trap_get_class(vcpu);
> -
>  		/*
>  		 * HVC already have an adjusted PC, which we need to
>  		 * correct in order to return to after having injected
> @@ -440,6 +442,22 @@ static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
>  			write_sysreg_el2(read_sysreg_el2(SYS_ELR) - 4, SYS_ELR);
>  	}
>  
> +	/*
> +	 * Check for the conditions of Cortex-A510's #2077057. When these occur
> +	 * SPSR_EL2 can't be trusted, but isn't needed either as it is
> +	 * unchanged from the value in vcpu_gp_regs(vcpu)->pstate.
> +	 * Did we just take a PAC exception when a step exception was expected?
> +	 */
> +	if (IS_ENABLED(CONFIG_ARM64_ERRATUM_2077057) &&

nit: we can drop this IS_ENABLED()...

> +	    cpus_have_const_cap(ARM64_WORKAROUND_2077057) &&

and make this a final cap. Being a ARM64_CPUCAP_LOCAL_CPU_ERRATUM, we
won't accept late CPUs on a system that wasn't affected until then.

> +	    ARM_EXCEPTION_CODE(*exit_code) != ARM_EXCEPTION_IRQ &&
> +	    esr_ec == ESR_ELx_EC_PAC &&
> +	    vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) {
> +		/* Active-not-pending? */
> +		if (*vcpu_cpsr(vcpu) & DBG_SPSR_SS)
> +			write_sysreg_el2(*vcpu_cpsr(vcpu), SYS_SPSR);

Err... Isn't this way too late? The function starts with:

	vcpu->arch.ctxt.regs.pstate = read_sysreg_el2(SYS_SPSR);

which is just another way to write:

	*vcpu_cpsr(vcpu) = read_sysreg_el2(SYS_SPSR);

By that time, the vcpu's PSTATE is terminally corrupted.

I think you need to hoist this workaround way up, before we call into
early_exit_filter() as it will assume that the guest pstate is correct
(this is used by both pKVM and the NV code).

Something like this (untested):

diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index 93bf140cc972..a8a1502db237 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -402,6 +402,26 @@ static inline bool kvm_hyp_handle_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
 	return false;
 }
 
+static inline void synchronize_vcpu_pstate(struct kvm_vcpu *vcpu,
+					   u64 *exit_code)
+{
+	/*
+	 * Check for the conditions of Cortex-A510's #2077057. When these occur
+	 * SPSR_EL2 can't be trusted, but isn't needed either as it is
+	 * unchanged from the value in vcpu_gp_regs(vcpu)->pstate.
+	 * Did we just take a PAC exception when a step exception (being in the
+	 * Active-not-pending state) was expected?
+	 */
+	if (cpus_have_final_cap(ARM64_WORKAROUND_2077057)	&&
+	    ARM_EXCEPTION_CODE(*exit_code) != ARM_EXCEPTION_IRQ	&&
+	    kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_PAC	&&
+	    vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP		&&
+	    *vcpu_cpsr(vcpu) & DBG_SPSR_SS)
+		write_sysreg_el2(*vcpu_cpsr(vcpu), SYS_SPSR);
+
+	*vcpu_cpsr(vcpu) = read_sysreg_el2(SYS_SPSR);
+}
+
 /*
  * Return true when we were able to fixup the guest exit and should return to
  * the guest, false when we should restore the host state and return to the
@@ -415,7 +435,7 @@ static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
 	 * Save PSTATE early so that we can evaluate the vcpu mode
 	 * early on.
 	 */
-	vcpu->arch.ctxt.regs.pstate = read_sysreg_el2(SYS_SPSR);
+	synchronize_vcpu_pstate(vcpu, exit_code);
 
 	/*
 	 * Check whether we want to repaint the state one way or
@@ -442,22 +462,6 @@ static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
 			write_sysreg_el2(read_sysreg_el2(SYS_ELR) - 4, SYS_ELR);
 	}
 
-	/*
-	 * Check for the conditions of Cortex-A510's #2077057. When these occur
-	 * SPSR_EL2 can't be trusted, but isn't needed either as it is
-	 * unchanged from the value in vcpu_gp_regs(vcpu)->pstate.
-	 * Did we just take a PAC exception when a step exception was expected?
-	 */
-	if (IS_ENABLED(CONFIG_ARM64_ERRATUM_2077057) &&
-	    cpus_have_const_cap(ARM64_WORKAROUND_2077057) &&
-	    ARM_EXCEPTION_CODE(*exit_code) != ARM_EXCEPTION_IRQ &&
-	    esr_ec == ESR_ELx_EC_PAC &&
-	    vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) {
-		/* Active-not-pending? */
-		if (*vcpu_cpsr(vcpu) & DBG_SPSR_SS)
-			write_sysreg_el2(*vcpu_cpsr(vcpu), SYS_SPSR);
-	}
-
 	/*
 	 * We're using the raw exception code in order to only process
 	 * the trap if no SError is pending. We will come back to the

> +	}
> +
>  	/*
>  	 * We're using the raw exception code in order to only process
>  	 * the trap if no SError is pending. We will come back to the
> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
> index 870c39537dd0..2e7cd3fecca6 100644
> --- a/arch/arm64/tools/cpucaps
> +++ b/arch/arm64/tools/cpucaps
> @@ -55,6 +55,7 @@ WORKAROUND_1418040
>  WORKAROUND_1463225
>  WORKAROUND_1508412
>  WORKAROUND_1542419
> +WORKAROUND_2077057
>  WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>  WORKAROUND_TSB_FLUSH_FAILURE
>  WORKAROUND_TRBE_WRITE_OUT_OF_RANGE

Other than that, I'm happy to take the series as a whole ASAP, if only
for the two pretty embarrassing bug fixes. If you can respin it
shortly and address the comments above, I'd like it into -rc2.

Thanks,

	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

  reply	other threads:[~2022-01-25 16:51 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-25 15:37 [PATCH 0/4] KVM: arm64: A510 errata workaround and fixes for fixup_guest_exit() James Morse
2022-01-25 15:37 ` James Morse
2022-01-25 15:38 ` [PATCH 1/4] arm64: Add Cortex-A510 CPU part definition James Morse
2022-01-25 15:38   ` James Morse
2022-01-25 15:38 ` [PATCH 2/4] KVM: arm64: Avoid consuming a stale esr value when SError occur James Morse
2022-01-25 15:38   ` James Morse
2022-01-25 15:38 ` [PATCH 3/4] KVM: arm64: Stop handle_exit() from handling HVC twice when an SError occurs James Morse
2022-01-25 15:38   ` James Morse
2022-01-25 15:38 ` [PATCH 4/4] KVM: arm64: Workaround Cortex-A510's single-step and PAC trap errata James Morse
2022-01-25 15:38   ` James Morse
2022-01-25 16:51   ` Marc Zyngier [this message]
2022-01-25 16:51     ` Marc Zyngier
2022-01-25 18:19     ` James Morse
2022-01-25 18:19       ` James Morse
2022-01-25 18:36       ` Marc Zyngier
2022-01-25 18:36         ` Marc Zyngier
2022-01-26 16:49         ` James Morse
2022-01-26 16:49           ` James Morse

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87mtjj7o7y.wl-maz@kernel.org \
    --to=maz@kernel.org \
    --cc=anshuman.khandual@arm.com \
    --cc=catalin.marinas@arm.com \
    --cc=james.morse@arm.com \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.