Re: [PATCH] ACPI/PPTT: fix off-by-one error

["Heyne, Maximilian" <mheyne@amazon.de>]

On Wed, May 07, 2025 at 01:56:53PM +0100, Sudeep Holla wrote:
> On Wed, May 07, 2025 at 12:42:14PM +0000, Heyne, Maximilian wrote:
> > On Wed, May 07, 2025 at 01:30:53PM +0100, Sudeep Holla wrote:
> > > On Wed, May 07, 2025 at 11:56:48AM +0000, Heyne, Maximilian wrote:
> > > > On Wed, May 07, 2025 at 12:52:18PM +0100, Sudeep Holla wrote:
> > > > > 
> > > > > Just to understand, this node is absolutely processor node with no
> > > > > private resources ? I find it hard to trust this as most of the CPUs
> > > > > do have L1 I&D caches. If they were present the table can't abruptly end
> > > > > like this.
> > > > 
> > > > Yes looks like it. In our case the ACPI subtable has length 0x14 which is
> > > > exactly sizeof(acpi_pptt_processor).
> > > > 
> > > 
> > > OK, this seem like it is emulated platform with no private resources as
> > > it is specified in the other similar patch clearly(QEMU/VM). So this
> > > doesn't match real platforms. Your PPTT is wrong if it is real hardware
> > > platform as you must have private resources.
> > > 
> > > Anyways if we allow emulation to present CPUs without private resources
> > > we may have to consider allowing this as the computed pointer will match
> > > the table end.
> > 
> > Is there a need by the ACPI specification that the Cache information
> > must come after the processor information? Because on our platform there
> > is Cache and it's described but at a different location seemingly. It
> > looks like caches are described first and then the CPUs.
> >
> 
> That is fine but you must have reference to those caches in the processor
> node and the length of the node won't be 0x14 in that case and you shouldn't
> hit this issue. So if this is real platform, then yes I am must say you
> PPTT is wrong especially if there are caches in the table as you say just
> that processor nodes are not pointing to them correctly then ?

The ACPI tables in our case describe a core first which references the
cache as private resource and then a thread whose parent is the core but
this doesn't have a private resource. This is how it looks like:

[C8Eh 3214   1]                Subtable Type : 00 [Processor Hierarchy Node]
[C8Fh 3215   1]                       Length : 1C
[C90h 3216   2]                     Reserved : 0000
[C92h 3218   4]        Flags (decoded below) : 00000002
                            Physical package : 0
                     ACPI Processor ID valid : 1
                       Processor is a thread : 0
                              Node is a leaf : 0
                    Identical Implementation : 0
[C96h 3222   4]                       Parent : 000000A2
[C9Ah 3226   4]            ACPI Processor ID : 0000003F
[C9Eh 3230   4]      Private Resource Number : 00000002
[CA2h 3234   4]             Private Resource : 00000072
[CA6h 3238   4]             Private Resource : 0000008A

[CAAh 3242   1]                Subtable Type : 00 [Processor Hierarchy Node]
[CABh 3243   1]                       Length : 14
[CACh 3244   2]                     Reserved : 0000
[CAEh 3246   4]        Flags (decoded below) : 0000000E
                            Physical package : 0
                     ACPI Processor ID valid : 1
                       Processor is a thread : 1
                              Node is a leaf : 1
                    Identical Implementation : 0
[CB2h 3250   4]                       Parent : 00000C8E
[CB6h 3254   4]            ACPI Processor ID : 0000003F
[CBAh 3258   4]      Private Resource Number : 00000000

> 
> > I can try to drill even deeper here if you insist. As said I'm no
> > subject matter expert here. But is there something obviously wrong with
> > my patch or would it be ok to just take it?
> >
> 
> Yes you much check your PPTT if it is real hardware platform. I am OK
> with the change in terms of QEMU or VM. You may need to reword commit
> message a bit. I will respond separately.
> 
> -- 
> Regards,
> Sudeep



Amazon Web Services Development Center Germany GmbH
Tamara-Danz-Str. 13
10243 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597