qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2 0/3] acpi: Add machine option to disable SPCR table
@ 2025-04-22 12:03 Li Chen
  2025-04-22 12:05 ` [PATCH V2 1/3] " Li Chen
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Li Chen @ 2025-04-22 12:03 UTC (permalink / raw)
  To: Li Chen, Peter Maydell, Shannon Zhao, Michael S. Tsirkin,
	Igor Mammedov, Ani Sinha, Eduardo Habkost, Marcel Apfelbaum,
	"Philippe Mathieu-Daudé", Yanan Wang, Zhao Liu,
	Song Gao, Jiaxun Yang, Sunil V L, Palmer Dabbelt,
	Alistair Francis, Weiwei Li, qemu-arm, qemu-devel, qemu-riscv

This series introduces a new machine option, spcr=on|off, allowing users
to disable the ACPI SPCR (Serial Port Console Redirection) table.
By default, SPCR is enabled. Disabling it can help ensure that the guest's
console behavior is determined solely by kernel command-line parameters
on arch like arm64, avoiding unintended serial console configurations imposed
 by firmware.

Also add tests on AArch64 and RISC-V virt machines using TCG and UEFI boot.

Changes since v1:
- Add bios-tables-test for RISC-V and ARM as suggested by 
- Add Acked-by from Michael S. Tsirkin for the first patch
- Add Reviewed-by from Bibo Mao for the first patch

Li Chen (3):
  acpi: Add machine option to disable SPCR table as suggested by Philippe Mathieu-Daudé
  tests/qtest/bios-tables-test: Add test for disabling SPCR on AArch64
  tests/qtest/bios-tables-test: Add test for disabling SPCR on RISC-V

 hw/arm/virt-acpi-build.c       |  5 +++-
 hw/core/machine.c              | 22 ++++++++++++++++++
 hw/loongarch/virt-acpi-build.c |  4 +++-
 hw/riscv/virt-acpi-build.c     |  5 +++-
 include/hw/boards.h            |  1 +
 qemu-options.hx                |  5 ++++
 tests/qtest/bios-tables-test.c | 42 ++++++++++++++++++++++++++++++++++
 7 files changed, 81 insertions(+), 3 deletions(-)

-- 
2.49.0




^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH V2 1/3] acpi: Add machine option to disable SPCR table
  2025-04-22 12:03 [PATCH V2 0/3] acpi: Add machine option to disable SPCR table Li Chen
@ 2025-04-22 12:05 ` Li Chen
  2025-04-22 13:18   ` Philippe Mathieu-Daudé
  2025-04-23  0:11   ` Gavin Shan
  2025-04-22 12:06 ` [PATCH V2 2/3] tests/qtest/bios-tables-test: Add test for disabling SPCR on AArch64 Li Chen
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 12+ messages in thread
From: Li Chen @ 2025-04-22 12:05 UTC (permalink / raw)
  To: Peter Maydell, Shannon Zhao, Michael S. Tsirkin, Igor Mammedov,
	Ani Sinha, Eduardo Habkost, Marcel Apfelbaum,
	"Philippe Mathieu-Daudé", Yanan Wang, Zhao Liu,
	Song Gao, Jiaxun Yang, Sunil V L, Palmer Dabbelt,
	Alistair Francis, Weiwei Li, qemu-arm, qemu-devel, qemu-riscv

From: Li Chen <chenl311@chinatelecom.cn>

The ACPI SPCR (Serial Port Console Redirection) table allows firmware
to specify a preferred serial console device to the operating system.
On ARM64 systems, Linux by default respects this table: even if the
kernel command line does not include a hardware serial console (e.g.,
"console=ttyAMA0"), the kernel still register the serial device
referenced by SPCR as a printk console.

While this behavior is standard-compliant, it can lead to situations
where guest console behavior is influenced by platform firmware rather
than user-specified configuration. To make guest console behavior more
predictable and under user control, this patch introduces a machine
option to explicitly disable SPCR table exposure:

    -machine spcr=off

By default, the option is enabled (spcr=on), preserving existing
behavior. When disabled, QEMU will omit the SPCR table from the guest's
ACPI namespace, ensuring that only consoles explicitly declared in the
kernel command line are registered.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
Reviewed-by: Bibo Mao <maobibo@loongson.cn>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---

Changes since V1: add Reviewed-by and Acked-by

 hw/arm/virt-acpi-build.c       |  5 ++++-
 hw/core/machine.c              | 22 ++++++++++++++++++++++
 hw/loongarch/virt-acpi-build.c |  4 +++-
 hw/riscv/virt-acpi-build.c     |  5 ++++-
 include/hw/boards.h            |  1 +
 qemu-options.hx                |  5 +++++
 6 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 3ac8f8e178..f25c3b26ce 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -940,7 +940,10 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
     }
 
     acpi_add_table(table_offsets, tables_blob);
-    spcr_setup(tables_blob, tables->linker, vms);
+
+    if (ms->enable_spcr) {
+        spcr_setup(tables_blob, tables->linker, vms);
+    }
 
     acpi_add_table(table_offsets, tables_blob);
     build_dbg2(tables_blob, tables->linker, vms);
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 63c6ef93d2..d56f44f4e8 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -590,6 +590,20 @@ static void machine_set_nvdimm(Object *obj, bool value, Error **errp)
     ms->nvdimms_state->is_enabled = value;
 }
 
+static bool machine_get_spcr(Object *obj, Error **errp)
+{
+    MachineState *ms = MACHINE(obj);
+
+    return ms->enable_spcr;
+}
+
+static void machine_set_spcr(Object *obj, bool value, Error **errp)
+{
+    MachineState *ms = MACHINE(obj);
+
+    ms->enable_spcr = value;
+}
+
 static bool machine_get_hmat(Object *obj, Error **errp)
 {
     MachineState *ms = MACHINE(obj);
@@ -1294,6 +1308,14 @@ static void machine_initfn(Object *obj)
                                         "Table (HMAT)");
     }
 
+    /* SPCR */
+    ms->enable_spcr = true;
+    object_property_add_bool(obj, "spcr", machine_get_spcr, machine_set_spcr);
+    object_property_set_description(obj, "spcr",
+                                   "Set on/off to enable/disable "
+                                   "ACPI Serial Port Console Redirection "
+                                   "Table (spcr)");
+
     /* default to mc->default_cpus */
     ms->smp.cpus = mc->default_cpus;
     ms->smp.max_cpus = mc->default_cpus;
diff --git a/hw/loongarch/virt-acpi-build.c b/hw/loongarch/virt-acpi-build.c
index fced6c445a..0e437bcf25 100644
--- a/hw/loongarch/virt-acpi-build.c
+++ b/hw/loongarch/virt-acpi-build.c
@@ -557,7 +557,9 @@ static void acpi_build(AcpiBuildTables *tables, MachineState *machine)
     acpi_add_table(table_offsets, tables_blob);
     build_srat(tables_blob, tables->linker, machine);
     acpi_add_table(table_offsets, tables_blob);
-    spcr_setup(tables_blob, tables->linker, machine);
+
+    if (machine->enable_spcr)
+        spcr_setup(tables_blob, tables->linker, machine);
 
     if (machine->numa_state->num_nodes) {
         if (machine->numa_state->have_numa_distance) {
diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
index 1ad6800508..7f6d221c63 100644
--- a/hw/riscv/virt-acpi-build.c
+++ b/hw/riscv/virt-acpi-build.c
@@ -680,7 +680,10 @@ static void virt_acpi_build(RISCVVirtState *s, AcpiBuildTables *tables)
     build_rhct(tables_blob, tables->linker, s);
 
     acpi_add_table(table_offsets, tables_blob);
-    spcr_setup(tables_blob, tables->linker, s);
+
+    if (ms->enable_spcr) {
+        spcr_setup(tables_blob, tables->linker, s);
+    }
 
     acpi_add_table(table_offsets, tables_blob);
     {
diff --git a/include/hw/boards.h b/include/hw/boards.h
index f22b2e7fc7..cdf2791a50 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -444,6 +444,7 @@ struct MachineState {
     SmpCache smp_cache;
     struct NVDIMMState *nvdimms_state;
     struct NumaState *numa_state;
+    bool enable_spcr;
 };
 
 /*
diff --git a/qemu-options.hx b/qemu-options.hx
index dc694a99a3..953680595f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -38,6 +38,7 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
     "                nvdimm=on|off controls NVDIMM support (default=off)\n"
     "                memory-encryption=@var{} memory encryption object to use (default=none)\n"
     "                hmat=on|off controls ACPI HMAT support (default=off)\n"
+    "                spcr=on|off controls ACPI SPCR support (default=on)\n"
 #ifdef CONFIG_POSIX
     "                aux-ram-share=on|off allocate auxiliary guest RAM as shared (default: off)\n"
 #endif
@@ -105,6 +106,10 @@ SRST
         Enables or disables ACPI Heterogeneous Memory Attribute Table
         (HMAT) support. The default is off.
 
+    ``spcr=on|off``
+        Enables or disables ACPI Serial Port Console Redirection Table
+        (SPCR) support. The default is on.
+
     ``aux-ram-share=on|off``
         Allocate auxiliary guest RAM as an anonymous file that is
         shareable with an external process.  This option applies to
-- 
2.49.0




^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH V2 2/3] tests/qtest/bios-tables-test: Add test for disabling SPCR on AArch64
  2025-04-22 12:03 [PATCH V2 0/3] acpi: Add machine option to disable SPCR table Li Chen
  2025-04-22 12:05 ` [PATCH V2 1/3] " Li Chen
@ 2025-04-22 12:06 ` Li Chen
  2025-04-22 12:07 ` [PATCH V2 3/3] tests/qtest/bios-tables-test: Add test for disabling SPCR on RISC-V Li Chen
  2025-04-23  6:27 ` [PATCH V2 0/3] acpi: Add machine option to disable SPCR table bibo mao
  3 siblings, 0 replies; 12+ messages in thread
From: Li Chen @ 2025-04-22 12:06 UTC (permalink / raw)
  To: Peter Maydell, Shannon Zhao, Michael S. Tsirkin, Igor Mammedov,
	Ani Sinha, Eduardo Habkost, Marcel Apfelbaum,
	"Philippe Mathieu-Daudé", Yanan Wang, Zhao Liu,
	Song Gao, Jiaxun Yang, Sunil V L, Palmer Dabbelt,
	Alistair Francis, Weiwei Li, qemu-arm, qemu-devel, qemu-riscv

From: Li Chen <chenl311@chinatelecom.cn>

Add ACPI SPCR table test case for ARM when SPCR was off.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 tests/qtest/bios-tables-test.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 0a333ec435..d2a1aa7fb3 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1789,6 +1789,24 @@ static void test_acpi_aarch64_virt_tcg_pxb(void)
     free_test_data(&data);
 }
 
+static void test_acpi_aarch64_virt_tcg_acpi_spcr(void)
+{
+    test_data data = {
+        .machine = "virt",
+        .arch = "aarch64",
+        .tcg_only = true,
+        .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd",
+        .uefi_fl2 = "pc-bios/edk2-arm-vars.fd",
+        .cd = "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2",
+        .ram_start = 0x40000000ULL,
+        .scan_len = 128ULL * 1024 * 1024,
+        .variant = ".acpispcr",
+    };
+
+    test_acpi_one("-cpu cortex-a57 "
+                  " -machine spcr=off", &data);
+    free_test_data(&data);
+}
 static void test_acpi_tcg_acpi_hmat(const char *machine, const char *arch)
 {
     test_data data = {};
@@ -2583,6 +2601,8 @@ int main(int argc, char *argv[])
             qtest_add_func("acpi/virt/pxb", test_acpi_aarch64_virt_tcg_pxb);
             qtest_add_func("acpi/virt/oem-fields",
                            test_acpi_aarch64_virt_oem_fields);
+            qtest_add_func("acpi/virt/acpispcr",
+                           test_acpi_aarch64_virt_tcg_acpi_spcr);
             if (qtest_has_device("virtio-iommu-pci")) {
                 qtest_add_func("acpi/virt/viot", test_acpi_aarch64_virt_viot);
             }
-- 
2.49.0




^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH V2 3/3] tests/qtest/bios-tables-test: Add test for disabling SPCR on RISC-V
  2025-04-22 12:03 [PATCH V2 0/3] acpi: Add machine option to disable SPCR table Li Chen
  2025-04-22 12:05 ` [PATCH V2 1/3] " Li Chen
  2025-04-22 12:06 ` [PATCH V2 2/3] tests/qtest/bios-tables-test: Add test for disabling SPCR on AArch64 Li Chen
@ 2025-04-22 12:07 ` Li Chen
  2025-04-23  6:27 ` [PATCH V2 0/3] acpi: Add machine option to disable SPCR table bibo mao
  3 siblings, 0 replies; 12+ messages in thread
From: Li Chen @ 2025-04-22 12:07 UTC (permalink / raw)
  To: Peter Maydell, Shannon Zhao, Michael S. Tsirkin, Igor Mammedov,
	Ani Sinha, Eduardo Habkost, Marcel Apfelbaum,
	"Philippe Mathieu-Daudé", Yanan Wang, Zhao Liu,
	Song Gao, Jiaxun Yang, Sunil V L, Palmer Dabbelt,
	Alistair Francis, Weiwei Li, qemu-arm, qemu-devel, qemu-riscv

From: Li Chen <chenl311@chinatelecom.cn>

Add ACPI SPCR table test case for RISC-V when SPCR was off.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 tests/qtest/bios-tables-test.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index d2a1aa7fb3..44de152a36 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1807,6 +1807,26 @@ static void test_acpi_aarch64_virt_tcg_acpi_spcr(void)
                   " -machine spcr=off", &data);
     free_test_data(&data);
 }
+
+static void test_acpi_riscv_virt_tcg_acpi_spcr(void)
+{
+    test_data data = {
+        .machine = "virt",
+        .arch = "riscv64",
+        .tcg_only = true,
+        .uefi_fl1 = "pc-bios/edk2-riscv-code.fd",
+        .uefi_fl2 = "pc-bios/edk2-riscv-vars.fd",
+        .cd = "tests/data/uefi-boot-images/bios-tables-test.riscv64.iso.qcow2",
+        .ram_start = 0x80000000ULL,
+        .scan_len = 128ULL * 1024 * 1024,
+        .variant = ".acpispcr",
+    };
+
+    test_acpi_one("-cpu rva22s64 "
+                  "-machine spcr=off", &data);
+    free_test_data(&data);
+}
+
 static void test_acpi_tcg_acpi_hmat(const char *machine, const char *arch)
 {
     test_data data = {};
@@ -2612,6 +2632,8 @@ int main(int argc, char *argv[])
             qtest_add_func("acpi/virt", test_acpi_riscv64_virt_tcg);
             qtest_add_func("acpi/virt/numamem",
                            test_acpi_riscv64_virt_tcg_numamem);
+            qtest_add_func("acpi/virt/acpispcr",
+                           test_acpi_riscv_virt_tcg_acpi_spcr);
         }
     }
     ret = g_test_run();
-- 
2.49.0




^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH V2 1/3] acpi: Add machine option to disable SPCR table
  2025-04-22 12:05 ` [PATCH V2 1/3] " Li Chen
@ 2025-04-22 13:18   ` Philippe Mathieu-Daudé
  2025-04-22 15:25     ` Li Chen
  2025-04-23  0:11   ` Gavin Shan
  1 sibling, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-04-22 13:18 UTC (permalink / raw)
  To: Li Chen, Peter Maydell, Shannon Zhao, Michael S. Tsirkin,
	Igor Mammedov, Ani Sinha, Eduardo Habkost, Marcel Apfelbaum,
	Yanan Wang, Zhao Liu, Song Gao, Jiaxun Yang, Sunil V L,
	Palmer Dabbelt, Alistair Francis, Weiwei Li, qemu-arm, qemu-devel,
	qemu-riscv

Hi,

On 22/4/25 14:05, Li Chen wrote:
> From: Li Chen <chenl311@chinatelecom.cn>
> 
> The ACPI SPCR (Serial Port Console Redirection) table allows firmware
> to specify a preferred serial console device to the operating system.
> On ARM64 systems, Linux by default respects this table: even if the
> kernel command line does not include a hardware serial console (e.g.,
> "console=ttyAMA0"), the kernel still register the serial device
> referenced by SPCR as a printk console.
> 
> While this behavior is standard-compliant, it can lead to situations
> where guest console behavior is influenced by platform firmware rather
> than user-specified configuration. To make guest console behavior more
> predictable and under user control, this patch introduces a machine
> option to explicitly disable SPCR table exposure:
> 
>      -machine spcr=off
> 
> By default, the option is enabled (spcr=on), preserving existing
> behavior. When disabled, QEMU will omit the SPCR table from the guest's
> ACPI namespace, ensuring that only consoles explicitly declared in the
> kernel command line are registered.
> 
> Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
> Reviewed-by: Bibo Mao <maobibo@loongson.cn>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
> Changes since V1: add Reviewed-by and Acked-by
> 
>   hw/arm/virt-acpi-build.c       |  5 ++++-
>   hw/core/machine.c              | 22 ++++++++++++++++++++++
>   hw/loongarch/virt-acpi-build.c |  4 +++-
>   hw/riscv/virt-acpi-build.c     |  5 ++++-
>   include/hw/boards.h            |  1 +
>   qemu-options.hx                |  5 +++++
>   6 files changed, 39 insertions(+), 3 deletions(-)


> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index f22b2e7fc7..cdf2791a50 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -444,6 +444,7 @@ struct MachineState {
>       SmpCache smp_cache;
>       struct NVDIMMState *nvdimms_state;
>       struct NumaState *numa_state;
> +    bool enable_spcr;

I'm a bit reluctant to add a field used by 3 virt machines as
generic in MachineState. Shouldn't it be for each machine state?

Also I'm surprised we announce the SPCR table regardless a virt
serial exists. Shouldn't we have a disabled default and only
enable on virt machines, preferably checking the serial port
availability?

>   };
>   
>   /*
> diff --git a/qemu-options.hx b/qemu-options.hx
> index dc694a99a3..953680595f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -38,6 +38,7 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
>       "                nvdimm=on|off controls NVDIMM support (default=off)\n"
>       "                memory-encryption=@var{} memory encryption object to use (default=none)\n"
>       "                hmat=on|off controls ACPI HMAT support (default=off)\n"
> +    "                spcr=on|off controls ACPI SPCR support (default=on)\n"
>   #ifdef CONFIG_POSIX
>       "                aux-ram-share=on|off allocate auxiliary guest RAM as shared (default: off)\n"
>   #endif
> @@ -105,6 +106,10 @@ SRST
>           Enables or disables ACPI Heterogeneous Memory Attribute Table
>           (HMAT) support. The default is off.
>   
> +    ``spcr=on|off``
> +        Enables or disables ACPI Serial Port Console Redirection Table
> +        (SPCR) support. The default is on.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2 1/3] acpi: Add machine option to disable SPCR table
  2025-04-22 13:18   ` Philippe Mathieu-Daudé
@ 2025-04-22 15:25     ` Li Chen
  0 siblings, 0 replies; 12+ messages in thread
From: Li Chen @ 2025-04-22 15:25 UTC (permalink / raw)
  To: "Philippe Mathieu-Daudé"
  Cc: Peter Maydell, Shannon Zhao, Michael S. Tsirkin, Igor Mammedov,
	Ani Sinha, Eduardo Habkost, Marcel Apfelbaum, Yanan Wang,
	Zhao Liu, Song Gao, Jiaxun Yang, Sunil V L, Palmer Dabbelt,
	Alistair Francis, Weiwei Li, qemu-arm, qemu-devel, qemu-riscv

Hi Philippe Mathieu-Daudé,

Thanks for your review!

 ---- On Tue, 22 Apr 2025 21:18:46 +0800  Philippe Mathieu-Daudé <philmd@linaro.org> wrote --- 
 > Hi,
 > 
 > On 22/4/25 14:05, Li Chen wrote:
 > > diff --git a/include/hw/boards.h b/include/hw/boards.h
 > > index f22b2e7fc7..cdf2791a50 100644
 > > --- a/include/hw/boards.h
 > > +++ b/include/hw/boards.h
 > > @@ -444,6 +444,7 @@ struct MachineState {
 > >       SmpCache smp_cache;
 > >       struct NVDIMMState *nvdimms_state;
 > >       struct NumaState *numa_state;
 > > +    bool enable_spcr;
 > 
 > I'm a bit reluctant to add a field used by 3 virt machines as
 > generic in MachineState. Shouldn't it be for each machine state?

I looked for alternative locations for this field before submitting v1 but couldn't find any.
I re-evaluated potential locations, like struct AcpiBuildState, but ARM, LoongArch, and RISC-V 
just use their own AcpiBuildState.

BTW, ACPI-supporting virtual machines may eventually adopt SPCR. Although current there 
are just three users, future adoption is probable, making it a suitable field for each machine state.

Feel free to tell me if you have any other suggestions.

 > 
 > Also I'm surprised we announce the SPCR table regardless a virt
 > serial exists. Shouldn't we have a disabled default and only
 > enable on virt machines, preferably checking the serial port
 > availability?

So you mean we should avoid building SPCR for cases like "-serial none"? If so, I can add a patch to check serial
device availability before doing spcr_setup in  ARM/RISC-V/LoongArch codes. 
Should this change be the first patch in this series, or just a separate series?

Regards,
Li


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2 1/3] acpi: Add machine option to disable SPCR table
  2025-04-22 12:05 ` [PATCH V2 1/3] " Li Chen
  2025-04-22 13:18   ` Philippe Mathieu-Daudé
@ 2025-04-23  0:11   ` Gavin Shan
  2025-04-23  8:27     ` Li Chen
  1 sibling, 1 reply; 12+ messages in thread
From: Gavin Shan @ 2025-04-23  0:11 UTC (permalink / raw)
  To: Li Chen, Peter Maydell, Shannon Zhao, Michael S. Tsirkin,
	Igor Mammedov, Ani Sinha, Eduardo Habkost, Marcel Apfelbaum,
	Philippe Mathieu-Daudé, Yanan Wang, Zhao Liu, Song Gao,
	Jiaxun Yang, Sunil V L, Palmer Dabbelt, Alistair Francis,
	Weiwei Li, qemu-arm, qemu-devel, qemu-riscv

On 4/22/25 10:05 PM, Li Chen wrote:
> From: Li Chen <chenl311@chinatelecom.cn>
> 
> The ACPI SPCR (Serial Port Console Redirection) table allows firmware
> to specify a preferred serial console device to the operating system.
> On ARM64 systems, Linux by default respects this table: even if the
> kernel command line does not include a hardware serial console (e.g.,
> "console=ttyAMA0"), the kernel still register the serial device
> referenced by SPCR as a printk console.
> 
> While this behavior is standard-compliant, it can lead to situations
> where guest console behavior is influenced by platform firmware rather
> than user-specified configuration. To make guest console behavior more
> predictable and under user control, this patch introduces a machine
> option to explicitly disable SPCR table exposure:
> 
>      -machine spcr=off
> 
> By default, the option is enabled (spcr=on), preserving existing
> behavior. When disabled, QEMU will omit the SPCR table from the guest's
> ACPI namespace, ensuring that only consoles explicitly declared in the
> kernel command line are registered.
> 
> Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
> Reviewed-by: Bibo Mao <maobibo@loongson.cn>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
> Changes since V1: add Reviewed-by and Acked-by
> 
>   hw/arm/virt-acpi-build.c       |  5 ++++-
>   hw/core/machine.c              | 22 ++++++++++++++++++++++
>   hw/loongarch/virt-acpi-build.c |  4 +++-
>   hw/riscv/virt-acpi-build.c     |  5 ++++-
>   include/hw/boards.h            |  1 +
>   qemu-options.hx                |  5 +++++
>   6 files changed, 39 insertions(+), 3 deletions(-)
> 

One coding style issue below. With it fixed:

Reviewed-by: Gavin Shan <gshan@redhat.com>

> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 3ac8f8e178..f25c3b26ce 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -940,7 +940,10 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
>       }
>   
>       acpi_add_table(table_offsets, tables_blob);
> -    spcr_setup(tables_blob, tables->linker, vms);
> +
> +    if (ms->enable_spcr) {
> +        spcr_setup(tables_blob, tables->linker, vms);
> +    }
>   
>       acpi_add_table(table_offsets, tables_blob);
>       build_dbg2(tables_blob, tables->linker, vms);
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 63c6ef93d2..d56f44f4e8 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -590,6 +590,20 @@ static void machine_set_nvdimm(Object *obj, bool value, Error **errp)
>       ms->nvdimms_state->is_enabled = value;
>   }
>   
> +static bool machine_get_spcr(Object *obj, Error **errp)
> +{
> +    MachineState *ms = MACHINE(obj);
> +
> +    return ms->enable_spcr;
> +}
> +
> +static void machine_set_spcr(Object *obj, bool value, Error **errp)
> +{
> +    MachineState *ms = MACHINE(obj);
> +
> +    ms->enable_spcr = value;
> +}
> +
>   static bool machine_get_hmat(Object *obj, Error **errp)
>   {
>       MachineState *ms = MACHINE(obj);
> @@ -1294,6 +1308,14 @@ static void machine_initfn(Object *obj)
>                                           "Table (HMAT)");
>       }
>   
> +    /* SPCR */
> +    ms->enable_spcr = true;
> +    object_property_add_bool(obj, "spcr", machine_get_spcr, machine_set_spcr);
> +    object_property_set_description(obj, "spcr",
> +                                   "Set on/off to enable/disable "
> +                                   "ACPI Serial Port Console Redirection "
> +                                   "Table (spcr)");
> +
>       /* default to mc->default_cpus */
>       ms->smp.cpus = mc->default_cpus;
>       ms->smp.max_cpus = mc->default_cpus;
> diff --git a/hw/loongarch/virt-acpi-build.c b/hw/loongarch/virt-acpi-build.c
> index fced6c445a..0e437bcf25 100644
> --- a/hw/loongarch/virt-acpi-build.c
> +++ b/hw/loongarch/virt-acpi-build.c
> @@ -557,7 +557,9 @@ static void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>       acpi_add_table(table_offsets, tables_blob);
>       build_srat(tables_blob, tables->linker, machine);
>       acpi_add_table(table_offsets, tables_blob);
> -    spcr_setup(tables_blob, tables->linker, machine);
> +
> +    if (machine->enable_spcr)
> +        spcr_setup(tables_blob, tables->linker, machine);
>   

	if (machine->enable_spcr) {
	    spcr_setup(tables_blob, tables->linker, machine);
	}

>       if (machine->numa_state->num_nodes) {
>           if (machine->numa_state->have_numa_distance) {
> diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
> index 1ad6800508..7f6d221c63 100644
> --- a/hw/riscv/virt-acpi-build.c
> +++ b/hw/riscv/virt-acpi-build.c
> @@ -680,7 +680,10 @@ static void virt_acpi_build(RISCVVirtState *s, AcpiBuildTables *tables)
>       build_rhct(tables_blob, tables->linker, s);
>   
>       acpi_add_table(table_offsets, tables_blob);
> -    spcr_setup(tables_blob, tables->linker, s);
> +
> +    if (ms->enable_spcr) {
> +        spcr_setup(tables_blob, tables->linker, s);
> +    }
>   
>       acpi_add_table(table_offsets, tables_blob);
>       {
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index f22b2e7fc7..cdf2791a50 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -444,6 +444,7 @@ struct MachineState {
>       SmpCache smp_cache;
>       struct NVDIMMState *nvdimms_state;
>       struct NumaState *numa_state;
> +    bool enable_spcr;
>   };
>   
>   /*
> diff --git a/qemu-options.hx b/qemu-options.hx
> index dc694a99a3..953680595f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -38,6 +38,7 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
>       "                nvdimm=on|off controls NVDIMM support (default=off)\n"
>       "                memory-encryption=@var{} memory encryption object to use (default=none)\n"
>       "                hmat=on|off controls ACPI HMAT support (default=off)\n"
> +    "                spcr=on|off controls ACPI SPCR support (default=on)\n"
>   #ifdef CONFIG_POSIX
>       "                aux-ram-share=on|off allocate auxiliary guest RAM as shared (default: off)\n"
>   #endif
> @@ -105,6 +106,10 @@ SRST
>           Enables or disables ACPI Heterogeneous Memory Attribute Table
>           (HMAT) support. The default is off.
>   
> +    ``spcr=on|off``
> +        Enables or disables ACPI Serial Port Console Redirection Table
> +        (SPCR) support. The default is on.
> +
>       ``aux-ram-share=on|off``
>           Allocate auxiliary guest RAM as an anonymous file that is
>           shareable with an external process.  This option applies to

Thanks,
Gavin



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2 0/3] acpi: Add machine option to disable SPCR table
  2025-04-22 12:03 [PATCH V2 0/3] acpi: Add machine option to disable SPCR table Li Chen
                   ` (2 preceding siblings ...)
  2025-04-22 12:07 ` [PATCH V2 3/3] tests/qtest/bios-tables-test: Add test for disabling SPCR on RISC-V Li Chen
@ 2025-04-23  6:27 ` bibo mao
  2025-04-23  8:36   ` Li Chen
  3 siblings, 1 reply; 12+ messages in thread
From: bibo mao @ 2025-04-23  6:27 UTC (permalink / raw)
  To: Li Chen, Peter Maydell, Shannon Zhao, Michael S. Tsirkin,
	Igor Mammedov, Ani Sinha, Eduardo Habkost, Marcel Apfelbaum,
	Philippe Mathieu-Daudé, Yanan Wang, Zhao Liu, Song Gao,
	Jiaxun Yang, Sunil V L, Palmer Dabbelt, Alistair Francis,
	Weiwei Li, qemu-arm, qemu-devel, qemu-riscv



On 2025/4/22 下午8:03, Li Chen wrote:
> This series introduces a new machine option, spcr=on|off, allowing users
> to disable the ACPI SPCR (Serial Port Console Redirection) table.
> By default, SPCR is enabled. Disabling it can help ensure that the guest > console behavior is determined solely by kernel command-line parameters
Hi Li,

SPCR only provides serial port HW description information.

However how to use is determined by Linux kernel, Can you describe the 
detailed scenario which is unintended serial console configurations 
imposed by firmware?

Regards
Bibo Mao
> on arch like arm64, avoiding unintended serial console configurations imposed
>   by firmware.
> 
> Also add tests on AArch64 and RISC-V virt machines using TCG and UEFI boot.
> 
> Changes since v1:
> - Add bios-tables-test for RISC-V and ARM as suggested by
> - Add Acked-by from Michael S. Tsirkin for the first patch
> - Add Reviewed-by from Bibo Mao for the first patch
> 
> Li Chen (3):
>    acpi: Add machine option to disable SPCR table as suggested by Philippe Mathieu-Daudé
>    tests/qtest/bios-tables-test: Add test for disabling SPCR on AArch64
>    tests/qtest/bios-tables-test: Add test for disabling SPCR on RISC-V
> 
>   hw/arm/virt-acpi-build.c       |  5 +++-
>   hw/core/machine.c              | 22 ++++++++++++++++++
>   hw/loongarch/virt-acpi-build.c |  4 +++-
>   hw/riscv/virt-acpi-build.c     |  5 +++-
>   include/hw/boards.h            |  1 +
>   qemu-options.hx                |  5 ++++
>   tests/qtest/bios-tables-test.c | 42 ++++++++++++++++++++++++++++++++++
>   7 files changed, 81 insertions(+), 3 deletions(-)
> 



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2 1/3] acpi: Add machine option to disable SPCR table
  2025-04-23  0:11   ` Gavin Shan
@ 2025-04-23  8:27     ` Li Chen
  0 siblings, 0 replies; 12+ messages in thread
From: Li Chen @ 2025-04-23  8:27 UTC (permalink / raw)
  To: Gavin Shan
  Cc: Peter Maydell, Shannon Zhao, Michael S. Tsirkin, Igor Mammedov,
	Ani Sinha, Eduardo Habkost, Marcel Apfelbaum,
	"Philippe Mathieu-Daudé", Yanan Wang, Zhao Liu,
	Song Gao, Jiaxun Yang, Sunil V L, Palmer Dabbelt,
	Alistair Francis, Weiwei Li, qemu-arm, qemu-devel, qemu-riscv

Hi Gavin,

 ---- On Wed, 23 Apr 2025 08:11:24 +0800  Gavin Shan <gshan@redhat.com> wrote --- 
 > On 4/22/25 10:05 PM, Li Chen wrote:
 > > From: Li Chen <chenl311@chinatelecom.cn>
 > > 
 > > The ACPI SPCR (Serial Port Console Redirection) table allows firmware
 > > to specify a preferred serial console device to the operating system.
 > > On ARM64 systems, Linux by default respects this table: even if the
 > > kernel command line does not include a hardware serial console (e.g.,
 > > "console=ttyAMA0"), the kernel still register the serial device
 > > referenced by SPCR as a printk console.
 > > 
 > > While this behavior is standard-compliant, it can lead to situations
 > > where guest console behavior is influenced by platform firmware rather
 > > than user-specified configuration. To make guest console behavior more
 > > predictable and under user control, this patch introduces a machine
 > > option to explicitly disable SPCR table exposure:
 > > 
 > >      -machine spcr=off
 > > 
 > > By default, the option is enabled (spcr=on), preserving existing
 > > behavior. When disabled, QEMU will omit the SPCR table from the guest's
 > > ACPI namespace, ensuring that only consoles explicitly declared in the
 > > kernel command line are registered.
 > > 
 > > Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
 > > Reviewed-by: Bibo Mao <maobibo@loongson.cn>
 > > Acked-by: Michael S. Tsirkin <mst@redhat.com>
 > > ---
 > > 
 > > Changes since V1: add Reviewed-by and Acked-by
 > > 
 > >   hw/arm/virt-acpi-build.c       |  5 ++++-
 > >   hw/core/machine.c              | 22 ++++++++++++++++++++++
 > >   hw/loongarch/virt-acpi-build.c |  4 +++-
 > >   hw/riscv/virt-acpi-build.c     |  5 ++++-
 > >   include/hw/boards.h            |  1 +
 > >   qemu-options.hx                |  5 +++++
 > >   6 files changed, 39 insertions(+), 3 deletions(-)
 > > 
 > 
 > One coding style issue below. With it fixed:
 > 
 > Reviewed-by: Gavin Shan <gshan@redhat.com>

Thanks for your catch! I would fix it in the next version.

Regards,
Li


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2 0/3] acpi: Add machine option to disable SPCR table
  2025-04-23  6:27 ` [PATCH V2 0/3] acpi: Add machine option to disable SPCR table bibo mao
@ 2025-04-23  8:36   ` Li Chen
  2025-04-23  8:49     ` bibo mao
  0 siblings, 1 reply; 12+ messages in thread
From: Li Chen @ 2025-04-23  8:36 UTC (permalink / raw)
  To: bibo mao
  Cc: Peter Maydell, Shannon Zhao, Michael S. Tsirkin, Igor Mammedov,
	Ani Sinha, Eduardo Habkost, Marcel Apfelbaum,
	"Philippe Mathieu-Daudé", Yanan Wang, Zhao Liu,
	Song Gao, Jiaxun Yang, Sunil V L, Palmer Dabbelt,
	Alistair Francis, Weiwei Li, qemu-arm, qemu-devel, qemu-riscv

Hi Bibo,

 ---- On Wed, 23 Apr 2025 14:27:36 +0800  bibo mao <maobibo@loongson.cn> wrote --- 
 > 
 > 
 > On 2025/4/22 下午8:03, Li Chen wrote:
 > > This series introduces a new machine option, spcr=on|off, allowing users
 > > to disable the ACPI SPCR (Serial Port Console Redirection) table.
 > > By default, SPCR is enabled. Disabling it can help ensure that the guest > console behavior is determined solely by kernel command-line parameters
 > Hi Li,
 > 
 > SPCR only provides serial port HW description information.
 > 
 > However how to use is determined by Linux kernel, Can you describe the 
 > detailed scenario which is unintended serial console configurations 
 > imposed by firmware?

Yes, it is decided by the kernel, more specifically, by how architecture-specific code utilizes acpi_parse_spcr. For example, in the 5.10 arm64 kernel 
https://elixir.bootlin.com/linux/v5.10.236/source/arch/arm64/kernel/acpi.c#L236,  it passes true to acpi_parse_spcr's param enable_console.

So, SPCR consoles are respected by Arm64 Linux by default. Therefore, even without a console=ttyAMA0 configuration, ttyAMA0 is still added as a preferred console
and used by printk.

Regards,
Li


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2 0/3] acpi: Add machine option to disable SPCR table
  2025-04-23  8:36   ` Li Chen
@ 2025-04-23  8:49     ` bibo mao
  2025-04-23  9:17       ` Li Chen
  0 siblings, 1 reply; 12+ messages in thread
From: bibo mao @ 2025-04-23  8:49 UTC (permalink / raw)
  To: Li Chen
  Cc: Peter Maydell, Shannon Zhao, Michael S. Tsirkin, Igor Mammedov,
	Ani Sinha, Eduardo Habkost, Marcel Apfelbaum,
	Philippe Mathieu-Daudé, Yanan Wang, Zhao Liu, Song Gao,
	Jiaxun Yang, Sunil V L, Palmer Dabbelt, Alistair Francis,
	Weiwei Li, qemu-arm, qemu-devel, qemu-riscv



On 2025/4/23 下午4:36, Li Chen wrote:
> Hi Bibo,
> 
>   ---- On Wed, 23 Apr 2025 14:27:36 +0800  bibo mao <maobibo@loongson.cn> wrote ---
>   >
>   >
>   > On 2025/4/22 下午8:03, Li Chen wrote:
>   > > This series introduces a new machine option, spcr=on|off, allowing users
>   > > to disable the ACPI SPCR (Serial Port Console Redirection) table.
>   > > By default, SPCR is enabled. Disabling it can help ensure that the guest > console behavior is determined solely by kernel command-line parameters
>   > Hi Li,
>   >
>   > SPCR only provides serial port HW description information.
>   >
>   > However how to use is determined by Linux kernel, Can you describe the
>   > detailed scenario which is unintended serial console configurations
>   > imposed by firmware?
> 
> Yes, it is decided by the kernel, more specifically, by how architecture-specific code utilizes acpi_parse_spcr. For example, in the 5.10 arm64 kernel
> https://elixir.bootlin.com/linux/v5.10.236/source/arch/arm64/kernel/acpi.c#L236,  it passes true to acpi_parse_spcr's param enable_console.
> 
> So, SPCR consoles are respected by Arm64 Linux by default. Therefore, even without a console=ttyAMA0 configuration, ttyAMA0 is still added as a preferred console
> and used by printk.
About serial port, there is ACPI SPCR table and DSDT table. About how to 
use, it is decided by kernel, so I think that it probably kernel usage 
issue.

However I have no opposing view with your patch, adding option 
"spcr=on|off" sounds good.

Regards
Bibo Mao
> 
> Regards,
> Li
> 



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2 0/3] acpi: Add machine option to disable SPCR table
  2025-04-23  8:49     ` bibo mao
@ 2025-04-23  9:17       ` Li Chen
  0 siblings, 0 replies; 12+ messages in thread
From: Li Chen @ 2025-04-23  9:17 UTC (permalink / raw)
  To: bibo mao
  Cc: Peter Maydell, Shannon Zhao, Michael S. Tsirkin, Igor Mammedov,
	Ani Sinha, Eduardo Habkost, Marcel Apfelbaum,
	"Philippe Mathieu-Daudé", Yanan Wang, Zhao Liu,
	Song Gao, Jiaxun Yang, Sunil V L, Palmer Dabbelt,
	Alistair Francis, Weiwei Li, qemu-arm, qemu-devel, qemu-riscv

Hi Bibo,

 ---- On Wed, 23 Apr 2025 16:49:09 +0800  bibo mao <maobibo@loongson.cn> wrote --- 
 > 
 > 
 > On 2025/4/23 下午4:36, Li Chen wrote:
 > > Hi Bibo,
 > > 
 > >   ---- On Wed, 23 Apr 2025 14:27:36 +0800  bibo mao <maobibo@loongson.cn> wrote ---
 > >   >
 > >   >
 > >   > On 2025/4/22 下午8:03, Li Chen wrote:
 > >   > > This series introduces a new machine option, spcr=on|off, allowing users
 > >   > > to disable the ACPI SPCR (Serial Port Console Redirection) table.
 > >   > > By default, SPCR is enabled. Disabling it can help ensure that the guest > console behavior is determined solely by kernel command-line parameters
 > >   > Hi Li,
 > >   >
 > >   > SPCR only provides serial port HW description information.
 > >   >
 > >   > However how to use is determined by Linux kernel, Can you describe the
 > >   > detailed scenario which is unintended serial console configurations
 > >   > imposed by firmware?
 > > 
 > > Yes, it is decided by the kernel, more specifically, by how architecture-specific code utilizes acpi_parse_spcr. For example, in the 5.10 arm64 kernel
 > > https://elixir.bootlin.com/linux/v5.10.236/source/arch/arm64/kernel/acpi.c#L236,  it passes true to acpi_parse_spcr's param enable_console.
 > > 
 > > So, SPCR consoles are respected by Arm64 Linux by default. Therefore, even without a console=ttyAMA0 configuration, ttyAMA0 is still added as a preferred console
 > > and used by printk.
 > About serial port, there is ACPI SPCR table and DSDT table. About how to 
 > use, it is decided by kernel, so I think that it probably kernel usage 
 > issue.
 > 
 > However I have no opposing view with your patch, adding option 
 > "spcr=on|off" sounds good.


Thanks, I totally agree with you. BTW, newer kernels introduce the nospcr command-line argument, allowing users to control if respect SPCR or not:
https://elixir.bootlin.com/linux/v6.15-rc3/source/arch/arm64/kernel/acpi.c#L254

Regards,
Li


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-04-23  9:18 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-22 12:03 [PATCH V2 0/3] acpi: Add machine option to disable SPCR table Li Chen
2025-04-22 12:05 ` [PATCH V2 1/3] " Li Chen
2025-04-22 13:18   ` Philippe Mathieu-Daudé
2025-04-22 15:25     ` Li Chen
2025-04-23  0:11   ` Gavin Shan
2025-04-23  8:27     ` Li Chen
2025-04-22 12:06 ` [PATCH V2 2/3] tests/qtest/bios-tables-test: Add test for disabling SPCR on AArch64 Li Chen
2025-04-22 12:07 ` [PATCH V2 3/3] tests/qtest/bios-tables-test: Add test for disabling SPCR on RISC-V Li Chen
2025-04-23  6:27 ` [PATCH V2 0/3] acpi: Add machine option to disable SPCR table bibo mao
2025-04-23  8:36   ` Li Chen
2025-04-23  8:49     ` bibo mao
2025-04-23  9:17       ` Li Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).