ACK: [PATCH 1/1][Hirsute][Unstable] ACPI: GTDT: Don't corrupt interrupt mappings on watchdow probe failure

[Colin Ian King]

On 03/05/2021 22:51, dann frazier wrote:
> From: Marc Zyngier <maz at kernel.org>
> 
> BugLink: https://bugs.launchpad.net/bugs/1925075
> 
> When failing the driver probe because of invalid firmware properties,
> the GTDT driver unmaps the interrupt that it mapped earlier.
> 
> However, it never checks whether the mapping of the interrupt actially
> succeeded. Even more, should the firmware report an illegal interrupt
> number that overlaps with the GIC SGI range, this can result in an
> IPI being unmapped, and subsequent fireworks (as reported by Dann
> Frazier).
> 
> Rework the driver to have a slightly saner behaviour and actually
> check whether the interrupt has been mapped before unmapping things.
> 
> Reported-by: dann frazier <dann.frazier at canonical.com>
> Fixes: ca9ae5ec4ef0 ("acpi/arm64: Add SBSA Generic Watchdog support in GTDT driver")
> Signed-off-by: Marc Zyngier <maz at kernel.org>
> Link: https://lore.kernel.org/r/YH87dtTfwYgavusz@xps13.dannf
> Cc: <stable at vger.kernel.org>
> Cc: Fu Wei <wefu at redhat.com>
> Reviewed-by: Sudeep Holla <sudeep.holla at arm.com>
> Tested-by: dann frazier <dann.frazier at canonical.com>
> Tested-by: Hanjun Guo <guohanjun at huawei.com>
> Reviewed-by: Hanjun Guo <guohanjun at huawei.com>
> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi at arm.com>
> Link: https://lore.kernel.org/r/20210421164317.1718831-2-maz@kernel.org
> Signed-off-by: Catalin Marinas <catalin.marinas at arm.com>
> (cherry picked from commit 1ecd5b129252249b9bc03d7645a7bda512747277 linux-next)
> Signed-off-by: dann frazier <dann.frazier at canonical.com>
> ---
>  drivers/acpi/arm64/gtdt.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/acpi/arm64/gtdt.c b/drivers/acpi/arm64/gtdt.c
> index f2d0e5915dab..0a0a982f9c28 100644
> --- a/drivers/acpi/arm64/gtdt.c
> +++ b/drivers/acpi/arm64/gtdt.c
> @@ -329,7 +329,7 @@ static int __init gtdt_import_sbsa_gwdt(struct acpi_gtdt_watchdog *wd,
>  					int index)
>  {
>  	struct platform_device *pdev;
> -	int irq = map_gt_gsi(wd->timer_interrupt, wd->timer_flags);
> +	int irq;
>  
>  	/*
>  	 * According to SBSA specification the size of refresh and control
> @@ -338,7 +338,7 @@ static int __init gtdt_import_sbsa_gwdt(struct acpi_gtdt_watchdog *wd,
>  	struct resource res[] = {
>  		DEFINE_RES_MEM(wd->control_frame_address, SZ_4K),
>  		DEFINE_RES_MEM(wd->refresh_frame_address, SZ_4K),
> -		DEFINE_RES_IRQ(irq),
> +		{},
>  	};
>  	int nr_res = ARRAY_SIZE(res);
>  
> @@ -348,10 +348,11 @@ static int __init gtdt_import_sbsa_gwdt(struct acpi_gtdt_watchdog *wd,
>  
>  	if (!(wd->refresh_frame_address && wd->control_frame_address)) {
>  		pr_err(FW_BUG "failed to get the Watchdog base address.\n");
> -		acpi_unregister_gsi(wd->timer_interrupt);
>  		return -EINVAL;
>  	}
>  
> +	irq = map_gt_gsi(wd->timer_interrupt, wd->timer_flags);
> +	res[2] = (struct resource)DEFINE_RES_IRQ(irq);
>  	if (irq <= 0) {
>  		pr_warn("failed to map the Watchdog interrupt.\n");
>  		nr_res--;
> @@ -364,7 +365,8 @@ static int __init gtdt_import_sbsa_gwdt(struct acpi_gtdt_watchdog *wd,
>  	 */
>  	pdev = platform_device_register_simple("sbsa-gwdt", index, res, nr_res);
>  	if (IS_ERR(pdev)) {
> -		acpi_unregister_gsi(wd->timer_interrupt);
> +		if (irq > 0)
> +			acpi_unregister_gsi(wd->timer_interrupt);
>  		return PTR_ERR(pdev);
>  	}
>  
> 

Makes sense, positive test results.

Acked-by: Colin Ian King <colin.king at canonical.com>