| Message ID | 20240622093521.71770-1-liuwei09@cestc.cn (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [V3] ACPI: Add acpi=nospcr to disable ACPI SPCR as default console on arm64 | expand |
On Sat, Jun 22, 2024 at 05:35:21PM +0800, Liu Wei wrote: > For varying privacy and security reasons, sometimes we would like to > completely silence the serial console, and only enable it when needed. > > But there are many existing systems that depend on this console, > so add acpi=nospcr for this situation. Maybe it is just me, but i see nospcr and my brain expands it to "no speaker". Adding to that, your commit message says "completely silence"... > + nospcr -- disable ACPI SPCR as default console on ARM64 > + For ARM64, ONLY "acpi=off", "acpi=on", "acpi=force" or > + "acpi=nospcr" are available > + For RISCV64, ONLY "acpi=off", "acpi=on" or "acpi=force" > + are available How about putting the word 'serial' in here somewhere, just to give users an additional clue you are not talking about a speaker, CTRL-G etc. Andrew
From: Andrew Lunn <andrew@lunn.ch> > On Sat, Jun 22, 2024 at 05:35:21PM +0800, Liu Wei wrote: > > For varying privacy and security reasons, sometimes we would like to > > completely silence the serial console, and only enable it when needed. > > > > But there are many existing systems that depend on this console, > > so add acpi=nospcr for this situation. > > Maybe it is just me, but i see nospcr and my brain expands it to "no > speaker". Adding to that, your commit message says "completely > silence"... > > > + nospcr -- disable ACPI SPCR as default console on ARM64 > > + For ARM64, ONLY "acpi=off", "acpi=on", "acpi=force" or > > + "acpi=nospcr" are available > > + For RISCV64, ONLY "acpi=off", "acpi=on" or "acpi=force" > > + are available > > How about putting the word 'serial' in here somewhere, just to give > users an additional clue you are not talking about a speaker, CTRL-G > etc. Thank you for your suggestion. You mean acpi=nospcr_serial or acpi=no_spcrserial? However, it appears somewhat unconventional compared to the original acpi=* parameter. How about introducing a new one, such as acpi_no_spcr_serial or acpi_no_spcr_console? Best wishes, Liu Wei > Andrew
On Mon, Jun 24, 2024 at 01:04:04PM +0800, Liu Wei wrote: > From: Andrew Lunn <andrew@lunn.ch> > > > On Sat, Jun 22, 2024 at 05:35:21PM +0800, Liu Wei wrote: > > > For varying privacy and security reasons, sometimes we would like to > > > completely silence the serial console, and only enable it when needed. > > > > > > But there are many existing systems that depend on this console, > > > so add acpi=nospcr for this situation. > > > > Maybe it is just me, but i see nospcr and my brain expands it to "no > > speaker". Adding to that, your commit message says "completely > > silence"... > > > > > + nospcr -- disable ACPI SPCR as default console on ARM64 > > > + For ARM64, ONLY "acpi=off", "acpi=on", "acpi=force" or > > > + "acpi=nospcr" are available > > > + For RISCV64, ONLY "acpi=off", "acpi=on" or "acpi=force" > > > + are available > > > > How about putting the word 'serial' in here somewhere, just to give > > users an additional clue you are not talking about a speaker, CTRL-G > > etc. > > Thank you for your suggestion. > > You mean acpi=nospcr_serial or acpi=no_spcrserial? However, it appears > somewhat unconventional compared to the original acpi=* parameter. > > How about introducing a new one, such as acpi_no_spcr_serial or > acpi_no_spcr_console? I think acpi=nospcr is fine like it is. Just expand the acronym in the documentation so that it's obviously not talking about a speaker. Will
On Mon, Jun 24, 2024 at 01:04:04PM +0800, Liu Wei wrote: > From: Andrew Lunn <andrew@lunn.ch> > > > On Sat, Jun 22, 2024 at 05:35:21PM +0800, Liu Wei wrote: > > > For varying privacy and security reasons, sometimes we would like to > > > completely silence the serial console, and only enable it when needed. > > > > > > But there are many existing systems that depend on this console, > > > so add acpi=nospcr for this situation. > > > > Maybe it is just me, but i see nospcr and my brain expands it to "no > > speaker". Adding to that, your commit message says "completely > > silence"... > > > > > + nospcr -- disable ACPI SPCR as default console on ARM64 > > > + For ARM64, ONLY "acpi=off", "acpi=on", "acpi=force" or > > > + "acpi=nospcr" are available > > > + For RISCV64, ONLY "acpi=off", "acpi=on" or "acpi=force" > > > + are available > > > > How about putting the word 'serial' in here somewhere, just to give > > users an additional clue you are not talking about a speaker, CTRL-G > > etc. > > Thank you for your suggestion. > > You mean acpi=nospcr_serial or acpi=no_spcrserial? However, it appears > somewhat unconventional compared to the original acpi=* parameter. How about: nospcr -- disable ACPI SPCR as default _serial_ console on ARM64 Please as Will suggested, add a definition somewhere. Andrew
From: Andrew Lunn <andrew@lunn.ch> > On Mon, Jun 24, 2024 at 01:04:04PM +0800, Liu Wei wrote: > > From: Andrew Lunn <andrew@lunn.ch> > > > > > On Sat, Jun 22, 2024 at 05:35:21PM +0800, Liu Wei wrote: > > > > For varying privacy and security reasons, sometimes we would like to > > > > completely silence the serial console, and only enable it when needed. > > > > > > > > But there are many existing systems that depend on this console, > > > > so add acpi=nospcr for this situation. > > > > > > Maybe it is just me, but i see nospcr and my brain expands it to "no > > > speaker". Adding to that, your commit message says "completely > > > silence"... > > > > > > > + nospcr -- disable ACPI SPCR as default console on ARM64 > > > > + For ARM64, ONLY "acpi=off", "acpi=on", "acpi=force" or > > > > + "acpi=nospcr" are available > > > > + For RISCV64, ONLY "acpi=off", "acpi=on" or "acpi=force" > > > > + are available > > > > > > How about putting the word 'serial' in here somewhere, just to give > > > users an additional clue you are not talking about a speaker, CTRL-G > > > etc. > > > > Thank you for your suggestion. > > > > You mean acpi=nospcr_serial or acpi=no_spcrserial? However, it appears > > somewhat unconventional compared to the original acpi=* parameter. > > How about: > > nospcr -- disable ACPI SPCR as default _serial_ console on ARM64 > > Please as Will suggested, add a definition somewhere. > Ok, I will send the v4 version later. Thanks for all suggestion. Liu Wei > Andrew
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 11e57ba2985c..4c331cfb28f5 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -12,7 +12,7 @@ acpi= [HW,ACPI,X86,ARM64,RISCV64,EARLY] Advanced Configuration and Power Interface Format: { force | on | off | strict | noirq | rsdt | - copy_dsdt } + copy_dsdt | nospcr } force -- enable ACPI if default was off on -- enable ACPI but allow fallback to DT [arm64,riscv64] off -- disable ACPI if default was on @@ -21,8 +21,11 @@ strictly ACPI specification compliant. rsdt -- prefer RSDT over (default) XSDT copy_dsdt -- copy DSDT to memory - For ARM64 and RISCV64, ONLY "acpi=off", "acpi=on" or - "acpi=force" are available + nospcr -- disable ACPI SPCR as default console on ARM64 + For ARM64, ONLY "acpi=off", "acpi=on", "acpi=force" or + "acpi=nospcr" are available + For RISCV64, ONLY "acpi=off", "acpi=on" or "acpi=force" + are available See also Documentation/power/runtime_pm.rst, pci=noacpi diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c index e0e7b93c16cc..35cb12f3b9bd 100644 --- a/arch/arm64/kernel/acpi.c +++ b/arch/arm64/kernel/acpi.c @@ -45,6 +45,7 @@ EXPORT_SYMBOL(acpi_pci_disabled); static bool param_acpi_off __initdata; static bool param_acpi_on __initdata; static bool param_acpi_force __initdata; +static bool param_acpi_nospcr __initdata; static int __init parse_acpi(char *arg) { @@ -58,6 +59,8 @@ static int __init parse_acpi(char *arg) param_acpi_on = true; else if (strcmp(arg, "force") == 0) /* force ACPI to be enabled */ param_acpi_force = true; + else if (strcmp(arg, "nospcr") == 0) /* disable ACPI SPCR as default console */ + param_acpi_nospcr = true; else return -EINVAL; /* Core will print when we return error */ @@ -237,7 +240,19 @@ void __init acpi_boot_table_init(void) acpi_put_table(facs); } #endif - acpi_parse_spcr(earlycon_acpi_spcr_enable, true); + + /* + * For varying privacy and security reasons, sometimes need + * to completely silence the serial console output, and only + * enable it when needed. + * But there are many existing systems that depend on this + * behavior, use acpi=nospcr for this. + */ + acpi_parse_spcr(earlycon_acpi_spcr_enable, + !param_acpi_nospcr); + pr_info("Use ACPI SPCR as default console: %s\n", + param_acpi_nospcr ? "No" : "Yes"); + if (IS_ENABLED(CONFIG_ACPI_BGRT)) acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt); }
For varying privacy and security reasons, sometimes we would like to completely silence the serial console, and only enable it when needed. But there are many existing systems that depend on this console, so add acpi=nospcr for this situation. Signed-off-by: Liu Wei <liuwei09@cestc.cn> Suggested-by: Prarit Bhargava <prarit@redhat.com> --- v2: Add a config option suggested by Prarit v3: Use cmdline acpi=nospcr instead of config --- Documentation/admin-guide/kernel-parameters.txt | 9 ++++++--- arch/arm64/kernel/acpi.c | 17 ++++++++++++++++- 2 files changed, 22 insertions(+), 4 deletions(-)