[PATCH] clocksource/drivers/arm_arch_timer: limit XGene-1 workaround
Marc Zyngier
maz at kernel.org
Mon Oct 16 09:54:08 PDT 2023
On Mon, 16 Oct 2023 16:31:27 +0100,
Andre Przywara <andre.przywara at 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 at arm.com>
> Signed-off-by: Andre Przywara <andre.przywara at 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 at kernel.org>
M.
--
Without deviation from the norm, progress is not possible.
More information about the linux-arm-kernel
mailing list