Re: [PATCH v3 04/29] ACPI / PPTT: Add a helper to fill a cpumask from a cache_id

[Ben Horgan]

Hi Jonathan, Jeremy,

(Replying for James as I'll post the next version of this series.)

On 10/24/25 15:22, Jonathan Cameron wrote:
> On Wed, 22 Oct 2025 07:58:36 -0500
> Jeremy Linton <jeremy.linton@xxxxxxx> wrote:
> 
>> Hi,
>>
>> This is largely looking pretty solid, but..
>>
>>
>> On 10/17/25 1:56 PM, James Morse wrote:
>>> MPAM identifies CPUs by the cache_id in the PPTT cache structure.
>>>
>>> The driver needs to know which CPUs are associated with the cache.
>>> The CPUs may not all be online, so cacheinfo does not have the
>>> information.
>>>
>>> Add a helper to pull this information out of the PPTT.
>>>
>>> CC: Rohit Mathew <Rohit.Mathew@xxxxxxx>
>>> Signed-off-by: James Morse <james.morse@xxxxxxx>
>>> Reviewed-by: Sudeep Holla <sudeep.holla@xxxxxxx>
>>> Tested-by: Fenghua Yu <fenghuay@xxxxxxxxxx>
>>> ---
[...]
>>
>> Is the core acpi definition in actbl2.h correct? Shouldn't it be 
>> something along the lines of:
>>
>> struct acpi_pptt_cache_v1 {
>>   struct acpi_subtable_header header;
>>   u16 reservedd;
>>   u32 flags;
>>   u32 next_level_of_cache;
>>   u32 size;
>>   u32 number_of_sets;
>>   u8 associativity;
>>   u8 attributes;
>>   u16 lines_size;
>>   u32 cache_id;
>> }
>>
>>
>> Then that solves the detection of the additional field problem correctly 
>> because the length (24 vs 28) of the subtable then tells you which 
>> version your dealing with. (and goes back to why much of this is coded 
>> to use ACPI_ADD_PTR rather than structure+ logic.)
>>
> 
> Do we want to deal with arguing the change in ACPICA? 
> I fully agree that it would be much nicer if that didn't use this weird
> bits of structures approach :(  

For the moment I've added:

struct acpi_pptt_cache_v1_full {
	struct acpi_pptt_cache		f;
	struct acpi_pptt_cache_v1	extra;
} __packed;

in drivers/acpi/pptt.c
A stop gap but the length can be used to check for field presence and
you can avoid some ACPI_ADD_PTR usages.

> 
> https://github.com/acpica/acpica/blob/master/source/include/actbl2.h#L3497
> is where this is coming from.
> 
> Maybe can do it in parallel. Rafael, what do you think is best way forwards
> with this?
> 
> Jonathan
Thanks,

Ben