Re: [PATCH v2 01/17] ACPI: GTDT: Account for GTDTv3 size when walking the platform timer descriptors

[Sudeep Holla <sudeep.holla@kernel.org>]

On Fri, May 15, 2026 at 12:23:50PM +0100, Marc Zyngier wrote:
> On Fri, 15 May 2026 10:51:52 +0100,
> Sudeep Holla <sudeep.holla@kernel.org> wrote:
> > 
> > On Thu, May 14, 2026 at 04:09:29PM +0100, Marc Zyngier wrote:
> > > Since ARMv8.1, the architecture has grown an EL2-private virtual
> > > timer. This has been described in ACPI since ACPI v6.3 and revision
> > > 3 of the GTDT table.
> > > 
> > > An aditional structure was added in ACPICA, though in a rather
> > > bizarre way, and merged in v5.1 as 8f5a14d053100 ("ACPICA: ACPI 6.3:
> > > add GTDT Revision 3 support").
> > > 
> > > Finally plug the table parsing in GTDT, and correct the parsing of
> > > the platform timer subtables to account for the expanded size of
> > > the base table.
> > > 
> > > Suggested-by: Sudeep Holla <sudeep.holla@kernel.org>
> > > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > > ---
> > >  drivers/acpi/arm64/gtdt.c | 15 ++++++++++++++-
> > >  1 file changed, 14 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/acpi/arm64/gtdt.c b/drivers/acpi/arm64/gtdt.c
> > > index ffc867bac2d60..b9d9b8edf2df7 100644
> > > --- a/drivers/acpi/arm64/gtdt.c
> > > +++ b/drivers/acpi/arm64/gtdt.c
> > > @@ -32,6 +32,12 @@ struct acpi_gtdt_descriptor {
> > >  	struct acpi_table_gtdt *gtdt;
> > >  	void *gtdt_end;
> > >  	void *platform_timer;
> > > +	bool v3;
> > > +};
> > > +
> > > +struct gtdt_v3 {
> > > +	struct acpi_table_gtdt	gtdt_v2;
> > > +	struct acpi_gtdt_el2	el2_vtimer;
> > >  };
> > >  
> > >  static struct acpi_gtdt_descriptor acpi_gtdt_desc __initdata;
> > > @@ -39,8 +45,14 @@ static struct acpi_gtdt_descriptor acpi_gtdt_desc __initdata;
> > >  static __init bool platform_timer_valid(void *platform_timer)
> > >  {
> > >  	struct acpi_gtdt_header *gh = platform_timer;
> > > +	void *platform_timer_begin;
> > > +
> > > +	if (acpi_gtdt_desc.v3)
> > > +		platform_timer_begin = container_of(acpi_gtdt_desc.gtdt, struct gtdt_v3, gtdt_v2) + 1;
> > > +	else
> > > +		platform_timer_begin = acpi_gtdt_desc.gtdt + 1;
> > >
> > > -	return (platform_timer >= (void *)(acpi_gtdt_desc.gtdt + 1) &&
> > > +	return (platform_timer >= platform_timer_begin &&
> > >  		platform_timer < acpi_gtdt_desc.gtdt_end &&
> > >  		gh->length != 0 &&
> > >  		platform_timer + gh->length <= acpi_gtdt_desc.gtdt_end);
> > > @@ -169,6 +181,7 @@ int __init acpi_gtdt_init(struct acpi_table_header *table,
> > >  	acpi_gtdt_desc.gtdt = gtdt;
> > >  	acpi_gtdt_desc.gtdt_end = (void *)table + table->length;
> > >  	acpi_gtdt_desc.platform_timer = NULL;
> > > +	acpi_gtdt_desc.v3 = gtdt->header.revision >= 3 && gtdt->header.length >= sizeof(struct gtdt_v3);
> > 
> > Regarding Sashiko’s comment about the missing length validation for GTDT v2, I
> > realised that the current check could cause a malformed v3 table to be
> > interpreted as v2 if its length does not match the expected v3
> > length.
> 
> Yeah, that's overall dodgy. As much as I hate having to write a
> validating parser for ACPI, we need to be prepared for the worst.
> 
> > It would be better to fail early and return an error rather than allow
> > processing to continue with the table incorrectly interpreted as v2.
> 
> How about something like the hack below?
>

LGTM and might please Sashiko too.

Reviewed-by:  Sudeep Holla <sudeep.holla@kernel.org>

-- 
Regards,
Sudeep