Re: [PATCH 01/16] ACPI: GTDT: Parse information related to the EL2 virtual timer

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

On Thu, May 07, 2026 at 01:55: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 allow it to be eventually
> presented to the architected timer driver.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  drivers/acpi/arm64/gtdt.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/acpi/arm64/gtdt.c b/drivers/acpi/arm64/gtdt.c
> index ffc867bac2d60..1a58007fccf0a 100644
> --- a/drivers/acpi/arm64/gtdt.c
> +++ b/drivers/acpi/arm64/gtdt.c
> @@ -88,6 +88,19 @@ static int __init map_gt_gsi(u32 interrupt, u32 flags)
>  	return acpi_register_gsi(NULL, interrupt, trigger, polarity);
>  }
>  
> +struct gtdt_v3 {
> +	struct acpi_table_gtdt	gtdt_v2;
> +	struct acpi_gtdt_el2	el2_vtimer;
> +};
> +
> +static struct acpi_gtdt_el2 *gtdt_to_el2_vtimer(struct acpi_table_gtdt *gtdt)
> +{
> +	if (gtdt->header.revision < 3)
> +		return NULL;
> +

We should validate the length as well just in case some platform bumped version
but didn't add these fields. Again it is problem if the platform timers are
also missing. We can just add the check in acpi_gtdt_init() if the version >=3
perhaps? If (gtdt->header.length < sizeof(struct gtdt_v3) invalid table or
something.

Also we have in platform_timer_valid()
	platform_timer >= (void *)(acpi_gtdt_desc.gtdt + 1)
which is still valid, but again if someone messes up in v3 keeping
offset where virtual EL2 timer GSI should be present as they just copied
it from v2, then we might allow it as valid.

Do it make sense to add that check as well in acpi_gtdt_init()? I mean
the offset should be >= sizeof(struct gtdt_v3).

Both checks can be added to acpi_gtdt_init() to avoid checking for each
platform timer instance may be ?

Other than this couple of minor nit, this patch looks fine.

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

-- 
Regards,
Sudeep