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

[Prarit Bhargava <prarit@redhat.com>]

On 6/18/24 23:43, Liu Wei wrote:
> From: Sudeep Holla <sudeep.holla@arm.com>
> 
> 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@cestc.cn>
>>>>> ---
>>>>>    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
> 
> 
> 
>