Message ID | 20240823113142.161727-3-jeeheng.sia@starfivetech.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Upgrade ACPI SPCR table to support SPCR table revision 4 format | expand |
On Fri, Aug 23, 2024 at 04:31:41AM -0700, Sia Jee Heng wrote: > Update the SPCR table to accommodate the SPCR Table revision 4 [1]. > The SPCR table has been modified to adhere to the revision 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> > --- > hw/acpi/aml-build.c | 20 ++++++++++++++++---- > 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, 38 insertions(+), 13 deletions(-) > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > index 6d4517cfbe..ad0a306db1 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,9 +2042,21 @@ 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); > - > + if (rev < 4) { > + /* Reserved */ > + build_append_int_noprefix(table_data, 0, 4); > + } else { > + /* 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 f76fb117ad..cc27ba7daf 100644 > --- a/hw/arm/virt-acpi-build.c > +++ b/hw/arm/virt-acpi-build.c > @@ -435,7 +435,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > > /* > * Serial Port Console Redirection Table (SPCR) > - * Rev: 1.07 > + * Rev: 1.10 Why should this comment be updated? Since revision 2 of SPCR table ARM uses corresponds to only 1.07 right? > */ > static void > spcr_setup(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > @@ -464,8 +464,12 @@ spcr_setup(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > .pci_flags = 0, > .pci_segment = 0, > }; > - > - build_spcr(table_data, linker, &serial, 2, vms->oem_id, vms->oem_table_id); > + /* > + * Passing NULL as the SPCR Table for Revision 2 doesn't support > + * NameSpaceString. > + */ > + build_spcr(table_data, linker, &serial, 2, vms->oem_id, vms->oem_table_id, > + NULL); > } > > /* > diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c > index 36d6a3a412..68ef15acac 100644 > --- a/hw/riscv/virt-acpi-build.c > +++ b/hw/riscv/virt-acpi-build.c > @@ -200,14 +200,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, > @@ -229,9 +230,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 Otherwise, LGTM. Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>
> -----Original Message----- > From: Sunil V L <sunilvl@ventanamicro.com> > Sent: Friday, August 23, 2024 9:15 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 RESEND v4 2/3] hw/acpi: Upgrade ACPI SPCR table to support SPCR table revision 4 format > > On Fri, Aug 23, 2024 at 04:31:41AM -0700, Sia Jee Heng wrote: > > Update the SPCR table to accommodate the SPCR Table revision 4 [1]. > > The SPCR table has been modified to adhere to the revision 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> > > --- > > hw/acpi/aml-build.c | 20 ++++++++++++++++---- > > 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, 38 insertions(+), 13 deletions(-) > > > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > > index 6d4517cfbe..ad0a306db1 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,9 +2042,21 @@ 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); > > - > > + if (rev < 4) { > > + /* Reserved */ > > + build_append_int_noprefix(table_data, 0, 4); > > + } else { > > + /* 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 f76fb117ad..cc27ba7daf 100644 > > --- a/hw/arm/virt-acpi-build.c > > +++ b/hw/arm/virt-acpi-build.c > > @@ -435,7 +435,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > > > > /* > > * Serial Port Console Redirection Table (SPCR) > > - * Rev: 1.07 > > + * Rev: 1.10 > Why should this comment be updated? Since revision 2 of SPCR table ARM > uses corresponds to only 1.07 right? You are right. I will revert this. > > > */ > > static void > > spcr_setup(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > > @@ -464,8 +464,12 @@ spcr_setup(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > > .pci_flags = 0, > > .pci_segment = 0, > > }; > > - > > - build_spcr(table_data, linker, &serial, 2, vms->oem_id, vms->oem_table_id); > > + /* > > + * Passing NULL as the SPCR Table for Revision 2 doesn't support > > + * NameSpaceString. > > + */ > > + build_spcr(table_data, linker, &serial, 2, vms->oem_id, vms->oem_table_id, > > + NULL); > > } > > > > /* > > diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c > > index 36d6a3a412..68ef15acac 100644 > > --- a/hw/riscv/virt-acpi-build.c > > +++ b/hw/riscv/virt-acpi-build.c > > @@ -200,14 +200,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, > > @@ -229,9 +230,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 > > Otherwise, LGTM. > > Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c index 6d4517cfbe..ad0a306db1 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,9 +2042,21 @@ 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); - + if (rev < 4) { + /* Reserved */ + build_append_int_noprefix(table_data, 0, 4); + } else { + /* 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 f76fb117ad..cc27ba7daf 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -435,7 +435,7 @@ 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) @@ -464,8 +464,12 @@ spcr_setup(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) .pci_flags = 0, .pci_segment = 0, }; - - build_spcr(table_data, linker, &serial, 2, vms->oem_id, vms->oem_table_id); + /* + * Passing NULL as the SPCR Table for Revision 2 doesn't support + * NameSpaceString. + */ + build_spcr(table_data, linker, &serial, 2, vms->oem_id, vms->oem_table_id, + NULL); } /* diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c index 36d6a3a412..68ef15acac 100644 --- a/hw/riscv/virt-acpi-build.c +++ b/hw/riscv/virt-acpi-build.c @@ -200,14 +200,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, @@ -229,9 +230,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