diff mbox series

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

Message ID 1556242821-5080-1-git-send-email-zhangshaokun@hisilicon.com (mailing list archive)
State New, archived
Headers show
Series [1/2] ACPI/PPTT: Add variable to record max cache line size | expand

Commit Message

Shaokun Zhang April 26, 2019, 1:40 a.m. UTC
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(-)

Comments

Jeremy Linton April 26, 2019, 5:02 p.m. UTC | #1
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);
>
Sudeep Holla April 29, 2019, 11:26 a.m. UTC | #2
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
Shaokun Zhang April 30, 2019, 2:13 a.m. UTC | #3
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);
>>
> 
> 
> .
>
Shaokun Zhang April 30, 2019, 2:19 a.m. UTC | #4
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
> 
> .
>
diff mbox series

Patch

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);