RE: [RESEND RFC v1 2/2] hw/riscv/virt-acpi-build.c: Generate SPCR table

[JeeHeng Sia]

> -----Original Message-----
> From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> Sent: Friday, January 5, 2024 8:27 PM
> To: JeeHeng Sia <jeeheng.sia@starfivetech.com>; qemu-arm@nongnu.org; 
> qemu-devel@nongnu.org; qemu-riscv@nongnu.org
> Cc: mst@redhat.com; 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;
> zhiwei_liu@linux.alibaba.com
> Subject: Re: [RESEND RFC v1 2/2] hw/riscv/virt-acpi-build.c: Generate SPCR 
> table
> 
> 
> 
> On 1/5/24 06:06, Sia Jee Heng wrote:
> > Generate Serial Port Console Redirection Table (SPCR) for RISC-V
> > virtual machine.
> >
> > Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
> > ---
> >   hw/riscv/virt-acpi-build.c | 39 ++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 39 insertions(+)
> >
> > diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
> > index d4a02579d6..388b3d1a84 100644
> > --- a/hw/riscv/virt-acpi-build.c
> > +++ b/hw/riscv/virt-acpi-build.c
> > @@ -174,6 +174,42 @@ acpi_dsdt_add_uart(Aml *scope, const MemMapEntry 
> > *uart_memmap,
> >       aml_append(scope, dev);
> >   }
> >
> > +/*
> > + * Serial Port Console Redirection Table (SPCR)
> > + * Rev: 1.07
> 
> Shouldn't it be "Rev: 2.0"? The function is calling the common build_spcr() 
> that
> specifies
I will give them a generic name for both the arch build_spcr() and the
common build_spcr(). The revision info should be passed to the common
build_spcr().
> 
> +    AcpiTable table = { .sig = "SPCR", .rev = 2, .oem_id = oem_id,
> +                        .oem_table_id = oem_table_id };
> 
> 
> 
> Code LGTM regardless of the "Rev: " comment value.
> 
> 
> Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> 
> 
> 
> 
> 
> > + */
> > +
> > +static void
> > +build_spcr_rev2(GArray *table_data, BIOSLinker *linker, RISCVVirtState *s)
> > +{
> > +    AcpiSpcrData serial = {
> > +        .interface_type = 0,       /* 16550 compatible */
> > +        .base_addr.id = AML_AS_SYSTEM_MEMORY,
> > +        .base_addr.width = 32,
> > +        .base_addr.offset = 0,
> > +        .base_addr.size = 1,
> > +        .base_addr.addr = s->memmap[VIRT_UART0].base,
> > +        .interrupt_type = (1 << 4),/* Bit[4] RISC-V PLIC/APLIC */
> > +        .pc_interrupt = 0,
> > +        .interrupt = UART0_IRQ,
> > +        .baud_rate = 7,            /* 15200 */
> > +        .parity = 0,
> > +        .stop_bits = 1,
> > +        .flow_control = 0,
> > +        .terminal_type = 3,        /* ANSI */
> > +        .language = 0,             /* Language */
> > +        .pci_device_id = 0xffff,   /* not a PCI device*/
> > +        .pci_vendor_id = 0xffff,   /* not a PCI device*/
> > +        .pci_bus = 0,
> > +        .pci_device = 0,
> > +        .pci_function = 0,
> > +        .pci_flags = 0,
> > +        .pci_segment = 0,
> > +    };
> > +
> > +    build_spcr(table_data, linker, &serial, s->oem_id, s->oem_table_id);
> > +}
> > +
> >   /* RHCT Node[N] starts at offset 56 */
> >   #define RHCT_NODE_ARRAY_OFFSET 56
> >
> > @@ -555,6 +591,9 @@ static void virt_acpi_build(RISCVVirtState *s, 
> > AcpiBuildTables *tables)
> >       acpi_add_table(table_offsets, tables_blob);
> >       build_rhct(tables_blob, tables->linker, s);
> >
> > +    acpi_add_table(table_offsets, tables_blob);
> > +    build_spcr_rev2(tables_blob, tables->linker, s);
> > +
> >       acpi_add_table(table_offsets, tables_blob);
> >       {
> >           AcpiMcfgInfo mcfg = {