diff mbox series

[v2,1/2] arm64/acpi: panic when failed to init acpi table with acpi=force option

Message ID 20241125170758.518943-2-yeoreum.yun@arm.com (mailing list archive)
State New
Headers show
Series small fixes when boot with acpi=force option | expand

Commit Message

Yeo Reum Yun Nov. 25, 2024, 5:07 p.m. UTC
when the acpi=force option is used,
the system does not fall back to the device tree (DT).
If it fails to initialize the ACPI table, it cannot proceed further.
In such cases, the system should invoke panic() to avoid contradicting
the user's explicit intent, as failing or
proceeding with unintended behavior would violate their wishes.

Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
Reviewed-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/kernel/acpi.c | 2 ++
 1 file changed, 2 insertions(+)

--
LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}

Comments

Ard Biesheuvel Nov. 25, 2024, 5:30 p.m. UTC | #1
On Mon, 25 Nov 2024 at 18:08, Yeoreum Yun <yeoreum.yun@arm.com> wrote:
>
> when the acpi=force option is used,
> the system does not fall back to the device tree (DT).
> If it fails to initialize the ACPI table, it cannot proceed further.
> In such cases, the system should invoke panic() to avoid contradicting
> the user's explicit intent, as failing or
> proceeding with unintended behavior would violate their wishes.
>

Calling panic() at this point does not achieve anything useful,
though. Without ACPI tables or a DT, the only way to observe this
panic message is by using earlycon= with an explicit MMIO address, and
it might be better to limp on instead. Is there anything bad that
might happen because of this, other than the user's wishes getting
violated?


> Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> Reviewed-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/kernel/acpi.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index e6f66491fbe9..efdf24ed5c3e 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -225,6 +225,8 @@ void __init acpi_boot_table_init(void)
>                 pr_err("Failed to init ACPI tables\n");
>                 if (!param_acpi_force)
>                         disable_acpi();
> +               else
> +                       panic("Failed to boot with ACPI tables\n");
>         }
>
>  done:
> --
> LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
>
Mark Brown Nov. 25, 2024, 5:41 p.m. UTC | #2
On Mon, Nov 25, 2024 at 06:30:06PM +0100, Ard Biesheuvel wrote:
> On Mon, 25 Nov 2024 at 18:08, Yeoreum Yun <yeoreum.yun@arm.com> wrote:

> > when the acpi=force option is used,
> > the system does not fall back to the device tree (DT).
> > If it fails to initialize the ACPI table, it cannot proceed further.
> > In such cases, the system should invoke panic() to avoid contradicting
> > the user's explicit intent, as failing or
> > proceeding with unintended behavior would violate their wishes.

> Calling panic() at this point does not achieve anything useful,
> though. Without ACPI tables or a DT, the only way to observe this
> panic message is by using earlycon= with an explicit MMIO address, and
> it might be better to limp on instead. Is there anything bad that
> might happen because of this, other than the user's wishes getting
> violated?

It does rather depend why the user specified acpi=force, it's kind of an
unusual thing to specify on most systems...
Yeo Reum Yun Nov. 25, 2024, 5:59 p.m. UTC | #3
Hi Ard.

>
> Calling panic() at this point does not achieve anything useful,
> though. Without ACPI tables or a DT, the only way to observe this
> panic message is by using earlycon= with an explicit MMIO address, and
> it might be better to limp on instead. Is there anything bad that
> might happen because of this, other than the user's wishes getting
> violated?

IMHO, the most weird thing is progressing boot with acpi table although
it failed to initailise. in this situation continuing to boot maybe
dead in unexepceted places. I think it would be better to prevent
futher progress by calling the panic() in this situation.
diff mbox series

Patch

diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index e6f66491fbe9..efdf24ed5c3e 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -225,6 +225,8 @@  void __init acpi_boot_table_init(void)
 		pr_err("Failed to init ACPI tables\n");
 		if (!param_acpi_force)
 			disable_acpi();
+		else
+			panic("Failed to boot with ACPI tables\n");
 	}

 done: