Re: [PATCH] ACPI: Do not enable ACPI SPCR console by default on arm64

[Prarit Bhargava]

On 6/18/24 23:43, Liu Wei wrote:
> From: Sudeep Holla <sudeep.holla@xxxxxxx>
>
> On Tue, Jun 18, 2024 at 04:29:24PM +0100, Will Deacon wrote:
> > > On Thu, May 30, 2024 at 09:06:17AM -0400, Prarit Bhargava wrote:
> > > > On 5/29/24 21:53, Liu Wei wrote:
> > > > > Consistency with x86 and loongarch, don't enable ACPI SPCR console
> > > > > by default on arm64
> > > > >
> > > > > Signed-off-by: Liu Wei <liuwei09@xxxxxxxx>
> > > > > ---
> > > > >    arch/arm64/kernel/acpi.c | 3 ++-
> > > > >    1 file changed, 2 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> > > > > index dba8fcec7f33..1deda3e5a0d2 100644
> > > > > --- a/arch/arm64/kernel/acpi.c
> > > > > +++ b/arch/arm64/kernel/acpi.c
> > > > > @@ -227,7 +227,8 @@ void __init acpi_boot_table_init(void)
> > > > >    		if (earlycon_acpi_spcr_enable)
> > > > >    			early_init_dt_scan_chosen_stdout();
> > > > >    	} else {
> > > > > -		acpi_parse_spcr(earlycon_acpi_spcr_enable, true);
> > > > > +		/* Do not enable ACPI SPCR console by default */
> > > > > +		acpi_parse_spcr(earlycon_acpi_spcr_enable, false);
> > > > >    		if (IS_ENABLED(CONFIG_ACPI_BGRT))
> > > > >    			acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
> > > > >    	}
> > > >
> > > > It's been a while, and the status of arm hardware may have changed. IIRC the
> > > > choice to force enable this is that most arm hardware is headless and this
> > > > was a _required_ option for booting.
> > > >
> > > > I'm not sure if that's still the case as it's been a long time.
> > > >
> > > > Can anyone from the ARM community provide an approval here?
> > >
> > > I don't have a strong opinion either way, but adding the Arm ACPI folks
> > > in case they care.
> > >
> > > Having said that, if the only rationale for this patch is consistency
> > > with other architectures, then I think I'd lean towards leaving the
> > > behaviour as-is so we don't give users a nasty surprise on their next
> > > kernel upgrade.
> >
> > +1, I am concerned about breaking existing behaviour on the platforms
> > in the wild. Also many platforms running ACPI would have already used
> > console cmdline parameter if SPCR is not their choice for the console.
> > So I don't see the need to align with other arch default behaviour here,
> > we can revisit if someone shouts with a real reason as why cmdline option
> > is not viable.
>
> For varying privacy and security reasons, sometimes we would like to
> completely silence the serial console output, and only enable it through
> cmdline when needed.

FWIW, I understand that concern but the feedback you're receiving is 
that there are many existing systems that depend on this behavior. 
Breaking users for your concerns is not a good idea.

Perhaps a solution is to add yet-another-config-option, or add a kernel 
parameter to disable the SPCR console on ARM?

Something like "acpi=nospcr"?

P.

> On ARM, it is difficult because SPCR is enabled by default.
>
> Thanks for your patience,
> Liu Wei
>
> > --
> > Regards,
> > Sudeep