[PATCH 1/1] target/arm: calculate cache sizes properly

[PATCH 1/1] target/arm: calculate cache sizes properly

[Marcin Juszkiewicz]

Neoverse-V1 TRM says that NumSets are [27:13] not :32 like in code.

With this fix all cores which use make_ccsidr64() function have proper
size of cpu caches.

Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
---
 target/arm/tcg/cpu64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/tcg/cpu64.c b/target/arm/tcg/cpu64.c
index fe232eb306..98144e1d20 100644
--- a/target/arm/tcg/cpu64.c
+++ b/target/arm/tcg/cpu64.c
@@ -50,7 +50,7 @@ static uint64_t make_ccsidr64(unsigned assoc, unsigned linesize,
     sets = cachesize / (assoc * linesize);
     assert(cachesize % (assoc * linesize) == 0);
 
-    return ((uint64_t)(sets - 1) << 32)
+    return ((uint64_t)(sets - 1) << 13)
          | ((assoc - 1) << 3)
          | (lg_linesize - 4);
 }
-- 
2.45.2

Re: [PATCH 1/1] target/arm: calculate cache sizes properly

[Peter Maydell]

On Wed, 10 Jul 2024 at 18:35, Marcin Juszkiewicz
<marcin.juszkiewicz@linaro.org> wrote:
>
> Neoverse-V1 TRM says that NumSets are [27:13] not :32 like in code.

NumSets in fields [27:13] is the 32-bit CCSIDR_EL1 format
(i.e. what you have when FEAT_CCIDX is not implemented).
The make_ccsidr64() function provides the 64-bit CCSIDR_EL1
format (i.e. the one when FEAT_CCIDX is implemented).

I would suspect this is an accidental error in the Neoverse-V1
TRM -- if you have access to the real hardware and can dump
the CCSIDR_EL1 value I would expect you'll see it matches
what QEMU is implementing here (and that ID_AA64MMFR2_EL1.CCIDX
also says that FEAT_CCIDX is implemented). See also the comment
"The Neoverse-V1 r1p2 TRM lists 32-bit format CCSIDR_EL1 values"
in aarch64_neoverse_v1_initfn() where we document that we
don't trust what the TRM is saying here.

How did you run into this? Is there some guest software
which is assuming the old 32-bit format and not checking
ID_AA64MMFR2_EL1.CCIDX to see if it needs to use the new one?

thanks
-- PMM

Re: [PATCH 1/1] target/arm: calculate cache sizes properly

[Marcin Juszkiewicz]

On 11.07.2024 12:19, Peter Maydell wrote:
> On Wed, 10 Jul 2024 at 18:35, Marcin Juszkiewicz
> <marcin.juszkiewicz@linaro.org> wrote:
>>
>> Neoverse-V1 TRM says that NumSets are [27:13] not :32 like in code.
> 
> NumSets in fields [27:13] is the 32-bit CCSIDR_EL1 format
> (i.e. what you have when FEAT_CCIDX is not implemented).
> The make_ccsidr64() function provides the 64-bit CCSIDR_EL1
> format (i.e. the one when FEAT_CCIDX is implemented).
> 
> I would suspect this is an accidental error in the Neoverse-V1
> TRM -- if you have access to the real hardware and can dump
> the CCSIDR_EL1 value I would expect you'll see it matches
> what QEMU is implementing here (and that ID_AA64MMFR2_EL1.CCIDX
> also says that FEAT_CCIDX is implemented). See also the comment
> "The Neoverse-V1 r1p2 TRM lists 32-bit format CCSIDR_EL1 values"
> in aarch64_neoverse_v1_initfn() where we document that we
> don't trust what the TRM is saying here.
> 
> How did you run into this? Is there some guest software
> which is assuming the old 32-bit format and not checking
> ID_AA64MMFR2_EL1.CCIDX to see if it needs to use the new one?

I started adding cpu cache info into PPTT and so far the only code I 
used was old 32-bit format one:

https://openfw.io/edk2-devel/20240710-acpi65-v4-6-bc32224e4be4@linaro.org/


Will check for CCIDX and adapt proper way. But that has to wait for 
August due to vacation plans.