[PATCH v6] acpi/virt: suppress UART device & SPCR when guest has no serial hardware

[PATCH v6] acpi/virt: suppress UART device & SPCR when guest has no serial hardware

[Li Chen]

From: Li Chen <chenl311@chinatelecom.cn>

virt machines always instantiate a PL011/16550 UART at slot 0 and describe
it in ACPI (DSDT and optional SPCR table). When the command line disables
the serial backend (e.g. "-serial none"), the guest still sees the UART as
a preferred console even though it is not usable.

Teach the virt ACPI code to omit the UART device and SPCR when there is no
serial backend attached. This matches the hardware that the guest can
actually use and avoids confusing firmware or OS code that relies on SPCR.

The bios-tables-test qtests rely on an ACPI UART node and SPCR entry for
UEFI-based virt machines. To keep those tests working we create a UART
with a "null" chardev backend instead. This preserves the ACPI tables
while discarding the firmware's serial output so it does not corrupt the
TAP stdout stream.

Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>
---
v4->v5:
- Also suppress UART device & SPCR when guest has no serial hardware on loongarch
- rename serial_exist to serial_exists
- fix style issue
v5->v6: 
- Fix: tap parsingg error caused by changing serial to none; now set to null.

 hw/arm/virt-acpi-build.c       | 15 +++++++++------
 hw/loongarch/virt-acpi-build.c |  8 +++++---
 hw/riscv/virt-acpi-build.c     |  8 ++++++--
 include/system/system.h        |  2 ++
 system/vl.c                    |  5 +++++
 tests/qtest/bios-tables-test.c |  6 ++++--
 6 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 200e2a1da7..16d3b63030 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -64,6 +64,7 @@
 #include "hw/virtio/virtio-acpi.h"
 #include "target/arm/cpu.h"
 #include "target/arm/multiprocessing.h"
+#include "system/system.h"
 
 #define ARM_SPI_BASE 32
 
@@ -1031,11 +1032,13 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
      */
     scope = aml_scope("\\_SB");
     acpi_dsdt_add_cpus(scope, vms);
-    acpi_dsdt_add_uart(scope, &memmap[VIRT_UART0],
-                       (irqmap[VIRT_UART0] + ARM_SPI_BASE), 0);
-    if (vms->second_ns_uart_present) {
-        acpi_dsdt_add_uart(scope, &memmap[VIRT_UART1],
-                           (irqmap[VIRT_UART1] + ARM_SPI_BASE), 1);
+    if (serial_exists()) {
+        acpi_dsdt_add_uart(scope, &memmap[VIRT_UART0],
+                           (irqmap[VIRT_UART0] + ARM_SPI_BASE), 0);
+        if (vms->second_ns_uart_present) {
+            acpi_dsdt_add_uart(scope, &memmap[VIRT_UART1],
+                               (irqmap[VIRT_UART1] + ARM_SPI_BASE), 1);
+        }
     }
     if (vmc->acpi_expose_flash) {
         acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
@@ -1184,7 +1187,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
 
     acpi_add_table(table_offsets, tables_blob);
 
-    if (ms->acpi_spcr_enabled) {
+    if (ms->acpi_spcr_enabled && serial_exists()) {
         spcr_setup(tables_blob, tables->linker, vms);
     }
 
diff --git a/hw/loongarch/virt-acpi-build.c b/hw/loongarch/virt-acpi-build.c
index 3694c9827f..6eab907087 100644
--- a/hw/loongarch/virt-acpi-build.c
+++ b/hw/loongarch/virt-acpi-build.c
@@ -484,8 +484,10 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, MachineState *machine)
 
     acpi_table_begin(&table, table_data);
     dsdt = init_aml_allocator();
-    for (i = 0; i < VIRT_UART_COUNT; i++) {
-        build_uart_device_aml(dsdt, i);
+    if (serial_exists()) {
+        for (i = 0; i < VIRT_UART_COUNT; i++) {
+            build_uart_device_aml(dsdt, i);
+        }
     }
     build_pci_device_aml(dsdt, lvms);
     build_la_ged_aml(dsdt, machine);
@@ -557,7 +559,7 @@ static void acpi_build(AcpiBuildTables *tables, MachineState *machine)
     build_srat(tables_blob, tables->linker, machine);
     acpi_add_table(table_offsets, tables_blob);
 
-    if (machine->acpi_spcr_enabled)
+    if (machine->acpi_spcr_enabled && serial_exists())
         spcr_setup(tables_blob, tables->linker, machine);
 
     if (machine->numa_state->num_nodes) {
diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
index f1406cb683..e895bc4ddb 100644
--- a/hw/riscv/virt-acpi-build.c
+++ b/hw/riscv/virt-acpi-build.c
@@ -39,6 +39,7 @@
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "system/reset.h"
+#include "system/system.h"
 
 #define ACPI_BUILD_TABLE_SIZE             0x20000
 #define ACPI_BUILD_INTC_ID(socket, index) ((socket << 24) | (index))
@@ -474,7 +475,10 @@ static void build_dsdt(GArray *table_data,
                                  memmap[VIRT_APLIC_S].size, "RSCV0002");
     }
 
-    acpi_dsdt_add_uart(scope, &memmap[VIRT_UART0], UART0_IRQ);
+    if (serial_exists()) {
+        acpi_dsdt_add_uart(scope, &memmap[VIRT_UART0], UART0_IRQ);
+    }
+
     if (virt_is_iommu_sys_enabled(s)) {
         acpi_dsdt_add_iommu_sys(scope, &memmap[VIRT_IOMMU_SYS], IOMMU_SYS_IRQ);
     }
@@ -890,7 +894,7 @@ static void virt_acpi_build(RISCVVirtState *s, AcpiBuildTables *tables)
 
     acpi_add_table(table_offsets, tables_blob);
 
-    if (ms->acpi_spcr_enabled) {
+    if (ms->acpi_spcr_enabled && serial_exists()) {
         spcr_setup(tables_blob, tables->linker, s);
     }
 
diff --git a/include/system/system.h b/include/system/system.h
index 03a2d0e900..4a03f17e91 100644
--- a/include/system/system.h
+++ b/include/system/system.h
@@ -74,6 +74,8 @@ extern unsigned int nb_prom_envs;
 /* Return the Chardev for serial port i, or NULL if none */
 Chardev *serial_hd(int i);
 
+bool serial_exists(void);
+
 /* parallel ports */
 
 #define MAX_PARALLEL_PORTS 3
diff --git a/system/vl.c b/system/vl.c
index 5091fe52d9..3a2988cb47 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -1487,6 +1487,11 @@ Chardev *serial_hd(int i)
     return NULL;
 }
 
+bool serial_exists(void)
+{
+    return serial_hd(0) ? true : false;
+}
+
 static bool parallel_parse(const char *devname, Error **errp)
 {
     static int index = 0;
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 6b892ef23e..6b0e4be752 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -824,10 +824,12 @@ static char *test_acpi_create_args(test_data *data, const char *params)
         /*
          * TODO: convert '-drive if=pflash' to new syntax (see e33763be7cd3)
          * when arm/virt boad starts to support it.
+         * NOTE: Explicitly add "-serial null" to enable uart in DSDT
+         *       without mixing guest output into TAP stdout.
          */
         if (data->cd) {
             args = g_strdup_printf("-machine %s%s %s -accel tcg "
-                "-nodefaults -nographic "
+                "-nodefaults -serial null -nographic "
                 "-drive if=pflash,format=raw,file=%s,readonly=on "
                 "-drive if=pflash,format=raw,file=%s,snapshot=on -cdrom %s %s",
                 data->machine, data->machine_param ?: "",
@@ -835,7 +837,7 @@ static char *test_acpi_create_args(test_data *data, const char *params)
                 data->uefi_fl1, data->uefi_fl2, data->cd, params ? params : "");
         } else {
             args = g_strdup_printf("-machine %s%s %s -accel tcg "
-                "-nodefaults -nographic "
+                "-nodefaults -serial null -nographic "
                 "-drive if=pflash,format=raw,file=%s,readonly=on "
                 "-drive if=pflash,format=raw,file=%s,snapshot=on %s",
                 data->machine, data->machine_param ?: "",
-- 
2.51.0

Re: [PATCH v6] acpi/virt: suppress UART device & SPCR when guest has no serial hardware

[Michael S. Tsirkin]

On Thu, Dec 11, 2025 at 06:20:25PM +0800, Li Chen wrote:
> From: Li Chen <chenl311@chinatelecom.cn>
> 
> virt machines always instantiate a PL011/16550 UART at slot 0 and describe
> it in ACPI (DSDT and optional SPCR table). When the command line disables
> the serial backend (e.g. "-serial none"), the guest still sees the UART as
> a preferred console even though it is not usable.
> 
> Teach the virt ACPI code to omit the UART device and SPCR when there is no
> serial backend attached. This matches the hardware that the guest can
> actually use and avoids confusing firmware or OS code that relies on SPCR.
> 
> The bios-tables-test qtests rely on an ACPI UART node and SPCR entry for
> UEFI-based virt machines. To keep those tests working we create a UART
> with a "null" chardev backend instead. This preserves the ACPI tables
> while discarding the firmware's serial output so it does not corrupt the
> TAP stdout stream.
> 
> Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
> Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>

This bothers me:
won't this mean number of serial devices changes silently?  So for
example, if you install a guest, see that 1st serial device is broken,
configure it with 2nd one as a work around, now with your change there
is no second one so guest will be broken?


I seems safer to have compat machinery around and avoid
changing this for old machine types.




> ---
> v4->v5:
> - Also suppress UART device & SPCR when guest has no serial hardware on loongarch
> - rename serial_exist to serial_exists
> - fix style issue
> v5->v6: 
> - Fix: tap parsingg error caused by changing serial to none; now set to null.
> 
>  hw/arm/virt-acpi-build.c       | 15 +++++++++------
>  hw/loongarch/virt-acpi-build.c |  8 +++++---
>  hw/riscv/virt-acpi-build.c     |  8 ++++++--
>  include/system/system.h        |  2 ++
>  system/vl.c                    |  5 +++++
>  tests/qtest/bios-tables-test.c |  6 ++++--
>  6 files changed, 31 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 200e2a1da7..16d3b63030 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -64,6 +64,7 @@
>  #include "hw/virtio/virtio-acpi.h"
>  #include "target/arm/cpu.h"
>  #include "target/arm/multiprocessing.h"
> +#include "system/system.h"
>  
>  #define ARM_SPI_BASE 32
>  
> @@ -1031,11 +1032,13 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>       */
>      scope = aml_scope("\\_SB");
>      acpi_dsdt_add_cpus(scope, vms);
> -    acpi_dsdt_add_uart(scope, &memmap[VIRT_UART0],
> -                       (irqmap[VIRT_UART0] + ARM_SPI_BASE), 0);
> -    if (vms->second_ns_uart_present) {
> -        acpi_dsdt_add_uart(scope, &memmap[VIRT_UART1],
> -                           (irqmap[VIRT_UART1] + ARM_SPI_BASE), 1);
> +    if (serial_exists()) {
> +        acpi_dsdt_add_uart(scope, &memmap[VIRT_UART0],
> +                           (irqmap[VIRT_UART0] + ARM_SPI_BASE), 0);
> +        if (vms->second_ns_uart_present) {
> +            acpi_dsdt_add_uart(scope, &memmap[VIRT_UART1],
> +                               (irqmap[VIRT_UART1] + ARM_SPI_BASE), 1);
> +        }
>      }
>      if (vmc->acpi_expose_flash) {
>          acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
> @@ -1184,7 +1187,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
>  
>      acpi_add_table(table_offsets, tables_blob);
>  
> -    if (ms->acpi_spcr_enabled) {
> +    if (ms->acpi_spcr_enabled && serial_exists()) {
>          spcr_setup(tables_blob, tables->linker, vms);
>      }
>  
> diff --git a/hw/loongarch/virt-acpi-build.c b/hw/loongarch/virt-acpi-build.c
> index 3694c9827f..6eab907087 100644
> --- a/hw/loongarch/virt-acpi-build.c
> +++ b/hw/loongarch/virt-acpi-build.c
> @@ -484,8 +484,10 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, MachineState *machine)
>  
>      acpi_table_begin(&table, table_data);
>      dsdt = init_aml_allocator();
> -    for (i = 0; i < VIRT_UART_COUNT; i++) {
> -        build_uart_device_aml(dsdt, i);
> +    if (serial_exists()) {
> +        for (i = 0; i < VIRT_UART_COUNT; i++) {
> +            build_uart_device_aml(dsdt, i);
> +        }
>      }
>      build_pci_device_aml(dsdt, lvms);
>      build_la_ged_aml(dsdt, machine);
> @@ -557,7 +559,7 @@ static void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>      build_srat(tables_blob, tables->linker, machine);
>      acpi_add_table(table_offsets, tables_blob);
>  
> -    if (machine->acpi_spcr_enabled)
> +    if (machine->acpi_spcr_enabled && serial_exists())
>          spcr_setup(tables_blob, tables->linker, machine);
>  
>      if (machine->numa_state->num_nodes) {
> diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
> index f1406cb683..e895bc4ddb 100644
> --- a/hw/riscv/virt-acpi-build.c
> +++ b/hw/riscv/virt-acpi-build.c
> @@ -39,6 +39,7 @@
>  #include "qapi/error.h"
>  #include "qemu/error-report.h"
>  #include "system/reset.h"
> +#include "system/system.h"
>  
>  #define ACPI_BUILD_TABLE_SIZE             0x20000
>  #define ACPI_BUILD_INTC_ID(socket, index) ((socket << 24) | (index))
> @@ -474,7 +475,10 @@ static void build_dsdt(GArray *table_data,
>                                   memmap[VIRT_APLIC_S].size, "RSCV0002");
>      }
>  
> -    acpi_dsdt_add_uart(scope, &memmap[VIRT_UART0], UART0_IRQ);
> +    if (serial_exists()) {
> +        acpi_dsdt_add_uart(scope, &memmap[VIRT_UART0], UART0_IRQ);
> +    }
> +
>      if (virt_is_iommu_sys_enabled(s)) {
>          acpi_dsdt_add_iommu_sys(scope, &memmap[VIRT_IOMMU_SYS], IOMMU_SYS_IRQ);
>      }
> @@ -890,7 +894,7 @@ static void virt_acpi_build(RISCVVirtState *s, AcpiBuildTables *tables)
>  
>      acpi_add_table(table_offsets, tables_blob);
>  
> -    if (ms->acpi_spcr_enabled) {
> +    if (ms->acpi_spcr_enabled && serial_exists()) {
>          spcr_setup(tables_blob, tables->linker, s);
>      }
>  
> diff --git a/include/system/system.h b/include/system/system.h
> index 03a2d0e900..4a03f17e91 100644
> --- a/include/system/system.h
> +++ b/include/system/system.h
> @@ -74,6 +74,8 @@ extern unsigned int nb_prom_envs;
>  /* Return the Chardev for serial port i, or NULL if none */
>  Chardev *serial_hd(int i);
>  
> +bool serial_exists(void);
> +
>  /* parallel ports */
>  
>  #define MAX_PARALLEL_PORTS 3
> diff --git a/system/vl.c b/system/vl.c
> index 5091fe52d9..3a2988cb47 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -1487,6 +1487,11 @@ Chardev *serial_hd(int i)
>      return NULL;
>  }
>  
> +bool serial_exists(void)
> +{
> +    return serial_hd(0) ? true : false;
> +}
> +
>  static bool parallel_parse(const char *devname, Error **errp)
>  {
>      static int index = 0;
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index 6b892ef23e..6b0e4be752 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -824,10 +824,12 @@ static char *test_acpi_create_args(test_data *data, const char *params)
>          /*
>           * TODO: convert '-drive if=pflash' to new syntax (see e33763be7cd3)
>           * when arm/virt boad starts to support it.
> +         * NOTE: Explicitly add "-serial null" to enable uart in DSDT
> +         *       without mixing guest output into TAP stdout.
>           */
>          if (data->cd) {
>              args = g_strdup_printf("-machine %s%s %s -accel tcg "
> -                "-nodefaults -nographic "
> +                "-nodefaults -serial null -nographic "
>                  "-drive if=pflash,format=raw,file=%s,readonly=on "
>                  "-drive if=pflash,format=raw,file=%s,snapshot=on -cdrom %s %s",
>                  data->machine, data->machine_param ?: "",
> @@ -835,7 +837,7 @@ static char *test_acpi_create_args(test_data *data, const char *params)
>                  data->uefi_fl1, data->uefi_fl2, data->cd, params ? params : "");
>          } else {
>              args = g_strdup_printf("-machine %s%s %s -accel tcg "
> -                "-nodefaults -nographic "
> +                "-nodefaults -serial null -nographic "
>                  "-drive if=pflash,format=raw,file=%s,readonly=on "
>                  "-drive if=pflash,format=raw,file=%s,snapshot=on %s",
>                  data->machine, data->machine_param ?: "",
> -- 
> 2.51.0

Re: [PATCH v6] acpi/virt: suppress UART device & SPCR when guest has no serial hardware

[Li Chen]

Hi Michael,

 ---- On Thu, 11 Dec 2025 21:10:18 +0800  Michael S. Tsirkin <mst@redhat.com> wrote --- 
 > On Thu, Dec 11, 2025 at 06:20:25PM +0800, Li Chen wrote:
 > > From: Li Chen <chenl311@chinatelecom.cn>
 > > 
 > > virt machines always instantiate a PL011/16550 UART at slot 0 and describe
 > > it in ACPI (DSDT and optional SPCR table). When the command line disables
 > > the serial backend (e.g. "-serial none"), the guest still sees the UART as
 > > a preferred console even though it is not usable.
 > > 
 > > Teach the virt ACPI code to omit the UART device and SPCR when there is no
 > > serial backend attached. This matches the hardware that the guest can
 > > actually use and avoids confusing firmware or OS code that relies on SPCR.
 > > 
 > > The bios-tables-test qtests rely on an ACPI UART node and SPCR entry for
 > > UEFI-based virt machines. To keep those tests working we create a UART
 > > with a "null" chardev backend instead. This preserves the ACPI tables
 > > while discarding the firmware's serial output so it does not corrupt the
 > > TAP stdout stream.
 > > 
 > > Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
 > > Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
 > > Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>
 > 
 > This bothers me:
 > won't this mean number of serial devices changes silently?  So for
 > example, if you install a guest, see that 1st serial device is broken,
 > configure it with 2nd one as a work around, now with your change there
 > is no second one so guest will be broken?
 > 
 > 
 > I seems safer to have compat machinery around and avoid
 > changing this for old machine types.
 
Thanks for pointing this out, you’re right that changing the number of serial 
devices that the guest sees is risky for existing machine types.

For v7 I’m planning to take a more conservative approach and only gate SPCR
on the presence of a backend for the primary UART, while  keeping the ACPI
UART devices in DSDT unchanged. 
Concretely:
- virt ACPI DSDT will continue to unconditionally describe the UART(s) as before,
   so the guest-visible UART topology doesn’t change for existing virt-* machines.
- We only call spcr_setup() when there is a backend attached to UART0 (e.g. serial_hd(0) != NULL),
  so SPCR doesn’t point at a completely unusable console when the user passes -serial none.

In particular, for a command line like -serial none -serial pty, this means SPCR would not be present
(rather than silently switching it to UART1/pty). My reasoning is that, for existing virt machines,
SPCR is meant to describe “the board’s primary console (UART0)”, not “the first UART that happens
to have a host backend”, and changing that interpretation feels like it would need compat machinery
and a new machine type.

The qtests already force -serial null for the UEFI-based virt tests, so they will still exercise the SPCR path.

Would this behaviour be acceptable for you, or would you prefer that I go further and also add a compat
flag/new machine type where SPCR can follow “the first UART with a backend” while old virt-* machines
keep the current UART/SPCR semantics?

Regards,
Li​