[PATCH 1/2] ACPI/PPTT: Add variable to record max cache line size

[PATCH 1/2] ACPI/PPTT: Add variable to record max cache line size

[Shaokun Zhang]

Add coherency_max_size variable to record the maximum cache line size
detected from PPTT information for different cache levels. We will
synchronize it with CTR_EL0.CWG reporting in cache_line_size() for arm64.

Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Len Brown <lenb@kernel.org>
Cc: Jeremy Linton <jeremy.linton@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
To: linux-acpi@vger.kernel.org
Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
---
 drivers/acpi/pptt.c  | 7 ++++++-
 include/linux/acpi.h | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
index 065c4fc245d1..3998621e00ce 100644
--- a/drivers/acpi/pptt.c
+++ b/drivers/acpi/pptt.c
@@ -341,6 +341,8 @@ static struct acpi_pptt_cache *acpi_find_cache_node(struct acpi_table_header *ta
 	return found;
 }
 
+unsigned int coherency_max_size;
+
 /**
  * update_cache_properties() - Update cacheinfo for the given processor
  * @this_leaf: Kernel cache info structure being updated
@@ -360,8 +362,11 @@ static void update_cache_properties(struct cacheinfo *this_leaf,
 	this_leaf->fw_token = cpu_node;
 	if (found_cache->flags & ACPI_PPTT_SIZE_PROPERTY_VALID)
 		this_leaf->size = found_cache->size;
-	if (found_cache->flags & ACPI_PPTT_LINE_SIZE_VALID)
+	if (found_cache->flags & ACPI_PPTT_LINE_SIZE_VALID) {
 		this_leaf->coherency_line_size = found_cache->line_size;
+		coherency_max_size = this_leaf->coherency_line_size > coherency_max_size ?
+			this_leaf->coherency_line_size : coherency_max_size;
+	}
 	if (found_cache->flags & ACPI_PPTT_NUMBER_OF_SETS_VALID)
 		this_leaf->number_of_sets = found_cache->number_of_sets;
 	if (found_cache->flags & ACPI_PPTT_ASSOCIATIVITY_VALID)
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index d5dcebd7aad3..199cde875d5b 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -315,6 +315,7 @@ static inline bool acpi_sci_irq_valid(void)
 
 extern int sbf_port;
 extern unsigned long acpi_realmode_flags;
+extern unsigned int coherency_max_size;
 
 int acpi_register_gsi (struct device *dev, u32 gsi, int triggering, int polarity);
 int acpi_gsi_to_irq (u32 gsi, unsigned int *irq);
-- 
2.7.4


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

[PATCH 2/2] arm64: cacheinfo: Update cache_line_size detected from PPTT

[Shaokun Zhang]

cache_line_size is derived from CTR_EL0.CWG field and is called mostly
for I/O device drivers. For HiSilicon certain plantform, like the
Kunpeng920 server SoC, cache line sizes are different between L1/2
cache and L3 cache while L1 cache line size is 64-byte and L3 is 128-byte,
but CTR_EL0.CWG is misreporting using L1 cache line size.

We shall correct the right value which is important for I/O performance.
Let's update the cache line size if it is detected from PPTT information
when it is larger than CTR_EL0.CWG reporting.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Reported-by: Zhenfa Qiu <qiuzhenfa@hisilicon.com>
Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
---
 arch/arm64/include/asm/cache.h |  6 +-----
 arch/arm64/kernel/cacheinfo.c  | 15 +++++++++++++++
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/cache.h b/arch/arm64/include/asm/cache.h
index 926434f413fa..f120d48b27ac 100644
--- a/arch/arm64/include/asm/cache.h
+++ b/arch/arm64/include/asm/cache.h
@@ -91,11 +91,7 @@ static inline u32 cache_type_cwg(void)
 
 #define __read_mostly __attribute__((__section__(".data..read_mostly")))
 
-static inline int cache_line_size(void)
-{
-	u32 cwg = cache_type_cwg();
-	return cwg ? 4 << cwg : ARCH_DMA_MINALIGN;
-}
+extern int cache_line_size(void);
 
 /*
  * Read the effective value of CTR_EL0.
diff --git a/arch/arm64/kernel/cacheinfo.c b/arch/arm64/kernel/cacheinfo.c
index 0bf0a835122f..0b26d53790a8 100644
--- a/arch/arm64/kernel/cacheinfo.c
+++ b/arch/arm64/kernel/cacheinfo.c
@@ -28,6 +28,21 @@
 #define CLIDR_CTYPE(clidr, level)	\
 	(((clidr) & CLIDR_CTYPE_MASK(level)) >> CLIDR_CTYPE_SHIFT(level))
 
+int cache_line_size(void)
+{
+	u32 cwg = cache_type_cwg();
+
+	if (cwg == 0)
+		return ARCH_DMA_MINALIGN;
+#ifdef CONFIG_ACPI
+	/* compare cache line size detected from PPTT with CWG reporting */
+	if (coherency_max_size > (4 << cwg))
+		return coherency_max_size;
+#endif
+
+	return 4 << cwg;
+}
+
 static inline enum cache_type get_cache_type(int level)
 {
 	u64 clidr;
-- 
2.7.4


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

Re: [PATCH 1/2] ACPI/PPTT: Add variable to record max cache line size

[Jeremy Linton]

Hi,


On 4/25/19 8:40 PM, Shaokun Zhang wrote:
> Add coherency_max_size variable to record the maximum cache line size
> detected from PPTT information for different cache levels. We will
> synchronize it with CTR_EL0.CWG reporting in cache_line_size() for arm64.

Is the expectation that the largest cache line size will differ from the 
LLC line size?

Also, will it remain consistent across all PE's? I believe this is 
required, Yes?


> 
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Len Brown <lenb@kernel.org>
> Cc: Jeremy Linton <jeremy.linton@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> To: linux-acpi@vger.kernel.org
> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
> ---
>   drivers/acpi/pptt.c  | 7 ++++++-
>   include/linux/acpi.h | 1 +
>   2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
> index 065c4fc245d1..3998621e00ce 100644
> --- a/drivers/acpi/pptt.c
> +++ b/drivers/acpi/pptt.c
> @@ -341,6 +341,8 @@ static struct acpi_pptt_cache *acpi_find_cache_node(struct acpi_table_header *ta
>   	return found;
>   }
>   
> +unsigned int coherency_max_size;
> +
>   /**
>    * update_cache_properties() - Update cacheinfo for the given processor
>    * @this_leaf: Kernel cache info structure being updated
> @@ -360,8 +362,11 @@ static void update_cache_properties(struct cacheinfo *this_leaf,
>   	this_leaf->fw_token = cpu_node;
>   	if (found_cache->flags & ACPI_PPTT_SIZE_PROPERTY_VALID)
>   		this_leaf->size = found_cache->size;
> -	if (found_cache->flags & ACPI_PPTT_LINE_SIZE_VALID)
> +	if (found_cache->flags & ACPI_PPTT_LINE_SIZE_VALID) {
>   		this_leaf->coherency_line_size = found_cache->line_size;
> +		coherency_max_size = this_leaf->coherency_line_size > coherency_max_size ?
> +			this_leaf->coherency_line_size : coherency_max_size;
> +	}
>   	if (found_cache->flags & ACPI_PPTT_NUMBER_OF_SETS_VALID)
>   		this_leaf->number_of_sets = found_cache->number_of_sets;
>   	if (found_cache->flags & ACPI_PPTT_ASSOCIATIVITY_VALID)
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index d5dcebd7aad3..199cde875d5b 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -315,6 +315,7 @@ static inline bool acpi_sci_irq_valid(void)
>   
>   extern int sbf_port;
>   extern unsigned long acpi_realmode_flags;
> +extern unsigned int coherency_max_size;

Assuming that the LLC doesn't reflect the max cache line size, and it 
can't be pulled directly from the cpu_cacheinfo, I don't think this is 
the ideal place for this code. Given that DT has a cache line property 
as well, I suspect cache_shared_cpu_map_setup() may be a better place. 
Although, that isn't ideal either, as both these pieces of code have the 
pretense of being architecture neutral. Maybe a new call into 
arch/arm64/cacheinfo.c?


>   
>   int acpi_register_gsi (struct device *dev, u32 gsi, int triggering, int polarity);
>   int acpi_gsi_to_irq (u32 gsi, unsigned int *irq);
> 


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

Re: [PATCH 2/2] arm64: cacheinfo: Update cache_line_size detected from PPTT

[Jeremy Linton]

Hi,

On 4/25/19 8:40 PM, Shaokun Zhang wrote:
> cache_line_size is derived from CTR_EL0.CWG field and is called mostly
> for I/O device drivers. For HiSilicon certain plantform, like the

But there are core users too? Thinkgs like blk-mq, the trace ring 
buffer, iommu/iova, slub/slab. And a quick look seems to indicate a 
number of those users are going to be checking the cache line size 
before the cachinfo is populated (it happens fairly late via 
device_initcall() and a hp notifier). Is it going to be a problem if the 
value changes?


> Kunpeng920 server SoC, cache line sizes are different between L1/2
> cache and L3 cache while L1 cache line size is 64-byte and L3 is 128-byte,
> but CTR_EL0.CWG is misreporting using L1 cache line size.
> 
> We shall correct the right value which is important for I/O performance.
> Let's update the cache line size if it is detected from PPTT information
> when it is larger than CTR_EL0.CWG reporting.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Reported-by: Zhenfa Qiu <qiuzhenfa@hisilicon.com>
> Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
> ---
>   arch/arm64/include/asm/cache.h |  6 +-----
>   arch/arm64/kernel/cacheinfo.c  | 15 +++++++++++++++
>   2 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/cache.h b/arch/arm64/include/asm/cache.h
> index 926434f413fa..f120d48b27ac 100644
> --- a/arch/arm64/include/asm/cache.h
> +++ b/arch/arm64/include/asm/cache.h
> @@ -91,11 +91,7 @@ static inline u32 cache_type_cwg(void)
>   
>   #define __read_mostly __attribute__((__section__(".data..read_mostly")))
>   
> -static inline int cache_line_size(void)
> -{
> -	u32 cwg = cache_type_cwg();
> -	return cwg ? 4 << cwg : ARCH_DMA_MINALIGN;
> -}
> +extern int cache_line_size(void);
>   
>   /*
>    * Read the effective value of CTR_EL0.
> diff --git a/arch/arm64/kernel/cacheinfo.c b/arch/arm64/kernel/cacheinfo.c
> index 0bf0a835122f..0b26d53790a8 100644
> --- a/arch/arm64/kernel/cacheinfo.c
> +++ b/arch/arm64/kernel/cacheinfo.c
> @@ -28,6 +28,21 @@
>   #define CLIDR_CTYPE(clidr, level)	\
>   	(((clidr) & CLIDR_CTYPE_MASK(level)) >> CLIDR_CTYPE_SHIFT(level))
>   
> +int cache_line_size(void)
> +{
> +	u32 cwg = cache_type_cwg();
> +
> +	if (cwg == 0)
> +		return ARCH_DMA_MINALIGN;
> +#ifdef CONFIG_ACPI
> +	/* compare cache line size detected from PPTT with CWG reporting */
> +	if (coherency_max_size > (4 << cwg))
> +		return coherency_max_size;
> +#endif
> +
> +	return 4 << cwg;
> +}
> +
>   static inline enum cache_type get_cache_type(int level)
>   {
>   	u64 clidr;
> 


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

Re: [PATCH 2/2] arm64: cacheinfo: Update cache_line_size detected from PPTT

[Catalin Marinas]

On Fri, Apr 26, 2019 at 12:18:33PM -0500, Jeremy Linton wrote:
> On 4/25/19 8:40 PM, Shaokun Zhang wrote:
> > cache_line_size is derived from CTR_EL0.CWG field and is called mostly
> > for I/O device drivers. For HiSilicon certain plantform, like the
> 
> But there are core users too? Thinkgs like blk-mq, the trace ring buffer,
> iommu/iova, slub/slab.

cache_line_size() is indeed used in the core parts of the kernel, for
example when passing SLAB_HWCACHE_ALIGN on kmem_cache creation. Its
meaning is performance rather than coherency as we use ARCH_DMA_MINALIGN
for the latter.

> And a quick look seems to indicate a number of those
> users are going to be checking the cache line size before the cachinfo is
> populated (it happens fairly late via device_initcall() and a hp notifier).
> Is it going to be a problem if the value changes?

That's a good point. At a quick look I didn't see anything that would be
affected by a non-constant cache_line_size().

-- 
Catalin

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

Re: [PATCH 2/2] arm64: cacheinfo: Update cache_line_size detected from PPTT

[Catalin Marinas]

On Fri, Apr 26, 2019 at 09:40:21AM +0800, Shaokun Zhang wrote:
> diff --git a/arch/arm64/include/asm/cache.h b/arch/arm64/include/asm/cache.h
> index 926434f413fa..f120d48b27ac 100644
> --- a/arch/arm64/include/asm/cache.h
> +++ b/arch/arm64/include/asm/cache.h
> @@ -91,11 +91,7 @@ static inline u32 cache_type_cwg(void)
>  
>  #define __read_mostly __attribute__((__section__(".data..read_mostly")))
>  
> -static inline int cache_line_size(void)
> -{
> -	u32 cwg = cache_type_cwg();
> -	return cwg ? 4 << cwg : ARCH_DMA_MINALIGN;
> -}
> +extern int cache_line_size(void);

Nitpick: no need for 'extern' on function prototypes.

>  /*
>   * Read the effective value of CTR_EL0.
> diff --git a/arch/arm64/kernel/cacheinfo.c b/arch/arm64/kernel/cacheinfo.c
> index 0bf0a835122f..0b26d53790a8 100644
> --- a/arch/arm64/kernel/cacheinfo.c
> +++ b/arch/arm64/kernel/cacheinfo.c
> @@ -28,6 +28,21 @@
>  #define CLIDR_CTYPE(clidr, level)	\
>  	(((clidr) & CLIDR_CTYPE_MASK(level)) >> CLIDR_CTYPE_SHIFT(level))
>  
> +int cache_line_size(void)
> +{
> +	u32 cwg = cache_type_cwg();
> +
> +	if (cwg == 0)
> +		return ARCH_DMA_MINALIGN;
> +#ifdef CONFIG_ACPI
> +	/* compare cache line size detected from PPTT with CWG reporting */
> +	if (coherency_max_size > (4 << cwg))
> +		return coherency_max_size;
> +#endif
> +
> +	return 4 << cwg;
> +}

I'd rather have cache_line_size() report the PPTT information if
available, ignoring CWG with a fallback to the latter if not available.

We don't use cache_line_size() for DMA cache coherency, only
performance, so I think it's safe to return a value smaller than CWG in
cache_line_size().

-- 
Catalin

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

Re: [PATCH 2/2] arm64: cacheinfo: Update cache_line_size detected from PPTT

[Sudeep Holla]

On Fri, Apr 26, 2019 at 12:18:33PM -0500, Jeremy Linton wrote:
> Hi,
>
> On 4/25/19 8:40 PM, Shaokun Zhang wrote:
> > cache_line_size is derived from CTR_EL0.CWG field and is called mostly
> > for I/O device drivers. For HiSilicon certain plantform, like the
>
> But there are core users too? Thinkgs like blk-mq, the trace ring buffer,
> iommu/iova, slub/slab. And a quick look seems to indicate a number of those
> users are going to be checking the cache line size before the cachinfo is
> populated (it happens fairly late via device_initcall() and a hp notifier).
> Is it going to be a problem if the value changes?
>

Yes, I agree with that and share the same concern. If the users of these
can't get updated with the new value once cacheinfo is populated, then
we need to figure to solve this differently(I mean still from PPTT or
firmware info as we don't have anything more reliable).

--
Regards,
Sudeep

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

Re: [PATCH 2/2] arm64: cacheinfo: Update cache_line_size detected from PPTT

[Sudeep Holla]

On Sat, Apr 27, 2019 at 05:12:44PM +0100, Catalin Marinas wrote:
> On Fri, Apr 26, 2019 at 12:18:33PM -0500, Jeremy Linton wrote:
> > On 4/25/19 8:40 PM, Shaokun Zhang wrote:
> > > cache_line_size is derived from CTR_EL0.CWG field and is called mostly
> > > for I/O device drivers. For HiSilicon certain plantform, like the
> >
> > But there are core users too? Thinkgs like blk-mq, the trace ring buffer,
> > iommu/iova, slub/slab.
>
> cache_line_size() is indeed used in the core parts of the kernel, for
> example when passing SLAB_HWCACHE_ALIGN on kmem_cache creation. Its
> meaning is performance rather than coherency as we use ARCH_DMA_MINALIGN
> for the latter.
>
> > And a quick look seems to indicate a number of those
> > users are going to be checking the cache line size before the cachinfo is
> > populated (it happens fairly late via device_initcall() and a hp notifier).
> > Is it going to be a problem if the value changes?
>
> That's a good point. At a quick look I didn't see anything that would be
> affected by a non-constant cache_line_size().
>
Ah, that's good. But won't it still affect early boot allocations(if the
smaller init time value is used before cacheinfo is populated) ? Sorry,
I haven't looked at all the uses of cache_line_size().

But if cache_line_size() takes care of reading updated value and impact on
boot time allocations are minimal, then this solution should be fine.

--
Regards,
Sudeep

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

Re: [PATCH 1/2] ACPI/PPTT: Add variable to record max cache line size

[Sudeep Holla]

On Fri, Apr 26, 2019 at 12:02:45PM -0500, Jeremy Linton wrote:
> Hi,
>
>
> On 4/25/19 8:40 PM, Shaokun Zhang wrote:
> > Add coherency_max_size variable to record the maximum cache line size
> > detected from PPTT information for different cache levels. We will
> > synchronize it with CTR_EL0.CWG reporting in cache_line_size() for arm64.
>
> Is the expectation that the largest cache line size will differ from the LLC
> line size?
>

It's always better to assume so. If we are sure that LLC has max cache line
size, then we can simplify, but I don't think we can assure that :)

> Also, will it remain consistent across all PE's? I believe this is required,
> Yes?
>

It would be same as heterogeneous systems, we just choose max value for
the system.

>
> >
> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > Cc: Len Brown <lenb@kernel.org>
> > Cc: Jeremy Linton <jeremy.linton@arm.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > To: linux-acpi@vger.kernel.org
> > Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
> > ---
> >   drivers/acpi/pptt.c  | 7 ++++++-
> >   include/linux/acpi.h | 1 +
> >   2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
> > index 065c4fc245d1..3998621e00ce 100644
> > --- a/drivers/acpi/pptt.c
> > +++ b/drivers/acpi/pptt.c
> > @@ -341,6 +341,8 @@ static struct acpi_pptt_cache *acpi_find_cache_node(struct acpi_table_header *ta
> >   	return found;
> >   }
> > +unsigned int coherency_max_size;
> > +
> >   /**
> >    * update_cache_properties() - Update cacheinfo for the given processor
> >    * @this_leaf: Kernel cache info structure being updated
> > @@ -360,8 +362,11 @@ static void update_cache_properties(struct cacheinfo *this_leaf,
> >   	this_leaf->fw_token = cpu_node;
> >   	if (found_cache->flags & ACPI_PPTT_SIZE_PROPERTY_VALID)
> >   		this_leaf->size = found_cache->size;
> > -	if (found_cache->flags & ACPI_PPTT_LINE_SIZE_VALID)
> > +	if (found_cache->flags & ACPI_PPTT_LINE_SIZE_VALID) {
> >   		this_leaf->coherency_line_size = found_cache->line_size;
> > +		coherency_max_size = this_leaf->coherency_line_size > coherency_max_size ?
> > +			this_leaf->coherency_line_size : coherency_max_size;
> > +	}
> >   	if (found_cache->flags & ACPI_PPTT_NUMBER_OF_SETS_VALID)
> >   		this_leaf->number_of_sets = found_cache->number_of_sets;
> >   	if (found_cache->flags & ACPI_PPTT_ASSOCIATIVITY_VALID)
> > diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> > index d5dcebd7aad3..199cde875d5b 100644
> > --- a/include/linux/acpi.h
> > +++ b/include/linux/acpi.h
> > @@ -315,6 +315,7 @@ static inline bool acpi_sci_irq_valid(void)
> >   extern int sbf_port;
> >   extern unsigned long acpi_realmode_flags;
> > +extern unsigned int coherency_max_size;
>
> Assuming that the LLC doesn't reflect the max cache line size, and it can't
> be pulled directly from the cpu_cacheinfo, I don't think this is the ideal
> place for this code. Given that DT has a cache line property as well, I
> suspect cache_shared_cpu_map_setup() may be a better place. Although, that
> isn't ideal either, as both these pieces of code have the pretense of being
> architecture neutral. Maybe a new call into arch/arm64/cacheinfo.c?
>

Indeed, we need to consider DT systems too. We can just solve the issue
for ACPI based systems.

Or drivers/base/cacheinfo.c if other architectures are also interested or
doesn't affect them if we add one :), but I don't count too much on this
though as it may end up breaking some other archs.

--
Regards,
Sudeep

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

Re: [PATCH 2/2] arm64: cacheinfo: Update cache_line_size detected from PPTT

[Zhangshaokun]

Hi Catalin,

On 2019/4/28 0:16, Catalin Marinas wrote:
> On Fri, Apr 26, 2019 at 09:40:21AM +0800, Shaokun Zhang wrote:
>> diff --git a/arch/arm64/include/asm/cache.h b/arch/arm64/include/asm/cache.h
>> index 926434f413fa..f120d48b27ac 100644
>> --- a/arch/arm64/include/asm/cache.h
>> +++ b/arch/arm64/include/asm/cache.h
>> @@ -91,11 +91,7 @@ static inline u32 cache_type_cwg(void)
>>  
>>  #define __read_mostly __attribute__((__section__(".data..read_mostly")))
>>  
>> -static inline int cache_line_size(void)
>> -{
>> -	u32 cwg = cache_type_cwg();
>> -	return cwg ? 4 << cwg : ARCH_DMA_MINALIGN;
>> -}
>> +extern int cache_line_size(void);
> 
> Nitpick: no need for 'extern' on function prototypes.
> 

Oh, yes, my bad mistake.

>>  /*
>>   * Read the effective value of CTR_EL0.
>> diff --git a/arch/arm64/kernel/cacheinfo.c b/arch/arm64/kernel/cacheinfo.c
>> index 0bf0a835122f..0b26d53790a8 100644
>> --- a/arch/arm64/kernel/cacheinfo.c
>> +++ b/arch/arm64/kernel/cacheinfo.c
>> @@ -28,6 +28,21 @@
>>  #define CLIDR_CTYPE(clidr, level)	\
>>  	(((clidr) & CLIDR_CTYPE_MASK(level)) >> CLIDR_CTYPE_SHIFT(level))
>>  
>> +int cache_line_size(void)
>> +{
>> +	u32 cwg = cache_type_cwg();
>> +
>> +	if (cwg == 0)
>> +		return ARCH_DMA_MINALIGN;
>> +#ifdef CONFIG_ACPI
>> +	/* compare cache line size detected from PPTT with CWG reporting */
>> +	if (coherency_max_size > (4 << cwg))
>> +		return coherency_max_size;
>> +#endif
>> +
>> +	return 4 << cwg;
>> +}
> 
> I'd rather have cache_line_size() report the PPTT information if
> available, ignoring CWG with a fallback to the latter if not available.
> 

Okay, got it, I will follow it in next version.

> We don't use cache_line_size() for DMA cache coherency, only
> performance, so I think it's safe to return a value smaller than CWG in
> cache_line_size().

Agree, a nice idea.

Thanks,
Shaokun

> 


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

Re: [PATCH 1/2] ACPI/PPTT: Add variable to record max cache line size

[Zhangshaokun]

Hi Jeremy,

On 2019/4/27 1:02, Jeremy Linton wrote:
> Hi,
> 
> 
> On 4/25/19 8:40 PM, Shaokun Zhang wrote:
>> Add coherency_max_size variable to record the maximum cache line size
>> detected from PPTT information for different cache levels. We will
>> synchronize it with CTR_EL0.CWG reporting in cache_line_size() for arm64.
> 
> Is the expectation that the largest cache line size will differ from the LLC line size?
> 

Not sure, on HiSilicon this platform, it is so. For some platform,they may be
the same value. So I want to choose the maximum one.

> Also, will it remain consistent across all PE's? I believe this is required, Yes?
> 

Right.

> 
>>
>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>> Cc: Len Brown <lenb@kernel.org>
>> Cc: Jeremy Linton <jeremy.linton@arm.com>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> To: linux-acpi@vger.kernel.org
>> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
>> ---
>>   drivers/acpi/pptt.c  | 7 ++++++-
>>   include/linux/acpi.h | 1 +
>>   2 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
>> index 065c4fc245d1..3998621e00ce 100644
>> --- a/drivers/acpi/pptt.c
>> +++ b/drivers/acpi/pptt.c
>> @@ -341,6 +341,8 @@ static struct acpi_pptt_cache *acpi_find_cache_node(struct acpi_table_header *ta
>>       return found;
>>   }
>>   +unsigned int coherency_max_size;
>> +
>>   /**
>>    * update_cache_properties() - Update cacheinfo for the given processor
>>    * @this_leaf: Kernel cache info structure being updated
>> @@ -360,8 +362,11 @@ static void update_cache_properties(struct cacheinfo *this_leaf,
>>       this_leaf->fw_token = cpu_node;
>>       if (found_cache->flags & ACPI_PPTT_SIZE_PROPERTY_VALID)
>>           this_leaf->size = found_cache->size;
>> -    if (found_cache->flags & ACPI_PPTT_LINE_SIZE_VALID)
>> +    if (found_cache->flags & ACPI_PPTT_LINE_SIZE_VALID) {
>>           this_leaf->coherency_line_size = found_cache->line_size;
>> +        coherency_max_size = this_leaf->coherency_line_size > coherency_max_size ?
>> +            this_leaf->coherency_line_size : coherency_max_size;
>> +    }
>>       if (found_cache->flags & ACPI_PPTT_NUMBER_OF_SETS_VALID)
>>           this_leaf->number_of_sets = found_cache->number_of_sets;
>>       if (found_cache->flags & ACPI_PPTT_ASSOCIATIVITY_VALID)
>> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
>> index d5dcebd7aad3..199cde875d5b 100644
>> --- a/include/linux/acpi.h
>> +++ b/include/linux/acpi.h
>> @@ -315,6 +315,7 @@ static inline bool acpi_sci_irq_valid(void)
>>     extern int sbf_port;
>>   extern unsigned long acpi_realmode_flags;
>> +extern unsigned int coherency_max_size;
> 
> Assuming that the LLC doesn't reflect the max cache line size, and it can't be pulled directly from the cpu_cacheinfo, I don't think this is the ideal place for this code. Given that DT has a cache line property as well, I suspect cache_shared_cpu_map_setup() may be a better place. Although, that isn't ideal either, as both these pieces of code have the pretense of being architecture neutral. Maybe a new call into arch/arm64/cacheinfo.c?
> 

Oh, you are right that I missed DT mode and only parsed this from PPTT information.
I will try and consider to do it in arch/arm64/kernel/cacheinfo.c

Thanks,
Shaokun

> 
>>     int acpi_register_gsi (struct device *dev, u32 gsi, int triggering, int polarity);
>>   int acpi_gsi_to_irq (u32 gsi, unsigned int *irq);
>>
> 
> 
> .
> 


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

Re: [PATCH 1/2] ACPI/PPTT: Add variable to record max cache line size

[Zhangshaokun]

Hi Sudeep,

On 2019/4/29 19:26, Sudeep Holla wrote:
> On Fri, Apr 26, 2019 at 12:02:45PM -0500, Jeremy Linton wrote:
>> Hi,
>>
>>
>> On 4/25/19 8:40 PM, Shaokun Zhang wrote:
>>> Add coherency_max_size variable to record the maximum cache line size
>>> detected from PPTT information for different cache levels. We will
>>> synchronize it with CTR_EL0.CWG reporting in cache_line_size() for arm64.
>>
>> Is the expectation that the largest cache line size will differ from the LLC
>> line size?
>>
> 
> It's always better to assume so. If we are sure that LLC has max cache line
> size, then we can simplify, but I don't think we can assure that :)
> 
>> Also, will it remain consistent across all PE's? I believe this is required,
>> Yes?
>>
> 
> It would be same as heterogeneous systems, we just choose max value for
> the system.
> 

Yes, right.

>>
>>>
>>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>>> Cc: Len Brown <lenb@kernel.org>
>>> Cc: Jeremy Linton <jeremy.linton@arm.com>
>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>> To: linux-acpi@vger.kernel.org
>>> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
>>> ---
>>>   drivers/acpi/pptt.c  | 7 ++++++-
>>>   include/linux/acpi.h | 1 +
>>>   2 files changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
>>> index 065c4fc245d1..3998621e00ce 100644
>>> --- a/drivers/acpi/pptt.c
>>> +++ b/drivers/acpi/pptt.c
>>> @@ -341,6 +341,8 @@ static struct acpi_pptt_cache *acpi_find_cache_node(struct acpi_table_header *ta
>>>   	return found;
>>>   }
>>> +unsigned int coherency_max_size;
>>> +
>>>   /**
>>>    * update_cache_properties() - Update cacheinfo for the given processor
>>>    * @this_leaf: Kernel cache info structure being updated
>>> @@ -360,8 +362,11 @@ static void update_cache_properties(struct cacheinfo *this_leaf,
>>>   	this_leaf->fw_token = cpu_node;
>>>   	if (found_cache->flags & ACPI_PPTT_SIZE_PROPERTY_VALID)
>>>   		this_leaf->size = found_cache->size;
>>> -	if (found_cache->flags & ACPI_PPTT_LINE_SIZE_VALID)
>>> +	if (found_cache->flags & ACPI_PPTT_LINE_SIZE_VALID) {
>>>   		this_leaf->coherency_line_size = found_cache->line_size;
>>> +		coherency_max_size = this_leaf->coherency_line_size > coherency_max_size ?
>>> +			this_leaf->coherency_line_size : coherency_max_size;
>>> +	}
>>>   	if (found_cache->flags & ACPI_PPTT_NUMBER_OF_SETS_VALID)
>>>   		this_leaf->number_of_sets = found_cache->number_of_sets;
>>>   	if (found_cache->flags & ACPI_PPTT_ASSOCIATIVITY_VALID)
>>> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
>>> index d5dcebd7aad3..199cde875d5b 100644
>>> --- a/include/linux/acpi.h
>>> +++ b/include/linux/acpi.h
>>> @@ -315,6 +315,7 @@ static inline bool acpi_sci_irq_valid(void)
>>>   extern int sbf_port;
>>>   extern unsigned long acpi_realmode_flags;
>>> +extern unsigned int coherency_max_size;
>>
>> Assuming that the LLC doesn't reflect the max cache line size, and it can't
>> be pulled directly from the cpu_cacheinfo, I don't think this is the ideal
>> place for this code. Given that DT has a cache line property as well, I
>> suspect cache_shared_cpu_map_setup() may be a better place. Although, that
>> isn't ideal either, as both these pieces of code have the pretense of being
>> architecture neutral. Maybe a new call into arch/arm64/cacheinfo.c?
>>
> 
> Indeed, we need to consider DT systems too. We can just solve the issue
> for ACPI based systems.
> 

Agree, I missed it before because our platform only supports ACPI mode.

> Or drivers/base/cacheinfo.c if other architectures are also interested or
> doesn't affect them if we add one :), but I don't count too much on this
> though as it may end up breaking some other archs.
> 

As Jeremy suggestion, arch/arm64/kernel/cacheinfo.c may be a good place and I
need to check it.

Thanks,
Shaokun

> --
> Regards,
> Sudeep
> 
> .
> 


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