* Re: [PATCH v2 2/3] hw/acpi: Upgrade ACPI SPCR table to support SPCR table version 4 format
2024-05-07 5:22 ` [PATCH v2 2/3] hw/acpi: Upgrade ACPI SPCR table to support SPCR table version 4 format Sia Jee Heng
@ 2024-05-13 4:01 ` Alistair Francis
2024-05-13 5:16 ` Sunil V L
2024-05-13 6:31 ` Michael S. Tsirkin
2 siblings, 0 replies; 13+ messages in thread
From: Alistair Francis @ 2024-05-13 4:01 UTC (permalink / raw)
To: Sia Jee Heng
Cc: qemu-arm, qemu-devel, qemu-riscv, mst, imammedo, anisinha,
peter.maydell, shannon.zhaosl, sunilvl, palmer, alistair.francis,
bin.meng, liwei1518, dbarboza, zhiwei_liu
On Tue, May 7, 2024 at 3:24 PM Sia Jee Heng
<jeeheng.sia@starfivetech.com> wrote:
>
> Update the SPCR table to accommodate the SPCR Table version 4 [1].
> The SPCR table has been modified to adhere to the version 4 format [2].
>
> [1]: https://learn.microsoft.com/en-us/windows-hardware/drivers/serports/serial-port-console-redirection-table
> [2]: https://github.com/acpica/acpica/pull/931
>
> Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/acpi/aml-build.c | 14 +++++++++++---
> hw/arm/virt-acpi-build.c | 10 ++++++++--
> hw/riscv/virt-acpi-build.c | 12 +++++++++---
> include/hw/acpi/acpi-defs.h | 7 +++++--
> include/hw/acpi/aml-build.h | 2 +-
> 5 files changed, 34 insertions(+), 11 deletions(-)
>
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 6d4517cfbe..7c43573eef 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1996,7 +1996,7 @@ static void build_processor_hierarchy_node(GArray *tbl, uint32_t flags,
>
> void build_spcr(GArray *table_data, BIOSLinker *linker,
> const AcpiSpcrData *f, const uint8_t rev,
> - const char *oem_id, const char *oem_table_id)
> + const char *oem_id, const char *oem_table_id, const char *name)
> {
> AcpiTable table = { .sig = "SPCR", .rev = rev, .oem_id = oem_id,
> .oem_table_id = oem_table_id };
> @@ -2042,8 +2042,16 @@ void build_spcr(GArray *table_data, BIOSLinker *linker,
> build_append_int_noprefix(table_data, f->pci_flags, 4);
> /* PCI Segment */
> build_append_int_noprefix(table_data, f->pci_segment, 1);
> - /* Reserved */
> - build_append_int_noprefix(table_data, 0, 4);
> + /* UartClkFreq */
> + build_append_int_noprefix(table_data, f->uart_clk_freq, 4);
> + /* PreciseBaudrate */
> + build_append_int_noprefix(table_data, f->precise_baudrate, 4);
> + /* NameSpaceStringLength */
> + build_append_int_noprefix(table_data, f->namespace_string_length, 2);
> + /* NameSpaceStringOffset */
> + build_append_int_noprefix(table_data, f->namespace_string_offset, 2);
> + /* NamespaceString[] */
> + g_array_append_vals(table_data, name, f->namespace_string_length);
>
> acpi_table_end(linker, &table);
> }
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 6a1bde61ce..cb345e8659 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -428,11 +428,12 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>
> /*
> * Serial Port Console Redirection Table (SPCR)
> - * Rev: 1.07
> + * Rev: 1.10
> */
> static void
> spcr_setup(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> {
> + const char name[] = ".";
> AcpiSpcrData serial = {
> .interface_type = 3, /* ARM PL011 UART */
> .base_addr.id = AML_AS_SYSTEM_MEMORY,
> @@ -456,9 +457,14 @@ spcr_setup(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> .pci_function = 0,
> .pci_flags = 0,
> .pci_segment = 0,
> + .uart_clk_freq = 0,
> + .precise_baudrate = 0,
> + .namespace_string_length = sizeof(name),
> + .namespace_string_offset = 88,
> };
>
> - build_spcr(table_data, linker, &serial, 2, vms->oem_id, vms->oem_table_id);
> + build_spcr(table_data, linker, &serial, 4, vms->oem_id, vms->oem_table_id,
> + name);
> }
>
> /*
> diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
> index 0925528160..5fa3942491 100644
> --- a/hw/riscv/virt-acpi-build.c
> +++ b/hw/riscv/virt-acpi-build.c
> @@ -176,14 +176,15 @@ acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
>
> /*
> * Serial Port Console Redirection Table (SPCR)
> - * Rev: 1.07
> + * Rev: 1.10
> */
>
> static void
> spcr_setup(GArray *table_data, BIOSLinker *linker, RISCVVirtState *s)
> {
> + const char name[] = ".";
> AcpiSpcrData serial = {
> - .interface_type = 0, /* 16550 compatible */
> + .interface_type = 0x12, /* 16550 compatible */
> .base_addr.id = AML_AS_SYSTEM_MEMORY,
> .base_addr.width = 32,
> .base_addr.offset = 0,
> @@ -205,9 +206,14 @@ spcr_setup(GArray *table_data, BIOSLinker *linker, RISCVVirtState *s)
> .pci_function = 0,
> .pci_flags = 0,
> .pci_segment = 0,
> + .uart_clk_freq = 0,
> + .precise_baudrate = 0,
> + .namespace_string_length = sizeof(name),
> + .namespace_string_offset = 88,
> };
>
> - build_spcr(table_data, linker, &serial, 2, s->oem_id, s->oem_table_id);
> + build_spcr(table_data, linker, &serial, 4, s->oem_id, s->oem_table_id,
> + name);
> }
>
> /* RHCT Node[N] starts at offset 56 */
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index 0e6e82b339..2e6e341998 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -112,7 +112,6 @@ typedef struct AcpiSpcrData {
> uint8_t flow_control;
> uint8_t terminal_type;
> uint8_t language;
> - uint8_t reserved1;
> uint16_t pci_device_id; /* Must be 0xffff if not PCI device */
> uint16_t pci_vendor_id; /* Must be 0xffff if not PCI device */
> uint8_t pci_bus;
> @@ -120,7 +119,11 @@ typedef struct AcpiSpcrData {
> uint8_t pci_function;
> uint32_t pci_flags;
> uint8_t pci_segment;
> - uint32_t reserved2;
> + uint32_t uart_clk_freq;
> + uint32_t precise_baudrate;
> + uint32_t namespace_string_length;
> + uint32_t namespace_string_offset;
> + char namespace_string[];
> } AcpiSpcrData;
>
> #define ACPI_FADT_ARM_PSCI_COMPLIANT (1 << 0)
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index a3784155cb..68c0f2dbee 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -500,5 +500,5 @@ void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
>
> void build_spcr(GArray *table_data, BIOSLinker *linker,
> const AcpiSpcrData *f, const uint8_t rev,
> - const char *oem_id, const char *oem_table_id);
> + const char *oem_id, const char *oem_table_id, const char *name);
> #endif
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 2/3] hw/acpi: Upgrade ACPI SPCR table to support SPCR table version 4 format
2024-05-07 5:22 ` [PATCH v2 2/3] hw/acpi: Upgrade ACPI SPCR table to support SPCR table version 4 format Sia Jee Heng
2024-05-13 4:01 ` Alistair Francis
@ 2024-05-13 5:16 ` Sunil V L
2024-05-14 11:16 ` JeeHeng Sia
2024-05-13 6:31 ` Michael S. Tsirkin
2 siblings, 1 reply; 13+ messages in thread
From: Sunil V L @ 2024-05-13 5:16 UTC (permalink / raw)
To: Sia Jee Heng
Cc: qemu-arm, qemu-devel, qemu-riscv, mst, imammedo, anisinha,
peter.maydell, shannon.zhaosl, palmer, alistair.francis, bin.meng,
liwei1518, dbarboza, zhiwei_liu
Hi Sia Jee Heng,
On Mon, May 06, 2024 at 10:22:11PM -0700, Sia Jee Heng wrote:
> Update the SPCR table to accommodate the SPCR Table version 4 [1].
> The SPCR table has been modified to adhere to the version 4 format [2].
>
> [1]: https://learn.microsoft.com/en-us/windows-hardware/drivers/serports/serial-port-console-redirection-table
> [2]: https://github.com/acpica/acpica/pull/931
>
> Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
> ---
> hw/acpi/aml-build.c | 14 +++++++++++---
> hw/arm/virt-acpi-build.c | 10 ++++++++--
> hw/riscv/virt-acpi-build.c | 12 +++++++++---
> include/hw/acpi/acpi-defs.h | 7 +++++--
> include/hw/acpi/aml-build.h | 2 +-
> 5 files changed, 34 insertions(+), 11 deletions(-)
>
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 6d4517cfbe..7c43573eef 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1996,7 +1996,7 @@ static void build_processor_hierarchy_node(GArray *tbl, uint32_t flags,
>
> void build_spcr(GArray *table_data, BIOSLinker *linker,
> const AcpiSpcrData *f, const uint8_t rev,
> - const char *oem_id, const char *oem_table_id)
> + const char *oem_id, const char *oem_table_id, const char *name)
> {
> AcpiTable table = { .sig = "SPCR", .rev = rev, .oem_id = oem_id,
> .oem_table_id = oem_table_id };
> @@ -2042,8 +2042,16 @@ void build_spcr(GArray *table_data, BIOSLinker *linker,
> build_append_int_noprefix(table_data, f->pci_flags, 4);
> /* PCI Segment */
> build_append_int_noprefix(table_data, f->pci_segment, 1);
> - /* Reserved */
> - build_append_int_noprefix(table_data, 0, 4);
> + /* UartClkFreq */
> + build_append_int_noprefix(table_data, f->uart_clk_freq, 4);
> + /* PreciseBaudrate */
> + build_append_int_noprefix(table_data, f->precise_baudrate, 4);
> + /* NameSpaceStringLength */
> + build_append_int_noprefix(table_data, f->namespace_string_length, 2);
> + /* NameSpaceStringOffset */
> + build_append_int_noprefix(table_data, f->namespace_string_offset, 2);
> + /* NamespaceString[] */
> + g_array_append_vals(table_data, name, f->namespace_string_length);
>
Is it possible to check the revision here and add new fields only if the
revision supports it? ARM maintainers are better to comment but IMO, we
better keep ARM's SPCR in the same current version since I don't know
how consumers like linux (and other OSs) react to the change.
Thanks!
Sunil
> acpi_table_end(linker, &table);
> }
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 6a1bde61ce..cb345e8659 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -428,11 +428,12 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>
> /*
> * Serial Port Console Redirection Table (SPCR)
> - * Rev: 1.07
> + * Rev: 1.10
> */
> static void
> spcr_setup(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> {
> + const char name[] = ".";
> AcpiSpcrData serial = {
> .interface_type = 3, /* ARM PL011 UART */
> .base_addr.id = AML_AS_SYSTEM_MEMORY,
> @@ -456,9 +457,14 @@ spcr_setup(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> .pci_function = 0,
> .pci_flags = 0,
> .pci_segment = 0,
> + .uart_clk_freq = 0,
> + .precise_baudrate = 0,
> + .namespace_string_length = sizeof(name),
> + .namespace_string_offset = 88,
> };
>
> - build_spcr(table_data, linker, &serial, 2, vms->oem_id, vms->oem_table_id);
> + build_spcr(table_data, linker, &serial, 4, vms->oem_id, vms->oem_table_id,
> + name);
> }
>
> /*
> diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
> index 0925528160..5fa3942491 100644
> --- a/hw/riscv/virt-acpi-build.c
> +++ b/hw/riscv/virt-acpi-build.c
> @@ -176,14 +176,15 @@ acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
>
> /*
> * Serial Port Console Redirection Table (SPCR)
> - * Rev: 1.07
> + * Rev: 1.10
> */
>
> static void
> spcr_setup(GArray *table_data, BIOSLinker *linker, RISCVVirtState *s)
> {
> + const char name[] = ".";
> AcpiSpcrData serial = {
> - .interface_type = 0, /* 16550 compatible */
> + .interface_type = 0x12, /* 16550 compatible */
> .base_addr.id = AML_AS_SYSTEM_MEMORY,
> .base_addr.width = 32,
> .base_addr.offset = 0,
> @@ -205,9 +206,14 @@ spcr_setup(GArray *table_data, BIOSLinker *linker, RISCVVirtState *s)
> .pci_function = 0,
> .pci_flags = 0,
> .pci_segment = 0,
> + .uart_clk_freq = 0,
> + .precise_baudrate = 0,
> + .namespace_string_length = sizeof(name),
> + .namespace_string_offset = 88,
> };
>
> - build_spcr(table_data, linker, &serial, 2, s->oem_id, s->oem_table_id);
> + build_spcr(table_data, linker, &serial, 4, s->oem_id, s->oem_table_id,
> + name);
> }
>
> /* RHCT Node[N] starts at offset 56 */
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index 0e6e82b339..2e6e341998 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -112,7 +112,6 @@ typedef struct AcpiSpcrData {
> uint8_t flow_control;
> uint8_t terminal_type;
> uint8_t language;
> - uint8_t reserved1;
> uint16_t pci_device_id; /* Must be 0xffff if not PCI device */
> uint16_t pci_vendor_id; /* Must be 0xffff if not PCI device */
> uint8_t pci_bus;
> @@ -120,7 +119,11 @@ typedef struct AcpiSpcrData {
> uint8_t pci_function;
> uint32_t pci_flags;
> uint8_t pci_segment;
> - uint32_t reserved2;
> + uint32_t uart_clk_freq;
> + uint32_t precise_baudrate;
> + uint32_t namespace_string_length;
> + uint32_t namespace_string_offset;
> + char namespace_string[];
> } AcpiSpcrData;
>
> #define ACPI_FADT_ARM_PSCI_COMPLIANT (1 << 0)
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index a3784155cb..68c0f2dbee 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -500,5 +500,5 @@ void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
>
> void build_spcr(GArray *table_data, BIOSLinker *linker,
> const AcpiSpcrData *f, const uint8_t rev,
> - const char *oem_id, const char *oem_table_id);
> + const char *oem_id, const char *oem_table_id, const char *name);
> #endif
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread* RE: [PATCH v2 2/3] hw/acpi: Upgrade ACPI SPCR table to support SPCR table version 4 format
2024-05-13 5:16 ` Sunil V L
@ 2024-05-14 11:16 ` JeeHeng Sia
0 siblings, 0 replies; 13+ messages in thread
From: JeeHeng Sia @ 2024-05-14 11:16 UTC (permalink / raw)
To: Sunil V L
Cc: qemu-arm@nongnu.org, qemu-devel@nongnu.org, qemu-riscv@nongnu.org,
mst@redhat.com, imammedo@redhat.com, anisinha@redhat.com,
peter.maydell@linaro.org, shannon.zhaosl@gmail.com,
palmer@dabbelt.com, alistair.francis@wdc.com,
bin.meng@windriver.com, liwei1518@gmail.com,
dbarboza@ventanamicro.com, zhiwei_liu@linux.alibaba.com
> -----Original Message-----
> From: Sunil V L <sunilvl@ventanamicro.com>
> Sent: Monday, May 13, 2024 1:16 PM
> To: JeeHeng Sia <jeeheng.sia@starfivetech.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org; qemu-riscv@nongnu.org; mst@redhat.com; imammedo@redhat.com;
> anisinha@redhat.com; peter.maydell@linaro.org; shannon.zhaosl@gmail.com; palmer@dabbelt.com; alistair.francis@wdc.com;
> bin.meng@windriver.com; liwei1518@gmail.com; dbarboza@ventanamicro.com; zhiwei_liu@linux.alibaba.com
> Subject: Re: [PATCH v2 2/3] hw/acpi: Upgrade ACPI SPCR table to support SPCR table version 4 format
>
> Hi Sia Jee Heng,
>
> On Mon, May 06, 2024 at 10:22:11PM -0700, Sia Jee Heng wrote:
> > Update the SPCR table to accommodate the SPCR Table version 4 [1].
> > The SPCR table has been modified to adhere to the version 4 format [2].
> >
> > [1]: https://learn.microsoft.com/en-us/windows-hardware/drivers/serports/serial-port-console-redirection-table
> > [2]: https://github.com/acpica/acpica/pull/931
> >
> > Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
> > ---
> > hw/acpi/aml-build.c | 14 +++++++++++---
> > hw/arm/virt-acpi-build.c | 10 ++++++++--
> > hw/riscv/virt-acpi-build.c | 12 +++++++++---
> > include/hw/acpi/acpi-defs.h | 7 +++++--
> > include/hw/acpi/aml-build.h | 2 +-
> > 5 files changed, 34 insertions(+), 11 deletions(-)
> >
> > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > index 6d4517cfbe..7c43573eef 100644
> > --- a/hw/acpi/aml-build.c
> > +++ b/hw/acpi/aml-build.c
> > @@ -1996,7 +1996,7 @@ static void build_processor_hierarchy_node(GArray *tbl, uint32_t flags,
> >
> > void build_spcr(GArray *table_data, BIOSLinker *linker,
> > const AcpiSpcrData *f, const uint8_t rev,
> > - const char *oem_id, const char *oem_table_id)
> > + const char *oem_id, const char *oem_table_id, const char *name)
> > {
> > AcpiTable table = { .sig = "SPCR", .rev = rev, .oem_id = oem_id,
> > .oem_table_id = oem_table_id };
> > @@ -2042,8 +2042,16 @@ void build_spcr(GArray *table_data, BIOSLinker *linker,
> > build_append_int_noprefix(table_data, f->pci_flags, 4);
> > /* PCI Segment */
> > build_append_int_noprefix(table_data, f->pci_segment, 1);
> > - /* Reserved */
> > - build_append_int_noprefix(table_data, 0, 4);
> > + /* UartClkFreq */
> > + build_append_int_noprefix(table_data, f->uart_clk_freq, 4);
> > + /* PreciseBaudrate */
> > + build_append_int_noprefix(table_data, f->precise_baudrate, 4);
> > + /* NameSpaceStringLength */
> > + build_append_int_noprefix(table_data, f->namespace_string_length, 2);
> > + /* NameSpaceStringOffset */
> > + build_append_int_noprefix(table_data, f->namespace_string_offset, 2);
> > + /* NamespaceString[] */
> > + g_array_append_vals(table_data, name, f->namespace_string_length);
> >
> Is it possible to check the revision here and add new fields only if the
> revision supports it? ARM maintainers are better to comment but IMO, we
> better keep ARM's SPCR in the same current version since I don't know
> how consumers like linux (and other OSs) react to the change.
Hi Sunil, there is only one prebuilt SPCR binary. By doing this, the qtest
will fail. I think I will let ARM maintainer to comment.
>
> Thanks!
> Sunil
> > acpi_table_end(linker, &table);
> > }
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index 6a1bde61ce..cb345e8659 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -428,11 +428,12 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> >
> > /*
> > * Serial Port Console Redirection Table (SPCR)
> > - * Rev: 1.07
> > + * Rev: 1.10
> > */
> > static void
> > spcr_setup(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> > {
> > + const char name[] = ".";
> > AcpiSpcrData serial = {
> > .interface_type = 3, /* ARM PL011 UART */
> > .base_addr.id = AML_AS_SYSTEM_MEMORY,
> > @@ -456,9 +457,14 @@ spcr_setup(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> > .pci_function = 0,
> > .pci_flags = 0,
> > .pci_segment = 0,
> > + .uart_clk_freq = 0,
> > + .precise_baudrate = 0,
> > + .namespace_string_length = sizeof(name),
> > + .namespace_string_offset = 88,
> > };
> >
> > - build_spcr(table_data, linker, &serial, 2, vms->oem_id, vms->oem_table_id);
> > + build_spcr(table_data, linker, &serial, 4, vms->oem_id, vms->oem_table_id,
> > + name);
> > }
> >
> > /*
> > diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
> > index 0925528160..5fa3942491 100644
> > --- a/hw/riscv/virt-acpi-build.c
> > +++ b/hw/riscv/virt-acpi-build.c
> > @@ -176,14 +176,15 @@ acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
> >
> > /*
> > * Serial Port Console Redirection Table (SPCR)
> > - * Rev: 1.07
> > + * Rev: 1.10
> > */
> >
> > static void
> > spcr_setup(GArray *table_data, BIOSLinker *linker, RISCVVirtState *s)
> > {
> > + const char name[] = ".";
> > AcpiSpcrData serial = {
> > - .interface_type = 0, /* 16550 compatible */
> > + .interface_type = 0x12, /* 16550 compatible */
> > .base_addr.id = AML_AS_SYSTEM_MEMORY,
> > .base_addr.width = 32,
> > .base_addr.offset = 0,
> > @@ -205,9 +206,14 @@ spcr_setup(GArray *table_data, BIOSLinker *linker, RISCVVirtState *s)
> > .pci_function = 0,
> > .pci_flags = 0,
> > .pci_segment = 0,
> > + .uart_clk_freq = 0,
> > + .precise_baudrate = 0,
> > + .namespace_string_length = sizeof(name),
> > + .namespace_string_offset = 88,
> > };
> >
> > - build_spcr(table_data, linker, &serial, 2, s->oem_id, s->oem_table_id);
> > + build_spcr(table_data, linker, &serial, 4, s->oem_id, s->oem_table_id,
> > + name);
> > }
> >
> > /* RHCT Node[N] starts at offset 56 */
> > diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> > index 0e6e82b339..2e6e341998 100644
> > --- a/include/hw/acpi/acpi-defs.h
> > +++ b/include/hw/acpi/acpi-defs.h
> > @@ -112,7 +112,6 @@ typedef struct AcpiSpcrData {
> > uint8_t flow_control;
> > uint8_t terminal_type;
> > uint8_t language;
> > - uint8_t reserved1;
> > uint16_t pci_device_id; /* Must be 0xffff if not PCI device */
> > uint16_t pci_vendor_id; /* Must be 0xffff if not PCI device */
> > uint8_t pci_bus;
> > @@ -120,7 +119,11 @@ typedef struct AcpiSpcrData {
> > uint8_t pci_function;
> > uint32_t pci_flags;
> > uint8_t pci_segment;
> > - uint32_t reserved2;
> > + uint32_t uart_clk_freq;
> > + uint32_t precise_baudrate;
> > + uint32_t namespace_string_length;
> > + uint32_t namespace_string_offset;
> > + char namespace_string[];
> > } AcpiSpcrData;
> >
> > #define ACPI_FADT_ARM_PSCI_COMPLIANT (1 << 0)
> > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> > index a3784155cb..68c0f2dbee 100644
> > --- a/include/hw/acpi/aml-build.h
> > +++ b/include/hw/acpi/aml-build.h
> > @@ -500,5 +500,5 @@ void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
> >
> > void build_spcr(GArray *table_data, BIOSLinker *linker,
> > const AcpiSpcrData *f, const uint8_t rev,
> > - const char *oem_id, const char *oem_table_id);
> > + const char *oem_id, const char *oem_table_id, const char *name);
> > #endif
> > --
> > 2.34.1
> >
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/3] hw/acpi: Upgrade ACPI SPCR table to support SPCR table version 4 format
2024-05-07 5:22 ` [PATCH v2 2/3] hw/acpi: Upgrade ACPI SPCR table to support SPCR table version 4 format Sia Jee Heng
2024-05-13 4:01 ` Alistair Francis
2024-05-13 5:16 ` Sunil V L
@ 2024-05-13 6:31 ` Michael S. Tsirkin
2024-05-14 11:18 ` JeeHeng Sia
2 siblings, 1 reply; 13+ messages in thread
From: Michael S. Tsirkin @ 2024-05-13 6:31 UTC (permalink / raw)
To: Sia Jee Heng
Cc: qemu-arm, qemu-devel, qemu-riscv, imammedo, anisinha,
peter.maydell, shannon.zhaosl, sunilvl, palmer, alistair.francis,
bin.meng, liwei1518, dbarboza, zhiwei_liu
On Mon, May 06, 2024 at 10:22:11PM -0700, Sia Jee Heng wrote:
> Update the SPCR table to accommodate the SPCR Table version 4 [1].
> The SPCR table has been modified to adhere to the version 4 format [2].
>
> [1]: https://learn.microsoft.com/en-us/windows-hardware/drivers/serports/serial-port-console-redirection-table
> [2]: https://github.com/acpica/acpica/pull/931
>
> Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
What does this achieve? We don't normally change unless it gets
us some useful feature.
> ---
> hw/acpi/aml-build.c | 14 +++++++++++---
> hw/arm/virt-acpi-build.c | 10 ++++++++--
> hw/riscv/virt-acpi-build.c | 12 +++++++++---
> include/hw/acpi/acpi-defs.h | 7 +++++--
> include/hw/acpi/aml-build.h | 2 +-
> 5 files changed, 34 insertions(+), 11 deletions(-)
>
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 6d4517cfbe..7c43573eef 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1996,7 +1996,7 @@ static void build_processor_hierarchy_node(GArray *tbl, uint32_t flags,
>
> void build_spcr(GArray *table_data, BIOSLinker *linker,
> const AcpiSpcrData *f, const uint8_t rev,
> - const char *oem_id, const char *oem_table_id)
> + const char *oem_id, const char *oem_table_id, const char *name)
> {
> AcpiTable table = { .sig = "SPCR", .rev = rev, .oem_id = oem_id,
> .oem_table_id = oem_table_id };
> @@ -2042,8 +2042,16 @@ void build_spcr(GArray *table_data, BIOSLinker *linker,
> build_append_int_noprefix(table_data, f->pci_flags, 4);
> /* PCI Segment */
> build_append_int_noprefix(table_data, f->pci_segment, 1);
> - /* Reserved */
> - build_append_int_noprefix(table_data, 0, 4);
> + /* UartClkFreq */
> + build_append_int_noprefix(table_data, f->uart_clk_freq, 4);
> + /* PreciseBaudrate */
> + build_append_int_noprefix(table_data, f->precise_baudrate, 4);
> + /* NameSpaceStringLength */
> + build_append_int_noprefix(table_data, f->namespace_string_length, 2);
> + /* NameSpaceStringOffset */
> + build_append_int_noprefix(table_data, f->namespace_string_offset, 2);
> + /* NamespaceString[] */
> + g_array_append_vals(table_data, name, f->namespace_string_length);
>
> acpi_table_end(linker, &table);
> }
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 6a1bde61ce..cb345e8659 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -428,11 +428,12 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>
> /*
> * Serial Port Console Redirection Table (SPCR)
> - * Rev: 1.07
> + * Rev: 1.10
> */
> static void
> spcr_setup(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> {
> + const char name[] = ".";
> AcpiSpcrData serial = {
> .interface_type = 3, /* ARM PL011 UART */
> .base_addr.id = AML_AS_SYSTEM_MEMORY,
> @@ -456,9 +457,14 @@ spcr_setup(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> .pci_function = 0,
> .pci_flags = 0,
> .pci_segment = 0,
> + .uart_clk_freq = 0,
> + .precise_baudrate = 0,
> + .namespace_string_length = sizeof(name),
> + .namespace_string_offset = 88,
> };
>
> - build_spcr(table_data, linker, &serial, 2, vms->oem_id, vms->oem_table_id);
> + build_spcr(table_data, linker, &serial, 4, vms->oem_id, vms->oem_table_id,
> + name);
> }
>
> /*
> diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
> index 0925528160..5fa3942491 100644
> --- a/hw/riscv/virt-acpi-build.c
> +++ b/hw/riscv/virt-acpi-build.c
> @@ -176,14 +176,15 @@ acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
>
> /*
> * Serial Port Console Redirection Table (SPCR)
> - * Rev: 1.07
> + * Rev: 1.10
> */
>
> static void
> spcr_setup(GArray *table_data, BIOSLinker *linker, RISCVVirtState *s)
> {
> + const char name[] = ".";
> AcpiSpcrData serial = {
> - .interface_type = 0, /* 16550 compatible */
> + .interface_type = 0x12, /* 16550 compatible */
> .base_addr.id = AML_AS_SYSTEM_MEMORY,
> .base_addr.width = 32,
> .base_addr.offset = 0,
> @@ -205,9 +206,14 @@ spcr_setup(GArray *table_data, BIOSLinker *linker, RISCVVirtState *s)
> .pci_function = 0,
> .pci_flags = 0,
> .pci_segment = 0,
> + .uart_clk_freq = 0,
> + .precise_baudrate = 0,
> + .namespace_string_length = sizeof(name),
> + .namespace_string_offset = 88,
> };
>
> - build_spcr(table_data, linker, &serial, 2, s->oem_id, s->oem_table_id);
> + build_spcr(table_data, linker, &serial, 4, s->oem_id, s->oem_table_id,
> + name);
> }
>
> /* RHCT Node[N] starts at offset 56 */
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index 0e6e82b339..2e6e341998 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -112,7 +112,6 @@ typedef struct AcpiSpcrData {
> uint8_t flow_control;
> uint8_t terminal_type;
> uint8_t language;
> - uint8_t reserved1;
> uint16_t pci_device_id; /* Must be 0xffff if not PCI device */
> uint16_t pci_vendor_id; /* Must be 0xffff if not PCI device */
> uint8_t pci_bus;
> @@ -120,7 +119,11 @@ typedef struct AcpiSpcrData {
> uint8_t pci_function;
> uint32_t pci_flags;
> uint8_t pci_segment;
> - uint32_t reserved2;
> + uint32_t uart_clk_freq;
> + uint32_t precise_baudrate;
> + uint32_t namespace_string_length;
> + uint32_t namespace_string_offset;
> + char namespace_string[];
> } AcpiSpcrData;
>
> #define ACPI_FADT_ARM_PSCI_COMPLIANT (1 << 0)
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index a3784155cb..68c0f2dbee 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -500,5 +500,5 @@ void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
>
> void build_spcr(GArray *table_data, BIOSLinker *linker,
> const AcpiSpcrData *f, const uint8_t rev,
> - const char *oem_id, const char *oem_table_id);
> + const char *oem_id, const char *oem_table_id, const char *name);
> #endif
> --
> 2.34.1
^ permalink raw reply [flat|nested] 13+ messages in thread* RE: [PATCH v2 2/3] hw/acpi: Upgrade ACPI SPCR table to support SPCR table version 4 format
2024-05-13 6:31 ` Michael S. Tsirkin
@ 2024-05-14 11:18 ` JeeHeng Sia
0 siblings, 0 replies; 13+ messages in thread
From: JeeHeng Sia @ 2024-05-14 11:18 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: qemu-arm@nongnu.org, qemu-devel@nongnu.org, qemu-riscv@nongnu.org,
imammedo@redhat.com, anisinha@redhat.com,
peter.maydell@linaro.org, shannon.zhaosl@gmail.com,
sunilvl@ventanamicro.com, palmer@dabbelt.com,
alistair.francis@wdc.com, bin.meng@windriver.com,
liwei1518@gmail.com, dbarboza@ventanamicro.com,
zhiwei_liu@linux.alibaba.com
> -----Original Message-----
> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Monday, May 13, 2024 2:31 PM
> To: JeeHeng Sia <jeeheng.sia@starfivetech.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org; qemu-riscv@nongnu.org; imammedo@redhat.com; anisinha@redhat.com;
> peter.maydell@linaro.org; shannon.zhaosl@gmail.com; sunilvl@ventanamicro.com; palmer@dabbelt.com; alistair.francis@wdc.com;
> bin.meng@windriver.com; liwei1518@gmail.com; dbarboza@ventanamicro.com; zhiwei_liu@linux.alibaba.com
> Subject: Re: [PATCH v2 2/3] hw/acpi: Upgrade ACPI SPCR table to support SPCR table version 4 format
>
> On Mon, May 06, 2024 at 10:22:11PM -0700, Sia Jee Heng wrote:
> > Update the SPCR table to accommodate the SPCR Table version 4 [1].
> > The SPCR table has been modified to adhere to the version 4 format [2].
> >
> > [1]: https://learn.microsoft.com/en-us/windows-hardware/drivers/serports/serial-port-console-redirection-table
> > [2]: https://github.com/acpica/acpica/pull/931
> >
> > Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
>
> What does this achieve? We don't normally change unless it gets
> us some useful feature.
The changes in Table 4 primarily include the addition of RISC-V support
and modifications to the Interrupt Type field. Additionally, new fields
added are Precise Baud Rate, NamespaceStringLength, NamespaceStringOffset,
and NamespaceString[].
>
>
> > ---
> > hw/acpi/aml-build.c | 14 +++++++++++---
> > hw/arm/virt-acpi-build.c | 10 ++++++++--
> > hw/riscv/virt-acpi-build.c | 12 +++++++++---
> > include/hw/acpi/acpi-defs.h | 7 +++++--
> > include/hw/acpi/aml-build.h | 2 +-
> > 5 files changed, 34 insertions(+), 11 deletions(-)
> >
> > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > index 6d4517cfbe..7c43573eef 100644
> > --- a/hw/acpi/aml-build.c
> > +++ b/hw/acpi/aml-build.c
> > @@ -1996,7 +1996,7 @@ static void build_processor_hierarchy_node(GArray *tbl, uint32_t flags,
> >
> > void build_spcr(GArray *table_data, BIOSLinker *linker,
> > const AcpiSpcrData *f, const uint8_t rev,
> > - const char *oem_id, const char *oem_table_id)
> > + const char *oem_id, const char *oem_table_id, const char *name)
> > {
> > AcpiTable table = { .sig = "SPCR", .rev = rev, .oem_id = oem_id,
> > .oem_table_id = oem_table_id };
> > @@ -2042,8 +2042,16 @@ void build_spcr(GArray *table_data, BIOSLinker *linker,
> > build_append_int_noprefix(table_data, f->pci_flags, 4);
> > /* PCI Segment */
> > build_append_int_noprefix(table_data, f->pci_segment, 1);
> > - /* Reserved */
> > - build_append_int_noprefix(table_data, 0, 4);
> > + /* UartClkFreq */
> > + build_append_int_noprefix(table_data, f->uart_clk_freq, 4);
> > + /* PreciseBaudrate */
> > + build_append_int_noprefix(table_data, f->precise_baudrate, 4);
> > + /* NameSpaceStringLength */
> > + build_append_int_noprefix(table_data, f->namespace_string_length, 2);
> > + /* NameSpaceStringOffset */
> > + build_append_int_noprefix(table_data, f->namespace_string_offset, 2);
> > + /* NamespaceString[] */
> > + g_array_append_vals(table_data, name, f->namespace_string_length);
> >
> > acpi_table_end(linker, &table);
> > }
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index 6a1bde61ce..cb345e8659 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -428,11 +428,12 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> >
> > /*
> > * Serial Port Console Redirection Table (SPCR)
> > - * Rev: 1.07
> > + * Rev: 1.10
> > */
> > static void
> > spcr_setup(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> > {
> > + const char name[] = ".";
> > AcpiSpcrData serial = {
> > .interface_type = 3, /* ARM PL011 UART */
> > .base_addr.id = AML_AS_SYSTEM_MEMORY,
> > @@ -456,9 +457,14 @@ spcr_setup(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> > .pci_function = 0,
> > .pci_flags = 0,
> > .pci_segment = 0,
> > + .uart_clk_freq = 0,
> > + .precise_baudrate = 0,
> > + .namespace_string_length = sizeof(name),
> > + .namespace_string_offset = 88,
> > };
> >
> > - build_spcr(table_data, linker, &serial, 2, vms->oem_id, vms->oem_table_id);
> > + build_spcr(table_data, linker, &serial, 4, vms->oem_id, vms->oem_table_id,
> > + name);
> > }
> >
> > /*
> > diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
> > index 0925528160..5fa3942491 100644
> > --- a/hw/riscv/virt-acpi-build.c
> > +++ b/hw/riscv/virt-acpi-build.c
> > @@ -176,14 +176,15 @@ acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
> >
> > /*
> > * Serial Port Console Redirection Table (SPCR)
> > - * Rev: 1.07
> > + * Rev: 1.10
> > */
> >
> > static void
> > spcr_setup(GArray *table_data, BIOSLinker *linker, RISCVVirtState *s)
> > {
> > + const char name[] = ".";
> > AcpiSpcrData serial = {
> > - .interface_type = 0, /* 16550 compatible */
> > + .interface_type = 0x12, /* 16550 compatible */
> > .base_addr.id = AML_AS_SYSTEM_MEMORY,
> > .base_addr.width = 32,
> > .base_addr.offset = 0,
> > @@ -205,9 +206,14 @@ spcr_setup(GArray *table_data, BIOSLinker *linker, RISCVVirtState *s)
> > .pci_function = 0,
> > .pci_flags = 0,
> > .pci_segment = 0,
> > + .uart_clk_freq = 0,
> > + .precise_baudrate = 0,
> > + .namespace_string_length = sizeof(name),
> > + .namespace_string_offset = 88,
> > };
> >
> > - build_spcr(table_data, linker, &serial, 2, s->oem_id, s->oem_table_id);
> > + build_spcr(table_data, linker, &serial, 4, s->oem_id, s->oem_table_id,
> > + name);
> > }
> >
> > /* RHCT Node[N] starts at offset 56 */
> > diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> > index 0e6e82b339..2e6e341998 100644
> > --- a/include/hw/acpi/acpi-defs.h
> > +++ b/include/hw/acpi/acpi-defs.h
> > @@ -112,7 +112,6 @@ typedef struct AcpiSpcrData {
> > uint8_t flow_control;
> > uint8_t terminal_type;
> > uint8_t language;
> > - uint8_t reserved1;
> > uint16_t pci_device_id; /* Must be 0xffff if not PCI device */
> > uint16_t pci_vendor_id; /* Must be 0xffff if not PCI device */
> > uint8_t pci_bus;
> > @@ -120,7 +119,11 @@ typedef struct AcpiSpcrData {
> > uint8_t pci_function;
> > uint32_t pci_flags;
> > uint8_t pci_segment;
> > - uint32_t reserved2;
> > + uint32_t uart_clk_freq;
> > + uint32_t precise_baudrate;
> > + uint32_t namespace_string_length;
> > + uint32_t namespace_string_offset;
> > + char namespace_string[];
> > } AcpiSpcrData;
> >
> > #define ACPI_FADT_ARM_PSCI_COMPLIANT (1 << 0)
> > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> > index a3784155cb..68c0f2dbee 100644
> > --- a/include/hw/acpi/aml-build.h
> > +++ b/include/hw/acpi/aml-build.h
> > @@ -500,5 +500,5 @@ void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
> >
> > void build_spcr(GArray *table_data, BIOSLinker *linker,
> > const AcpiSpcrData *f, const uint8_t rev,
> > - const char *oem_id, const char *oem_table_id);
> > + const char *oem_id, const char *oem_table_id, const char *name);
> > #endif
> > --
> > 2.34.1
^ permalink raw reply [flat|nested] 13+ messages in thread