Re: [PATCH 2/2] arm64: topology: Use PPTT to determine if PE is a thread

[Jeremy Linton <jeremy.linton@arm.com>]

Hi,

Thanks for testing and looking at this.

On 6/6/19 3:49 AM, John Garry wrote:
> On 23/05/2019 23:40, Jeremy Linton wrote:
>> ACPI 6.3 adds a thread flag to represent if a CPU/PE is
>> actually a thread. Given that the MPIDR_MT bit may not
>> represent this information consistently on homogeneous machines
>> we should prefer the PPTT flag if its available.
>>
> 
> Hi Jeremy,
> 
> I was just wondering if we should look to get this support backported 
> (when merged)?

I imagine that will happen..

> 
> I worry about the case of a system with the CPU having MT bit in the 
> MPIDR (while not actually threaded), i.e. the system for which these 
> PPTT flags were added (as I understand).

I have tested this patch on DAWN which happens to have the MT bit set, 
but isn't threaded, and it appears to work.

> 
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>> ---
>>  arch/arm64/kernel/topology.c | 8 +++++---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
>> index 0825c4a856e3..cbbedb53cf06 100644
>> --- a/arch/arm64/kernel/topology.c
>> +++ b/arch/arm64/kernel/topology.c
>> @@ -346,11 +346,9 @@ void remove_cpu_topology(unsigned int cpu)
>>   */
>>  static int __init parse_acpi_topology(void)
>>  {
>> -    bool is_threaded;
>> +    int is_threaded;
>>      int cpu, topology_id;
>>
>> -    is_threaded = read_cpuid_mpidr() & MPIDR_MT_BITMASK;
>> -
>>      for_each_possible_cpu(cpu) {
>>          int i, cache_id;
>>
>> @@ -358,6 +356,10 @@ static int __init parse_acpi_topology(void)
>>          if (topology_id < 0)
>>              return topology_id;
>>
>> +        is_threaded = acpi_pptt_cpu_is_thread(cpu);
>> +        if (is_threaded < 0)
>> +            is_threaded = read_cpuid_mpidr() & MPIDR_MT_BITMASK;
>> +
>>          if (is_threaded) {
>>              cpu_topology[cpu].thread_id = topology_id;
> 
> For described above scenario, this seems wrong.

I'm not sure I understand the concern.

This is going to ignore the MPIDR_MT bit on any machine with a PPTT 
revision > 1. Are you worried about the topology_id assignment?


> 
>>              topology_id = find_acpi_cpu_topology(cpu, 1);
>>
> 
> BTW, we did test an old kernel with 6.3 PPTT bios for this on D06 (some 
> versions have MT bit set), and it looked ok. But I am still a bit 
> skeptical.
> 
> Thanks,
> John
> 
> 


Thanks,