[PATCH] arm64/cache: Fix cache_type_cwg() for register generation

[Mark Rutland]

On Wed, Aug 17, 2022 at 05:56:24PM +0100, Mark Rutland wrote:
> On Wed, Aug 17, 2022 at 05:02:46PM +0100, Mark Brown wrote:
> > Ard noticed that since we converted CTR_EL0 to automatic generation we have
> > been seeing errors on some systems handling the value of cache_type_cwg()
> > such as
> > 
> >    CPU features: No Cache Writeback Granule information, assuming 128
> > 
> > This is because the manual definition of CTR_EL0_CWG_MASK was done without
> > a shift while our convention is to define the mask after shifting. This
> > means that the user in cache_type_cwg() was broken as it was written for
> > the manually written shift then mask. Fix this by reversing, we don't use
> > SYS_FIELD_GET() since that is defined in sysreg.h which we don't want to
> > include in cache.h.
> > 
> > The only other field where the _MASK for this register is used is IminLine
> > which is at offset 0 so unaffected.
> > 
> > Fixes: 9a3634d02301 ("arm64/sysreg: Convert CTR_EL0 to automatic generation")
> > Reported-by: Ard Biesheuvel <ardb at kernel.org>
> > Signed-off-by: Mark Brown <broonie at kernel.org>
> > ---
> >  arch/arm64/include/asm/cache.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/include/asm/cache.h b/arch/arm64/include/asm/cache.h
> > index 119e4aa02eb1..8445a3213b2f 100644
> > --- a/arch/arm64/include/asm/cache.h
> > +++ b/arch/arm64/include/asm/cache.h
> > @@ -67,7 +67,7 @@ static __always_inline int icache_is_vpipt(void)
> >  
> >  static inline u32 cache_type_cwg(void)
> >  {
> > -	return (read_cpuid_cachetype() >> CTR_EL0_CWG_SHIFT) & CTR_EL0_CWG_MASK;
> > +	return (read_cpuid_cachetype() & CTR_EL0_CWG_MASK) >> CTR_EL0_CWG_SHIFT;
> >  }
> 
> Can we follow the example of:
> 
>   5b345e39d3ebc213 ("arm64/sysreg: Standardise naming for CTR_EL0 fields")
> 
> ... and instead have:
> 
> | #define CTR_CWG(ctr)          SYS_FIELD_GET(CTR_EL0, CWG, ctr)
> 
> ... and:
> 
> | static inline u32 cache_type_cwg(void)
> | {
> | 	return CTR_CWG(read_cpuid_cachetype());
> | }
> 
> ... since AFAICT we had SYS_FIELD_GET() available in commit 
> 
>   9a3634d02301 ("arm64/sysreg: Convert CTR_EL0 to automatic generation")
> 
> ... and then it's all consistent and there's no confusion about how to apply
> MASK and SHIFT.
> 
> With that:
> 
>   Acked-by: Mark Rutland <mark.rutland at arm.com>

Ah; I see that blows up because we're missing an include for
<linux/bitfield.h>. We'll need to fix sysreg.h too, e.g. as below.

Mark.

---->8----
diff --git a/arch/arm64/include/asm/cache.h b/arch/arm64/include/asm/cache.h
index ca9b487112ccb..794fe2652cded 100644
--- a/arch/arm64/include/asm/cache.h
+++ b/arch/arm64/include/asm/cache.h
@@ -50,6 +50,7 @@ static inline unsigned int arch_slab_minalign(void)
         CTR_EL0_IMINLINE_MASK << CTR_EL0_IMINLINE_SHIFT)
 
 #define CTR_L1IP(ctr)          SYS_FIELD_GET(CTR_EL0, L1Ip, ctr)
+#define CTR_CWG(ctr)           SYS_FIELD_GET(CTR_EL0, CWG, ctr)
 
 #define ICACHEF_ALIASING       0
 #define ICACHEF_VPIPT          1
@@ -71,7 +72,7 @@ static __always_inline int icache_is_vpipt(void)
 
 static inline u32 cache_type_cwg(void)
 {
-       return (read_cpuid_cachetype() >> CTR_EL0_CWG_SHIFT) & CTR_EL0_CWG_MASK;
+       return CTR_CWG(read_cpuid_cachetype());
 }
 
 #define __read_mostly __section(".data..read_mostly")
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 7c71358d44c4a..7d806d1769d84 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -1116,6 +1116,7 @@
 
 #else
 
+#include <linux/bitfield.h>
 #include <linux/build_bug.h>
 #include <linux/types.h>
 #include <asm/alternative.h>
@@ -1209,8 +1210,6 @@
        par;                                                            \
 })
 
-#endif
-
 #define SYS_FIELD_GET(reg, field, val)         \
                 FIELD_GET(reg##_##field##_MASK, val)
 
@@ -1220,4 +1219,6 @@
 #define SYS_FIELD_PREP_ENUM(reg, field, val)           \
                 FIELD_PREP(reg##_##field##_MASK, reg##_##field##_##val)
 
+#endif /* __ASSEMBLY__ */
+
 #endif /* __ASM_SYSREG_H */