[PATCH] clocksource/drivers/arm_arch_timer: limit XGene-1 workaround

[PATCH] clocksource/drivers/arm_arch_timer: limit XGene-1 workaround

[Andre Przywara]

The AppliedMicro XGene-1 CPU has an erratum where the timer condition
would only consider TVAL, not CVAL. We currently apply a workaround when
seeing the PartNum field of MIDR_EL1 being 0x000, under the assumption
that this would match only the XGene-1 CPU model.
However even the Ampere eMAG (aka XGene-3) uses that same part number, and
only differs in the "Variant" and "Revision" fields: XGene-1's MIDR is
0x500f0000, our eMAG reports 0x503f0002. Experiments show the latter
doesn't show the faulty behaviour.

Increase the specificity of the check to only consider partnum 0x000 and
variant 0x00, to exclude the Ampere eMAG.

Fixes: 012f18850452 ("clocksource/drivers/arm_arch_timer: Work around broken CVAL implementations")
Reported-by: Ross Burton <ross.burton@arm.com>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
Hi,

I don't have access to an XGene-2 machine, so would be grateful if
anyone from Ampere could either shed some light on the applicability or
give it a test on such a machine.

And please let me know if the KVM bits require this to be split up in
two patches.

Cheers,
Andre

 arch/arm64/include/asm/cputype.h     | 3 ++-
 arch/arm64/kvm/guest.c               | 2 +-
 drivers/clocksource/arm_arch_timer.c | 5 +++--
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index 74d00feb62f03..7c7493cb571f9 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -86,7 +86,8 @@
 #define ARM_CPU_PART_NEOVERSE_N2	0xD49
 #define ARM_CPU_PART_CORTEX_A78C	0xD4B
 
-#define APM_CPU_PART_POTENZA		0x000
+#define APM_CPU_PART_XGENE		0x000
+#define APM_CPU_VAR_POTENZA		0x00
 
 #define CAVIUM_CPU_PART_THUNDERX	0x0A1
 #define CAVIUM_CPU_PART_THUNDERX_81XX	0x0A2
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 95f6945c44325..a1710e5fa72b6 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -874,7 +874,7 @@ u32 __attribute_const__ kvm_target_cpu(void)
 		break;
 	case ARM_CPU_IMP_APM:
 		switch (part_number) {
-		case APM_CPU_PART_POTENZA:
+		case APM_CPU_PART_XGENE:
 			return KVM_ARM_TARGET_XGENE_POTENZA;
 		}
 		break;
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 7dd2c615bce23..071b04f1ee730 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -836,8 +836,9 @@ static u64 __arch_timer_check_delta(void)
 		 * Note that TVAL is signed, thus has only 31 of its
 		 * 32 bits to express magnitude.
 		 */
-		MIDR_ALL_VERSIONS(MIDR_CPU_MODEL(ARM_CPU_IMP_APM,
-						 APM_CPU_PART_POTENZA)),
+		MIDR_REV_RANGE(MIDR_CPU_MODEL(ARM_CPU_IMP_APM,
+					      APM_CPU_PART_XGENE),
+			       APM_CPU_VAR_POTENZA, 0x0, 0xf),
 		{},
 	};
 
-- 
2.25.1


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

Re: [PATCH] clocksource/drivers/arm_arch_timer: limit XGene-1 workaround

[Marc Zyngier]

On Mon, 16 Oct 2023 16:31:27 +0100,
Andre Przywara <andre.przywara@arm.com> wrote:
> 
> The AppliedMicro XGene-1 CPU has an erratum where the timer condition
> would only consider TVAL, not CVAL. We currently apply a workaround when
> seeing the PartNum field of MIDR_EL1 being 0x000, under the assumption
> that this would match only the XGene-1 CPU model.
> However even the Ampere eMAG (aka XGene-3) uses that same part number, and
> only differs in the "Variant" and "Revision" fields: XGene-1's MIDR is
> 0x500f0000, our eMAG reports 0x503f0002. Experiments show the latter
> doesn't show the faulty behaviour.
> 
> Increase the specificity of the check to only consider partnum 0x000 and
> variant 0x00, to exclude the Ampere eMAG.
> 
> Fixes: 012f18850452 ("clocksource/drivers/arm_arch_timer: Work around broken CVAL implementations")
> Reported-by: Ross Burton <ross.burton@arm.com>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
> Hi,
> 
> I don't have access to an XGene-2 machine, so would be grateful if
> anyone from Ampere could either shed some light on the applicability or
> give it a test on such a machine.

I think these boxes are essentially "abandonware".

However, you can infer a couple of things from commit 0d4429301c4a and
xgene_edac_pmd_l2c_version1(), which seems to cover Variant==1 as well
as Variant==0 (and some other REVIDR shenanigans). I'm tempted to say
that XGene-2 is this Variant==1, but I have no idea wither it is
affected or not.

Given that nobody has ever seen this HW in the wild, I don't think
breaking the HW is a problem. Someone will shout if their unicorn is
broken.

> 
> And please let me know if the KVM bits require this to be split up in
> two patches.

Nah, it's just fine. This code path is now mostly unused.

> 
> Cheers,
> Andre
> 
>  arch/arm64/include/asm/cputype.h     | 3 ++-
>  arch/arm64/kvm/guest.c               | 2 +-
>  drivers/clocksource/arm_arch_timer.c | 5 +++--
>  3 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
> index 74d00feb62f03..7c7493cb571f9 100644
> --- a/arch/arm64/include/asm/cputype.h
> +++ b/arch/arm64/include/asm/cputype.h
> @@ -86,7 +86,8 @@
>  #define ARM_CPU_PART_NEOVERSE_N2	0xD49
>  #define ARM_CPU_PART_CORTEX_A78C	0xD4B
>  
> -#define APM_CPU_PART_POTENZA		0x000
> +#define APM_CPU_PART_XGENE		0x000
> +#define APM_CPU_VAR_POTENZA		0x00
>  
>  #define CAVIUM_CPU_PART_THUNDERX	0x0A1
>  #define CAVIUM_CPU_PART_THUNDERX_81XX	0x0A2
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index 95f6945c44325..a1710e5fa72b6 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -874,7 +874,7 @@ u32 __attribute_const__ kvm_target_cpu(void)
>  		break;
>  	case ARM_CPU_IMP_APM:
>  		switch (part_number) {
> -		case APM_CPU_PART_POTENZA:
> +		case APM_CPU_PART_XGENE:
>  			return KVM_ARM_TARGET_XGENE_POTENZA;
>  		}
>  		break;
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 7dd2c615bce23..071b04f1ee730 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -836,8 +836,9 @@ static u64 __arch_timer_check_delta(void)
>  		 * Note that TVAL is signed, thus has only 31 of its
>  		 * 32 bits to express magnitude.
>  		 */
> -		MIDR_ALL_VERSIONS(MIDR_CPU_MODEL(ARM_CPU_IMP_APM,
> -						 APM_CPU_PART_POTENZA)),
> +		MIDR_REV_RANGE(MIDR_CPU_MODEL(ARM_CPU_IMP_APM,
> +					      APM_CPU_PART_XGENE),
> +			       APM_CPU_VAR_POTENZA, 0x0, 0xf),
>  		{},
>  	};

Acked-by: Marc Zyngier <maz@kernel.org>

	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

Re: [PATCH] clocksource/drivers/arm_arch_timer: limit XGene-1 workaround

[Oliver Upton]

On Mon, Oct 16, 2023 at 04:31:27PM +0100, Andre Przywara wrote:
> The AppliedMicro XGene-1 CPU has an erratum where the timer condition
> would only consider TVAL, not CVAL. We currently apply a workaround when
> seeing the PartNum field of MIDR_EL1 being 0x000, under the assumption
> that this would match only the XGene-1 CPU model.
> However even the Ampere eMAG (aka XGene-3) uses that same part number, and
> only differs in the "Variant" and "Revision" fields: XGene-1's MIDR is
> 0x500f0000, our eMAG reports 0x503f0002. Experiments show the latter
> doesn't show the faulty behaviour.
> 
> Increase the specificity of the check to only consider partnum 0x000 and
> variant 0x00, to exclude the Ampere eMAG.
> 
> Fixes: 012f18850452 ("clocksource/drivers/arm_arch_timer: Work around broken CVAL implementations")
> Reported-by: Ross Burton <ross.burton@arm.com>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

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] clocksource/drivers/arm_arch_timer: limit XGene-1 workaround

[Catalin Marinas]

On Mon, 16 Oct 2023 16:31:27 +0100, Andre Przywara wrote:
> The AppliedMicro XGene-1 CPU has an erratum where the timer condition
> would only consider TVAL, not CVAL. We currently apply a workaround when
> seeing the PartNum field of MIDR_EL1 being 0x000, under the assumption
> that this would match only the XGene-1 CPU model.
> However even the Ampere eMAG (aka XGene-3) uses that same part number, and
> only differs in the "Variant" and "Revision" fields: XGene-1's MIDR is
> 0x500f0000, our eMAG reports 0x503f0002. Experiments show the latter
> doesn't show the faulty behaviour.
> 
> [...]

Applied to arm64 (for-next/misc), thanks!

[1/1] clocksource/drivers/arm_arch_timer: limit XGene-1 workaround
      https://git.kernel.org/arm64/c/851354cbd12b

-- 
Catalin


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

Re: [PATCH] clocksource/drivers/arm_arch_timer: limit XGene-1 workaround

[Daniel Lezcano]

On 18/10/2023 12:15, Catalin Marinas wrote:
> On Mon, 16 Oct 2023 16:31:27 +0100, Andre Przywara wrote:
>> The AppliedMicro XGene-1 CPU has an erratum where the timer condition
>> would only consider TVAL, not CVAL. We currently apply a workaround when
>> seeing the PartNum field of MIDR_EL1 being 0x000, under the assumption
>> that this would match only the XGene-1 CPU model.
>> However even the Ampere eMAG (aka XGene-3) uses that same part number, and
>> only differs in the "Variant" and "Revision" fields: XGene-1's MIDR is
>> 0x500f0000, our eMAG reports 0x503f0002. Experiments show the latter
>> doesn't show the faulty behaviour.
>>
>> [...]
> 
> Applied to arm64 (for-next/misc), thanks!

Noted, thanks


> [1/1] clocksource/drivers/arm_arch_timer: limit XGene-1 workaround
>        https://git.kernel.org/arm64/c/851354cbd12b
> 

-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


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