diff mbox series

[V3] ACPI: Add acpi=nospcr to disable ACPI SPCR as default console on arm64

Message ID 20240622093521.71770-1-liuwei09@cestc.cn (mailing list archive)
State New, archived
Headers show
Series [V3] ACPI: Add acpi=nospcr to disable ACPI SPCR as default console on arm64 | expand

Commit Message

Liu Wei June 22, 2024, 9:35 a.m. UTC
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(-)

Comments

Andrew Lunn June 22, 2024, 5:05 p.m. UTC | #1
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
Liu Wei June 24, 2024, 5:04 a.m. UTC | #2
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
Will Deacon June 24, 2024, 2:42 p.m. UTC | #3
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
Andrew Lunn June 24, 2024, 3:10 p.m. UTC | #4
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
Liu Wei June 25, 2024, 2:48 a.m. UTC | #5
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 mbox series

Patch

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);
 	}