Re: FEAT_CCIDX not tested when parsing ccsidr_el1

2024-03-07 Thread Fabio Estevam
Hi Łukasz,

On Wed, Mar 6, 2024 at 3:21 PM Łukasz Więcaszek
 wrote:

> I have already prepared a patch for our work related product and now I
> am thinking of mainstreaming that piece of work. What do you think
> about that?

Please send a formal patch via git send-email.


FEAT_CCIDX not tested when parsing ccsidr_el1

2024-03-06 Thread Łukasz Więcaszek
Hello Guys,

Guided by the "Sending patches" tutorial, I would like to present a
problem I have encountered and eventually discuss whether it would be
ok to submit a patch for that issue.

The problem is in ARM arch and is related to the asm function
"__asm_dcache_level" from arch/arm/cpu/armv8/cache.S file.

 line reads value of ccsidr_el1 register but does
not test against FEAT_CCIDX.
And when FEAT_CCIDX is implemented Associativity is coded on bits
[23:3] and NumSets on bits [55:32] which means that the number of
cache sets and ways is wrongly parsed and cleaning/invalidating of
cache does not work.

I have already prepared a patch for our work related product and now I
am thinking of mainstreaming that piece of work. What do you think
about that?

Briefly, patch would look like this:

diff --git a/arch/arm/cpu/armv8/cache.S b/arch/arm/cpu/armv8/cache.S
index 3fe935cf28..c9e46859b4 100644
--- a/arch/arm/cpu/armv8/cache.S
+++ b/arch/arm/cpu/armv8/cache.S
@@ -20,6 +20,7 @@
  *
  * x0: cache level
  * x1: 0 clean & invalidate, 1 invalidate only
+ * x16: FEAT_CCIDX
  * x2~x9: clobbered
  */
 .pushsection .text.__asm_dcache_level, "ax"
@@ -29,8 +30,14 @@ ENTRY(__asm_dcache_level)
isb /* sync change of cssidr_el1 */
mrs x6, ccsidr_el1  /* read the new cssidr_el1 */
ubfxx2, x6,  #0,  #3/* x2 <- log2(cache line size)-4 */
+   cbz x16, 3f /* check for FEAT_CCIDX */
+   ubfxx3, x6,  #3, #21/* x3 <- number of cache ways - 1 */
+   ubfxx4, x6, #32, #24/* x4 <- number of cache sets - 1 */
+   b   4f
+3:
ubfxx3, x6,  #3, #10/* x3 <- number of cache ways - 1 */
ubfxx4, x6, #13, #15/* x4 <- number of cache sets - 1 */
+4:
add x2, x2, #4  /* x2 <- log2(cache line size) */
clz w5, w3  /* bit position of #ways */
/* x12 <- cache level << 1 */
@@ -74,6 +81,8 @@ ENTRY(__asm_dcache_all)
ubfxx11, x10, #24, #3   /* x11 <- loc */
cbz x11, finished   /* if loc is 0, exit */
mov x15, lr
+   mrs x16, s3_0_c0_c7_2   /* read value of id_aa64mmfr2_el1*/
+   ubfxx16, x16, #20, #4   /* save FEAT_CCIDX identifier in x16 */
mov x0, #0  /* start flush at cache level 0 */
/* x0  <- cache level */
/* x10 <- clidr_el1 */


Best Regards,
Lukasz