* [RESEND RFC v1 0/2] RISC-V: ACPI: Enable SPCR @ 2024-01-05 9:06 Sia Jee Heng 2024-01-05 9:06 ` [RESEND RFC v1 1/2] hw/arm/virt-acpi-build.c: Migrate SPCR creation to common location Sia Jee Heng 2024-01-05 9:06 ` [RESEND RFC v1 2/2] hw/riscv/virt-acpi-build.c: Generate SPCR table Sia Jee Heng 0 siblings, 2 replies; 10+ messages in thread From: Sia Jee Heng @ 2024-01-05 9:06 UTC (permalink / raw) To: qemu-arm, qemu-devel, qemu-riscv Cc: mst, imammedo, anisinha, peter.maydell, jeeheng.sia, shannon.zhaosl, sunilvl, palmer, alistair.francis, bin.meng, liwei1518, dbarboza, zhiwei_liu This series focuses on enabling the Serial Port Console Redirection (SPCR) table for the RISC-V virt platform. Considering that ARM utilizes the same function, the initial patch involves migrating the build_spcr function to common code. This consolidation ensures that RISC-V avoids duplicating the function. The patch set is built upon Alistair's riscv-to-apply.next branch and relies on Sunil's patches at [1]. [1] https://lore.kernel.org/qemu-devel/20231218150247.466427-1-sunilvl@ventanamicro.com/ Sia Jee Heng (2): hw/arm/virt-acpi-build.c: Migrate SPCR creation to common location hw/riscv/virt-acpi-build.c: Generate SPCR table hw/acpi/aml-build.c | 51 ++++++++++++++++++++++++++++ hw/arm/virt-acpi-build.c | 68 +++++++++++++++---------------------- hw/riscv/virt-acpi-build.c | 39 +++++++++++++++++++++ include/hw/acpi/acpi-defs.h | 33 ++++++++++++++++++ include/hw/acpi/aml-build.h | 4 +++ 5 files changed, 154 insertions(+), 41 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [RESEND RFC v1 1/2] hw/arm/virt-acpi-build.c: Migrate SPCR creation to common location 2024-01-05 9:06 [RESEND RFC v1 0/2] RISC-V: ACPI: Enable SPCR Sia Jee Heng @ 2024-01-05 9:06 ` Sia Jee Heng 2024-01-05 12:19 ` Daniel Henrique Barboza 2024-01-05 9:06 ` [RESEND RFC v1 2/2] hw/riscv/virt-acpi-build.c: Generate SPCR table Sia Jee Heng 1 sibling, 1 reply; 10+ messages in thread From: Sia Jee Heng @ 2024-01-05 9:06 UTC (permalink / raw) To: qemu-arm, qemu-devel, qemu-riscv Cc: mst, imammedo, anisinha, peter.maydell, jeeheng.sia, shannon.zhaosl, sunilvl, palmer, alistair.francis, bin.meng, liwei1518, dbarboza, zhiwei_liu RISC-V should also generate the SPCR in a manner similar to ARM. Therefore, instead of replicating the code, relocate this function to the common AML build. Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com> --- hw/acpi/aml-build.c | 51 ++++++++++++++++++++++++++++ hw/arm/virt-acpi-build.c | 68 +++++++++++++++---------------------- include/hw/acpi/acpi-defs.h | 33 ++++++++++++++++++ include/hw/acpi/aml-build.h | 4 +++ 4 files changed, 115 insertions(+), 41 deletions(-) diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c index af66bde0f5..1efa534aa8 100644 --- a/hw/acpi/aml-build.c +++ b/hw/acpi/aml-build.c @@ -1994,6 +1994,57 @@ static void build_processor_hierarchy_node(GArray *tbl, uint32_t flags, } } +void build_spcr(GArray *table_data, BIOSLinker *linker, + const AcpiSpcrData *f, const char *oem_id, + const char *oem_table_id) +{ + AcpiTable table = { .sig = "SPCR", .rev = 2, .oem_id = oem_id, + .oem_table_id = oem_table_id }; + + acpi_table_begin(&table, table_data); + /* Interface type */ + build_append_int_noprefix(table_data, f->interface_type, 1); + /* Reserved */ + build_append_int_noprefix(table_data, 0, 3); + /* Base Address */ + build_append_gas(table_data, f->base_addr.id, f->base_addr.width, + f->base_addr.offset, f->base_addr.size, + f->base_addr.addr); + /* Interrupt type */ + build_append_int_noprefix(table_data, f->interrupt_type, 1); + /* IRQ */ + build_append_int_noprefix(table_data, f->pc_interrupt, 1); + /* Global System Interrupt */ + build_append_int_noprefix(table_data, f->interrupt, 4); + /* Baud Rate */ + build_append_int_noprefix(table_data, f->baud_rate, 1); + /* Parity */ + build_append_int_noprefix(table_data, f->parity, 1); + /* Stop Bits */ + build_append_int_noprefix(table_data, f->stop_bits, 1); + /* Flow Control */ + build_append_int_noprefix(table_data, f->flow_control, 1); + /* Terminal Type */ + build_append_int_noprefix(table_data, f->terminal_type, 1); + /* PCI Device ID */ + build_append_int_noprefix(table_data, f->pci_device_id, 2); + /* PCI Vendor ID */ + build_append_int_noprefix(table_data, f->pci_vendor_id, 2); + /* PCI Bus Number */ + build_append_int_noprefix(table_data, f->pci_bus, 1); + /* PCI Device Number */ + build_append_int_noprefix(table_data, f->pci_device, 1); + /* PCI Function Number */ + build_append_int_noprefix(table_data, f->pci_function, 1); + /* PCI Flags */ + 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); + + acpi_table_end(linker, &table); +} /* * ACPI spec, Revision 6.3 * 5.2.29 Processor Properties Topology Table (PPTT) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 510ab0dcca..a31f736d1a 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -431,48 +431,34 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) * Rev: 1.07 */ static void -build_spcr(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) +build_spcr_v2(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) { - AcpiTable table = { .sig = "SPCR", .rev = 2, .oem_id = vms->oem_id, - .oem_table_id = vms->oem_table_id }; - - acpi_table_begin(&table, table_data); - - /* Interface Type */ - build_append_int_noprefix(table_data, 3, 1); /* ARM PL011 UART */ - build_append_int_noprefix(table_data, 0, 3); /* Reserved */ - /* Base Address */ - build_append_gas(table_data, AML_AS_SYSTEM_MEMORY, 32, 0, 3, - vms->memmap[VIRT_UART].base); - /* Interrupt Type */ - build_append_int_noprefix(table_data, - (1 << 3) /* Bit[3] ARMH GIC interrupt */, 1); - build_append_int_noprefix(table_data, 0, 1); /* IRQ */ - /* Global System Interrupt */ - build_append_int_noprefix(table_data, - vms->irqmap[VIRT_UART] + ARM_SPI_BASE, 4); - build_append_int_noprefix(table_data, 3 /* 9600 */, 1); /* Baud Rate */ - build_append_int_noprefix(table_data, 0 /* No Parity */, 1); /* Parity */ - /* Stop Bits */ - build_append_int_noprefix(table_data, 1 /* 1 Stop bit */, 1); - /* Flow Control */ - build_append_int_noprefix(table_data, - (1 << 1) /* RTS/CTS hardware flow control */, 1); - /* Terminal Type */ - build_append_int_noprefix(table_data, 0 /* VT100 */, 1); - build_append_int_noprefix(table_data, 0, 1); /* Language */ - /* PCI Device ID */ - build_append_int_noprefix(table_data, 0xffff /* not a PCI device*/, 2); - /* PCI Vendor ID */ - build_append_int_noprefix(table_data, 0xffff /* not a PCI device*/, 2); - build_append_int_noprefix(table_data, 0, 1); /* PCI Bus Number */ - build_append_int_noprefix(table_data, 0, 1); /* PCI Device Number */ - build_append_int_noprefix(table_data, 0, 1); /* PCI Function Number */ - build_append_int_noprefix(table_data, 0, 4); /* PCI Flags */ - build_append_int_noprefix(table_data, 0, 1); /* PCI Segment */ - build_append_int_noprefix(table_data, 0, 4); /* Reserved */ + AcpiSpcrData serial = { + .interface_type = 3, /* ARM PL011 UART */ + .base_addr.id = AML_AS_SYSTEM_MEMORY, + .base_addr.width = 32, + .base_addr.offset = 0, + .base_addr.size = 3, + .base_addr.addr = vms->memmap[VIRT_UART].base, + .interrupt_type = (1 << 3),/* Bit[3] ARMH GIC interrupt*/ + .pc_interrupt = 0, /* IRQ */ + .interrupt = (vms->irqmap[VIRT_UART] + ARM_SPI_BASE), + .baud_rate = 3, /* 9600 */ + .parity = 0, /* No Parity */ + .stop_bits = 1, /* 1 Stop bit */ + .flow_control = 1 << 1, /* RTS/CTS hardware flow control */ + .terminal_type = 0, /* VT100 */ + .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, + }; - acpi_table_end(linker, &table); + build_spcr(table_data, linker, &serial, vms->oem_id, vms->oem_table_id); } /* @@ -930,7 +916,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables) } acpi_add_table(table_offsets, tables_blob); - build_spcr(tables_blob, tables->linker, vms); + build_spcr_v2(tables_blob, tables->linker, vms); acpi_add_table(table_offsets, tables_blob); build_dbg2(tables_blob, tables->linker, vms); diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h index 2b42e4192b..0e6e82b339 100644 --- a/include/hw/acpi/acpi-defs.h +++ b/include/hw/acpi/acpi-defs.h @@ -90,6 +90,39 @@ typedef struct AcpiFadtData { unsigned *xdsdt_tbl_offset; } AcpiFadtData; +typedef struct AcpiGas { + uint8_t id; /* Address space ID */ + uint8_t width; /* Register bit width */ + uint8_t offset; /* Register bit offset */ + uint8_t size; /* Access size */ + uint64_t addr; /* Address */ +} AcpiGas; + +/* SPCR (Serial Port Console Redirection table) */ +typedef struct AcpiSpcrData { + uint8_t interface_type; + uint8_t reserved[3]; + struct AcpiGas base_addr; + uint8_t interrupt_type; + uint8_t pc_interrupt; + uint32_t interrupt; /* Global system interrupt */ + uint8_t baud_rate; + uint8_t parity; + uint8_t stop_bits; + 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; + uint8_t pci_device; + uint8_t pci_function; + uint32_t pci_flags; + uint8_t pci_segment; + uint32_t reserved2; +} AcpiSpcrData; + #define ACPI_FADT_ARM_PSCI_COMPLIANT (1 << 0) #define ACPI_FADT_ARM_PSCI_USE_HVC (1 << 1) diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h index ff2a310270..44e03e76bd 100644 --- a/include/hw/acpi/aml-build.h +++ b/include/hw/acpi/aml-build.h @@ -497,4 +497,8 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f, void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog, const char *oem_id, const char *oem_table_id); + +void build_spcr(GArray *table_data, BIOSLinker *linker, + const AcpiSpcrData *f, const char *oem_id, + const char *oem_table_id); #endif -- 2.34.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [RESEND RFC v1 1/2] hw/arm/virt-acpi-build.c: Migrate SPCR creation to common location 2024-01-05 9:06 ` [RESEND RFC v1 1/2] hw/arm/virt-acpi-build.c: Migrate SPCR creation to common location Sia Jee Heng @ 2024-01-05 12:19 ` Daniel Henrique Barboza 2024-01-08 11:34 ` Sunil V L 2024-01-10 8:02 ` JeeHeng Sia 0 siblings, 2 replies; 10+ messages in thread From: Daniel Henrique Barboza @ 2024-01-05 12:19 UTC (permalink / raw) To: Sia Jee Heng, qemu-arm, qemu-devel, qemu-riscv Cc: mst, imammedo, anisinha, peter.maydell, shannon.zhaosl, sunilvl, palmer, alistair.francis, bin.meng, liwei1518, zhiwei_liu On 1/5/24 06:06, Sia Jee Heng wrote: > RISC-V should also generate the SPCR in a manner similar to ARM. > Therefore, instead of replicating the code, relocate this function > to the common AML build. > > Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com> > --- > hw/acpi/aml-build.c | 51 ++++++++++++++++++++++++++++ > hw/arm/virt-acpi-build.c | 68 +++++++++++++++---------------------- > include/hw/acpi/acpi-defs.h | 33 ++++++++++++++++++ > include/hw/acpi/aml-build.h | 4 +++ > 4 files changed, 115 insertions(+), 41 deletions(-) > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > index af66bde0f5..1efa534aa8 100644 > --- a/hw/acpi/aml-build.c > +++ b/hw/acpi/aml-build.c > @@ -1994,6 +1994,57 @@ static void build_processor_hierarchy_node(GArray *tbl, uint32_t flags, > } > } > > +void build_spcr(GArray *table_data, BIOSLinker *linker, > + const AcpiSpcrData *f, const char *oem_id, > + const char *oem_table_id) > +{ > + AcpiTable table = { .sig = "SPCR", .rev = 2, .oem_id = oem_id, > + .oem_table_id = oem_table_id }; > + > + acpi_table_begin(&table, table_data); > + /* Interface type */ > + build_append_int_noprefix(table_data, f->interface_type, 1); > + /* Reserved */ > + build_append_int_noprefix(table_data, 0, 3); > + /* Base Address */ > + build_append_gas(table_data, f->base_addr.id, f->base_addr.width, > + f->base_addr.offset, f->base_addr.size, > + f->base_addr.addr); > + /* Interrupt type */ > + build_append_int_noprefix(table_data, f->interrupt_type, 1); > + /* IRQ */ > + build_append_int_noprefix(table_data, f->pc_interrupt, 1); > + /* Global System Interrupt */ > + build_append_int_noprefix(table_data, f->interrupt, 4); > + /* Baud Rate */ > + build_append_int_noprefix(table_data, f->baud_rate, 1); > + /* Parity */ > + build_append_int_noprefix(table_data, f->parity, 1); > + /* Stop Bits */ > + build_append_int_noprefix(table_data, f->stop_bits, 1); > + /* Flow Control */ > + build_append_int_noprefix(table_data, f->flow_control, 1); > + /* Terminal Type */ > + build_append_int_noprefix(table_data, f->terminal_type, 1); > + /* PCI Device ID */ > + build_append_int_noprefix(table_data, f->pci_device_id, 2); > + /* PCI Vendor ID */ > + build_append_int_noprefix(table_data, f->pci_vendor_id, 2); > + /* PCI Bus Number */ > + build_append_int_noprefix(table_data, f->pci_bus, 1); > + /* PCI Device Number */ > + build_append_int_noprefix(table_data, f->pci_device, 1); > + /* PCI Function Number */ > + build_append_int_noprefix(table_data, f->pci_function, 1); > + /* PCI Flags */ > + 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); > + > + acpi_table_end(linker, &table); > +} > /* > * ACPI spec, Revision 6.3 > * 5.2.29 Processor Properties Topology Table (PPTT) > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > index 510ab0dcca..a31f736d1a 100644 > --- a/hw/arm/virt-acpi-build.c > +++ b/hw/arm/virt-acpi-build.c > @@ -431,48 +431,34 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > * Rev: 1.07 > */ > static void > -build_spcr(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > +build_spcr_v2(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) Nit: I don't understand the '_v2' in the name of this function. Is it just to not collide with the now public build_spcr()? Or does it have to do with the SPCR table being '.rev = 2'? Because if it's the latter, you can name the common helper 'build_spcr_rev2' (since both ARM and RISC-V use SPCR rev 2), keep this local build_spcr() initializing the AcpiSpcrData struct with ARM attributes and then call the common build_spcr_rev2(). Code looks good othewise. Thanks, Daniel > { > - AcpiTable table = { .sig = "SPCR", .rev = 2, .oem_id = vms->oem_id, > - .oem_table_id = vms->oem_table_id }; > - > - acpi_table_begin(&table, table_data); > - > - /* Interface Type */ > - build_append_int_noprefix(table_data, 3, 1); /* ARM PL011 UART */ > - build_append_int_noprefix(table_data, 0, 3); /* Reserved */ > - /* Base Address */ > - build_append_gas(table_data, AML_AS_SYSTEM_MEMORY, 32, 0, 3, > - vms->memmap[VIRT_UART].base); > - /* Interrupt Type */ > - build_append_int_noprefix(table_data, > - (1 << 3) /* Bit[3] ARMH GIC interrupt */, 1); > - build_append_int_noprefix(table_data, 0, 1); /* IRQ */ > - /* Global System Interrupt */ > - build_append_int_noprefix(table_data, > - vms->irqmap[VIRT_UART] + ARM_SPI_BASE, 4); > - build_append_int_noprefix(table_data, 3 /* 9600 */, 1); /* Baud Rate */ > - build_append_int_noprefix(table_data, 0 /* No Parity */, 1); /* Parity */ > - /* Stop Bits */ > - build_append_int_noprefix(table_data, 1 /* 1 Stop bit */, 1); > - /* Flow Control */ > - build_append_int_noprefix(table_data, > - (1 << 1) /* RTS/CTS hardware flow control */, 1); > - /* Terminal Type */ > - build_append_int_noprefix(table_data, 0 /* VT100 */, 1); > - build_append_int_noprefix(table_data, 0, 1); /* Language */ > - /* PCI Device ID */ > - build_append_int_noprefix(table_data, 0xffff /* not a PCI device*/, 2); > - /* PCI Vendor ID */ > - build_append_int_noprefix(table_data, 0xffff /* not a PCI device*/, 2); > - build_append_int_noprefix(table_data, 0, 1); /* PCI Bus Number */ > - build_append_int_noprefix(table_data, 0, 1); /* PCI Device Number */ > - build_append_int_noprefix(table_data, 0, 1); /* PCI Function Number */ > - build_append_int_noprefix(table_data, 0, 4); /* PCI Flags */ > - build_append_int_noprefix(table_data, 0, 1); /* PCI Segment */ > - build_append_int_noprefix(table_data, 0, 4); /* Reserved */ > + AcpiSpcrData serial = { > + .interface_type = 3, /* ARM PL011 UART */ > + .base_addr.id = AML_AS_SYSTEM_MEMORY, > + .base_addr.width = 32, > + .base_addr.offset = 0, > + .base_addr.size = 3, > + .base_addr.addr = vms->memmap[VIRT_UART].base, > + .interrupt_type = (1 << 3),/* Bit[3] ARMH GIC interrupt*/ > + .pc_interrupt = 0, /* IRQ */ > + .interrupt = (vms->irqmap[VIRT_UART] + ARM_SPI_BASE), > + .baud_rate = 3, /* 9600 */ > + .parity = 0, /* No Parity */ > + .stop_bits = 1, /* 1 Stop bit */ > + .flow_control = 1 << 1, /* RTS/CTS hardware flow control */ > + .terminal_type = 0, /* VT100 */ > + .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, > + }; > > - acpi_table_end(linker, &table); > + build_spcr(table_data, linker, &serial, vms->oem_id, vms->oem_table_id); > } > > /* > @@ -930,7 +916,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables) > } > > acpi_add_table(table_offsets, tables_blob); > - build_spcr(tables_blob, tables->linker, vms); > + build_spcr_v2(tables_blob, tables->linker, vms); > > acpi_add_table(table_offsets, tables_blob); > build_dbg2(tables_blob, tables->linker, vms); > diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h > index 2b42e4192b..0e6e82b339 100644 > --- a/include/hw/acpi/acpi-defs.h > +++ b/include/hw/acpi/acpi-defs.h > @@ -90,6 +90,39 @@ typedef struct AcpiFadtData { > unsigned *xdsdt_tbl_offset; > } AcpiFadtData; > > +typedef struct AcpiGas { > + uint8_t id; /* Address space ID */ > + uint8_t width; /* Register bit width */ > + uint8_t offset; /* Register bit offset */ > + uint8_t size; /* Access size */ > + uint64_t addr; /* Address */ > +} AcpiGas; > + > +/* SPCR (Serial Port Console Redirection table) */ > +typedef struct AcpiSpcrData { > + uint8_t interface_type; > + uint8_t reserved[3]; > + struct AcpiGas base_addr; > + uint8_t interrupt_type; > + uint8_t pc_interrupt; > + uint32_t interrupt; /* Global system interrupt */ > + uint8_t baud_rate; > + uint8_t parity; > + uint8_t stop_bits; > + 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; > + uint8_t pci_device; > + uint8_t pci_function; > + uint32_t pci_flags; > + uint8_t pci_segment; > + uint32_t reserved2; > +} AcpiSpcrData; > + > #define ACPI_FADT_ARM_PSCI_COMPLIANT (1 << 0) > #define ACPI_FADT_ARM_PSCI_USE_HVC (1 << 1) > > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h > index ff2a310270..44e03e76bd 100644 > --- a/include/hw/acpi/aml-build.h > +++ b/include/hw/acpi/aml-build.h > @@ -497,4 +497,8 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f, > > void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog, > const char *oem_id, const char *oem_table_id); > + > +void build_spcr(GArray *table_data, BIOSLinker *linker, > + const AcpiSpcrData *f, const char *oem_id, > + const char *oem_table_id); > #endif ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RESEND RFC v1 1/2] hw/arm/virt-acpi-build.c: Migrate SPCR creation to common location 2024-01-05 12:19 ` Daniel Henrique Barboza @ 2024-01-08 11:34 ` Sunil V L 2024-01-10 8:03 ` JeeHeng Sia 2024-01-10 8:02 ` JeeHeng Sia 1 sibling, 1 reply; 10+ messages in thread From: Sunil V L @ 2024-01-08 11:34 UTC (permalink / raw) To: Daniel Henrique Barboza Cc: Sia Jee Heng, qemu-arm, qemu-devel, qemu-riscv, mst, imammedo, anisinha, peter.maydell, shannon.zhaosl, palmer, alistair.francis, bin.meng, liwei1518, zhiwei_liu On Fri, Jan 05, 2024 at 09:19:14AM -0300, Daniel Henrique Barboza wrote: > > > On 1/5/24 06:06, Sia Jee Heng wrote: > > RISC-V should also generate the SPCR in a manner similar to ARM. > > Therefore, instead of replicating the code, relocate this function > > to the common AML build. > > > > Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com> > > --- > > hw/acpi/aml-build.c | 51 ++++++++++++++++++++++++++++ > > hw/arm/virt-acpi-build.c | 68 +++++++++++++++---------------------- > > include/hw/acpi/acpi-defs.h | 33 ++++++++++++++++++ > > include/hw/acpi/aml-build.h | 4 +++ > > 4 files changed, 115 insertions(+), 41 deletions(-) > > > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > > index af66bde0f5..1efa534aa8 100644 > > --- a/hw/acpi/aml-build.c > > +++ b/hw/acpi/aml-build.c > > @@ -1994,6 +1994,57 @@ static void build_processor_hierarchy_node(GArray *tbl, uint32_t flags, > > } > > } > > +void build_spcr(GArray *table_data, BIOSLinker *linker, > > + const AcpiSpcrData *f, const char *oem_id, > > + const char *oem_table_id) > > +{ > > + AcpiTable table = { .sig = "SPCR", .rev = 2, .oem_id = oem_id, > > + .oem_table_id = oem_table_id }; > > + > > + acpi_table_begin(&table, table_data); > > + /* Interface type */ > > + build_append_int_noprefix(table_data, f->interface_type, 1); > > + /* Reserved */ > > + build_append_int_noprefix(table_data, 0, 3); > > + /* Base Address */ > > + build_append_gas(table_data, f->base_addr.id, f->base_addr.width, > > + f->base_addr.offset, f->base_addr.size, > > + f->base_addr.addr); > > + /* Interrupt type */ > > + build_append_int_noprefix(table_data, f->interrupt_type, 1); > > + /* IRQ */ > > + build_append_int_noprefix(table_data, f->pc_interrupt, 1); > > + /* Global System Interrupt */ > > + build_append_int_noprefix(table_data, f->interrupt, 4); > > + /* Baud Rate */ > > + build_append_int_noprefix(table_data, f->baud_rate, 1); > > + /* Parity */ > > + build_append_int_noprefix(table_data, f->parity, 1); > > + /* Stop Bits */ > > + build_append_int_noprefix(table_data, f->stop_bits, 1); > > + /* Flow Control */ > > + build_append_int_noprefix(table_data, f->flow_control, 1); > > + /* Terminal Type */ > > + build_append_int_noprefix(table_data, f->terminal_type, 1); > > + /* PCI Device ID */ > > + build_append_int_noprefix(table_data, f->pci_device_id, 2); > > + /* PCI Vendor ID */ > > + build_append_int_noprefix(table_data, f->pci_vendor_id, 2); > > + /* PCI Bus Number */ > > + build_append_int_noprefix(table_data, f->pci_bus, 1); > > + /* PCI Device Number */ > > + build_append_int_noprefix(table_data, f->pci_device, 1); > > + /* PCI Function Number */ > > + build_append_int_noprefix(table_data, f->pci_function, 1); > > + /* PCI Flags */ > > + 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); > > + > > + acpi_table_end(linker, &table); > > +} > > /* > > * ACPI spec, Revision 6.3 > > * 5.2.29 Processor Properties Topology Table (PPTT) > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > > index 510ab0dcca..a31f736d1a 100644 > > --- a/hw/arm/virt-acpi-build.c > > +++ b/hw/arm/virt-acpi-build.c > > @@ -431,48 +431,34 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > > * Rev: 1.07 > > */ > > static void > > -build_spcr(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > > +build_spcr_v2(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > > Nit: I don't understand the '_v2' in the name of this function. Is it just to not collide > with the now public build_spcr()? Or does it have to do with the SPCR table being > '.rev = 2'? Because if it's the latter, you can name the common helper 'build_spcr_rev2' > (since both ARM and RISC-V use SPCR rev 2), keep this local build_spcr() initializing > the AcpiSpcrData struct with ARM attributes and then call the common build_spcr_rev2(). > My suggestion is, keep the build_spcr() generic and take version as the parameter. Thanks, Sunil ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [RESEND RFC v1 1/2] hw/arm/virt-acpi-build.c: Migrate SPCR creation to common location 2024-01-08 11:34 ` Sunil V L @ 2024-01-10 8:03 ` JeeHeng Sia 0 siblings, 0 replies; 10+ messages in thread From: JeeHeng Sia @ 2024-01-10 8:03 UTC (permalink / raw) To: Sunil V L, Daniel Henrique Barboza 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, zhiwei_liu@linux.alibaba.com > -----Original Message----- > From: Sunil V L <sunilvl@ventanamicro.com> > Sent: Monday, January 8, 2024 7:35 PM > To: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > Cc: JeeHeng Sia <jeeheng.sia@starfivetech.com>; 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; zhiwei_liu@linux.alibaba.com > Subject: Re: [RESEND RFC v1 1/2] hw/arm/virt-acpi-build.c: Migrate SPCR creation to common location > > On Fri, Jan 05, 2024 at 09:19:14AM -0300, Daniel Henrique Barboza wrote: > > > > > > On 1/5/24 06:06, Sia Jee Heng wrote: > > > RISC-V should also generate the SPCR in a manner similar to ARM. > > > Therefore, instead of replicating the code, relocate this function > > > to the common AML build. > > > > > > Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com> > > > --- > > > hw/acpi/aml-build.c | 51 ++++++++++++++++++++++++++++ > > > hw/arm/virt-acpi-build.c | 68 +++++++++++++++---------------------- > > > include/hw/acpi/acpi-defs.h | 33 ++++++++++++++++++ > > > include/hw/acpi/aml-build.h | 4 +++ > > > 4 files changed, 115 insertions(+), 41 deletions(-) > > > > > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > > > index af66bde0f5..1efa534aa8 100644 > > > --- a/hw/acpi/aml-build.c > > > +++ b/hw/acpi/aml-build.c > > > @@ -1994,6 +1994,57 @@ static void build_processor_hierarchy_node(GArray *tbl, uint32_t flags, > > > } > > > } > > > +void build_spcr(GArray *table_data, BIOSLinker *linker, > > > + const AcpiSpcrData *f, const char *oem_id, > > > + const char *oem_table_id) > > > +{ > > > + AcpiTable table = { .sig = "SPCR", .rev = 2, .oem_id = oem_id, > > > + .oem_table_id = oem_table_id }; > > > + > > > + acpi_table_begin(&table, table_data); > > > + /* Interface type */ > > > + build_append_int_noprefix(table_data, f->interface_type, 1); > > > + /* Reserved */ > > > + build_append_int_noprefix(table_data, 0, 3); > > > + /* Base Address */ > > > + build_append_gas(table_data, f->base_addr.id, f->base_addr.width, > > > + f->base_addr.offset, f->base_addr.size, > > > + f->base_addr.addr); > > > + /* Interrupt type */ > > > + build_append_int_noprefix(table_data, f->interrupt_type, 1); > > > + /* IRQ */ > > > + build_append_int_noprefix(table_data, f->pc_interrupt, 1); > > > + /* Global System Interrupt */ > > > + build_append_int_noprefix(table_data, f->interrupt, 4); > > > + /* Baud Rate */ > > > + build_append_int_noprefix(table_data, f->baud_rate, 1); > > > + /* Parity */ > > > + build_append_int_noprefix(table_data, f->parity, 1); > > > + /* Stop Bits */ > > > + build_append_int_noprefix(table_data, f->stop_bits, 1); > > > + /* Flow Control */ > > > + build_append_int_noprefix(table_data, f->flow_control, 1); > > > + /* Terminal Type */ > > > + build_append_int_noprefix(table_data, f->terminal_type, 1); > > > + /* PCI Device ID */ > > > + build_append_int_noprefix(table_data, f->pci_device_id, 2); > > > + /* PCI Vendor ID */ > > > + build_append_int_noprefix(table_data, f->pci_vendor_id, 2); > > > + /* PCI Bus Number */ > > > + build_append_int_noprefix(table_data, f->pci_bus, 1); > > > + /* PCI Device Number */ > > > + build_append_int_noprefix(table_data, f->pci_device, 1); > > > + /* PCI Function Number */ > > > + build_append_int_noprefix(table_data, f->pci_function, 1); > > > + /* PCI Flags */ > > > + 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); > > > + > > > + acpi_table_end(linker, &table); > > > +} > > > /* > > > * ACPI spec, Revision 6.3 > > > * 5.2.29 Processor Properties Topology Table (PPTT) > > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > > > index 510ab0dcca..a31f736d1a 100644 > > > --- a/hw/arm/virt-acpi-build.c > > > +++ b/hw/arm/virt-acpi-build.c > > > @@ -431,48 +431,34 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > > > * Rev: 1.07 > > > */ > > > static void > > > -build_spcr(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > > > +build_spcr_v2(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > > > > Nit: I don't understand the '_v2' in the name of this function. Is it just to not collide > > with the now public build_spcr()? Or does it have to do with the SPCR table being > > '.rev = 2'? Because if it's the latter, you can name the common helper 'build_spcr_rev2' > > (since both ARM and RISC-V use SPCR rev 2), keep this local build_spcr() initializing > > the AcpiSpcrData struct with ARM attributes and then call the common build_spcr_rev2(). > > > My suggestion is, keep the build_spcr() generic and take version as the > parameter. Alright. 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(). > > Thanks, > Sunil ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [RESEND RFC v1 1/2] hw/arm/virt-acpi-build.c: Migrate SPCR creation to common location 2024-01-05 12:19 ` Daniel Henrique Barboza 2024-01-08 11:34 ` Sunil V L @ 2024-01-10 8:02 ` JeeHeng Sia 2024-01-10 9:23 ` JeeHeng Sia 1 sibling, 1 reply; 10+ messages in thread From: JeeHeng Sia @ 2024-01-10 8:02 UTC (permalink / raw) To: Daniel Henrique Barboza, 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 > -----Original Message----- > From: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > Sent: Friday, January 5, 2024 8:19 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 1/2] hw/arm/virt-acpi-build.c: Migrate SPCR creation to common location > > > > On 1/5/24 06:06, Sia Jee Heng wrote: > > RISC-V should also generate the SPCR in a manner similar to ARM. > > Therefore, instead of replicating the code, relocate this function > > to the common AML build. > > > > Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com> > > --- > > hw/acpi/aml-build.c | 51 ++++++++++++++++++++++++++++ > > hw/arm/virt-acpi-build.c | 68 +++++++++++++++---------------------- > > include/hw/acpi/acpi-defs.h | 33 ++++++++++++++++++ > > include/hw/acpi/aml-build.h | 4 +++ > > 4 files changed, 115 insertions(+), 41 deletions(-) > > > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > > index af66bde0f5..1efa534aa8 100644 > > --- a/hw/acpi/aml-build.c > > +++ b/hw/acpi/aml-build.c > > @@ -1994,6 +1994,57 @@ static void build_processor_hierarchy_node(GArray *tbl, uint32_t flags, > > } > > } > > > > +void build_spcr(GArray *table_data, BIOSLinker *linker, > > + const AcpiSpcrData *f, const char *oem_id, > > + const char *oem_table_id) > > +{ > > + AcpiTable table = { .sig = "SPCR", .rev = 2, .oem_id = oem_id, > > + .oem_table_id = oem_table_id }; > > + > > + acpi_table_begin(&table, table_data); > > + /* Interface type */ > > + build_append_int_noprefix(table_data, f->interface_type, 1); > > + /* Reserved */ > > + build_append_int_noprefix(table_data, 0, 3); > > + /* Base Address */ > > + build_append_gas(table_data, f->base_addr.id, f->base_addr.width, > > + f->base_addr.offset, f->base_addr.size, > > + f->base_addr.addr); > > + /* Interrupt type */ > > + build_append_int_noprefix(table_data, f->interrupt_type, 1); > > + /* IRQ */ > > + build_append_int_noprefix(table_data, f->pc_interrupt, 1); > > + /* Global System Interrupt */ > > + build_append_int_noprefix(table_data, f->interrupt, 4); > > + /* Baud Rate */ > > + build_append_int_noprefix(table_data, f->baud_rate, 1); > > + /* Parity */ > > + build_append_int_noprefix(table_data, f->parity, 1); > > + /* Stop Bits */ > > + build_append_int_noprefix(table_data, f->stop_bits, 1); > > + /* Flow Control */ > > + build_append_int_noprefix(table_data, f->flow_control, 1); > > + /* Terminal Type */ > > + build_append_int_noprefix(table_data, f->terminal_type, 1); > > + /* PCI Device ID */ > > + build_append_int_noprefix(table_data, f->pci_device_id, 2); > > + /* PCI Vendor ID */ > > + build_append_int_noprefix(table_data, f->pci_vendor_id, 2); > > + /* PCI Bus Number */ > > + build_append_int_noprefix(table_data, f->pci_bus, 1); > > + /* PCI Device Number */ > > + build_append_int_noprefix(table_data, f->pci_device, 1); > > + /* PCI Function Number */ > > + build_append_int_noprefix(table_data, f->pci_function, 1); > > + /* PCI Flags */ > > + 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); > > + > > + acpi_table_end(linker, &table); > > +} > > /* > > * ACPI spec, Revision 6.3 > > * 5.2.29 Processor Properties Topology Table (PPTT) > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > > index 510ab0dcca..a31f736d1a 100644 > > --- a/hw/arm/virt-acpi-build.c > > +++ b/hw/arm/virt-acpi-build.c > > @@ -431,48 +431,34 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > > * Rev: 1.07 > > */ > > static void > > -build_spcr(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > > +build_spcr_v2(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > > Nit: I don't understand the '_v2' in the name of this function. Is it just to not collide > with the now public build_spcr()? Or does it have to do with the SPCR table being > '.rev = 2'? Because if it's the latter, you can name the common helper 'build_spcr_rev2' > (since both ARM and RISC-V use SPCR rev 2), keep this local build_spcr() initializing > the AcpiSpcrData struct with ARM attributes and then call the common build_spcr_rev2(). Sorry for the confusion. 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(). > > > Code looks good othewise. Thanks, > > > Daniel > > > > > { > > - AcpiTable table = { .sig = "SPCR", .rev = 2, .oem_id = vms->oem_id, > > - .oem_table_id = vms->oem_table_id }; > > - > > - acpi_table_begin(&table, table_data); > > - > > - /* Interface Type */ > > - build_append_int_noprefix(table_data, 3, 1); /* ARM PL011 UART */ > > - build_append_int_noprefix(table_data, 0, 3); /* Reserved */ > > - /* Base Address */ > > - build_append_gas(table_data, AML_AS_SYSTEM_MEMORY, 32, 0, 3, > > - vms->memmap[VIRT_UART].base); > > - /* Interrupt Type */ > > - build_append_int_noprefix(table_data, > > - (1 << 3) /* Bit[3] ARMH GIC interrupt */, 1); > > - build_append_int_noprefix(table_data, 0, 1); /* IRQ */ > > - /* Global System Interrupt */ > > - build_append_int_noprefix(table_data, > > - vms->irqmap[VIRT_UART] + ARM_SPI_BASE, 4); > > - build_append_int_noprefix(table_data, 3 /* 9600 */, 1); /* Baud Rate */ > > - build_append_int_noprefix(table_data, 0 /* No Parity */, 1); /* Parity */ > > - /* Stop Bits */ > > - build_append_int_noprefix(table_data, 1 /* 1 Stop bit */, 1); > > - /* Flow Control */ > > - build_append_int_noprefix(table_data, > > - (1 << 1) /* RTS/CTS hardware flow control */, 1); > > - /* Terminal Type */ > > - build_append_int_noprefix(table_data, 0 /* VT100 */, 1); > > - build_append_int_noprefix(table_data, 0, 1); /* Language */ > > - /* PCI Device ID */ > > - build_append_int_noprefix(table_data, 0xffff /* not a PCI device*/, 2); > > - /* PCI Vendor ID */ > > - build_append_int_noprefix(table_data, 0xffff /* not a PCI device*/, 2); > > - build_append_int_noprefix(table_data, 0, 1); /* PCI Bus Number */ > > - build_append_int_noprefix(table_data, 0, 1); /* PCI Device Number */ > > - build_append_int_noprefix(table_data, 0, 1); /* PCI Function Number */ > > - build_append_int_noprefix(table_data, 0, 4); /* PCI Flags */ > > - build_append_int_noprefix(table_data, 0, 1); /* PCI Segment */ > > - build_append_int_noprefix(table_data, 0, 4); /* Reserved */ > > + AcpiSpcrData serial = { > > + .interface_type = 3, /* ARM PL011 UART */ > > + .base_addr.id = AML_AS_SYSTEM_MEMORY, > > + .base_addr.width = 32, > > + .base_addr.offset = 0, > > + .base_addr.size = 3, > > + .base_addr.addr = vms->memmap[VIRT_UART].base, > > + .interrupt_type = (1 << 3),/* Bit[3] ARMH GIC interrupt*/ > > + .pc_interrupt = 0, /* IRQ */ > > + .interrupt = (vms->irqmap[VIRT_UART] + ARM_SPI_BASE), > > + .baud_rate = 3, /* 9600 */ > > + .parity = 0, /* No Parity */ > > + .stop_bits = 1, /* 1 Stop bit */ > > + .flow_control = 1 << 1, /* RTS/CTS hardware flow control */ > > + .terminal_type = 0, /* VT100 */ > > + .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, > > + }; > > > > - acpi_table_end(linker, &table); > > + build_spcr(table_data, linker, &serial, vms->oem_id, vms->oem_table_id); > > } > > > > /* > > @@ -930,7 +916,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables) > > } > > > > acpi_add_table(table_offsets, tables_blob); > > - build_spcr(tables_blob, tables->linker, vms); > > + build_spcr_v2(tables_blob, tables->linker, vms); > > > > acpi_add_table(table_offsets, tables_blob); > > build_dbg2(tables_blob, tables->linker, vms); > > diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h > > index 2b42e4192b..0e6e82b339 100644 > > --- a/include/hw/acpi/acpi-defs.h > > +++ b/include/hw/acpi/acpi-defs.h > > @@ -90,6 +90,39 @@ typedef struct AcpiFadtData { > > unsigned *xdsdt_tbl_offset; > > } AcpiFadtData; > > > > +typedef struct AcpiGas { > > + uint8_t id; /* Address space ID */ > > + uint8_t width; /* Register bit width */ > > + uint8_t offset; /* Register bit offset */ > > + uint8_t size; /* Access size */ > > + uint64_t addr; /* Address */ > > +} AcpiGas; > > + > > +/* SPCR (Serial Port Console Redirection table) */ > > +typedef struct AcpiSpcrData { > > + uint8_t interface_type; > > + uint8_t reserved[3]; > > + struct AcpiGas base_addr; > > + uint8_t interrupt_type; > > + uint8_t pc_interrupt; > > + uint32_t interrupt; /* Global system interrupt */ > > + uint8_t baud_rate; > > + uint8_t parity; > > + uint8_t stop_bits; > > + 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; > > + uint8_t pci_device; > > + uint8_t pci_function; > > + uint32_t pci_flags; > > + uint8_t pci_segment; > > + uint32_t reserved2; > > +} AcpiSpcrData; > > + > > #define ACPI_FADT_ARM_PSCI_COMPLIANT (1 << 0) > > #define ACPI_FADT_ARM_PSCI_USE_HVC (1 << 1) > > > > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h > > index ff2a310270..44e03e76bd 100644 > > --- a/include/hw/acpi/aml-build.h > > +++ b/include/hw/acpi/aml-build.h > > @@ -497,4 +497,8 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f, > > > > void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog, > > const char *oem_id, const char *oem_table_id); > > + > > +void build_spcr(GArray *table_data, BIOSLinker *linker, > > + const AcpiSpcrData *f, const char *oem_id, > > + const char *oem_table_id); > > #endif ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [RESEND RFC v1 1/2] hw/arm/virt-acpi-build.c: Migrate SPCR creation to common location 2024-01-10 8:02 ` JeeHeng Sia @ 2024-01-10 9:23 ` JeeHeng Sia 0 siblings, 0 replies; 10+ messages in thread From: JeeHeng Sia @ 2024-01-10 9:23 UTC (permalink / raw) To: Daniel Henrique Barboza, 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 > -----Original Message----- > From: JeeHeng Sia > Sent: Wednesday, January 10, 2024 4:02 PM > To: Daniel Henrique Barboza <dbarboza@ventanamicro.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 1/2] hw/arm/virt-acpi-build.c: Migrate SPCR creation to common location > > > > > -----Original Message----- > > From: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > > Sent: Friday, January 5, 2024 8:19 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 1/2] hw/arm/virt-acpi-build.c: Migrate SPCR creation to common location > > > > > > > > On 1/5/24 06:06, Sia Jee Heng wrote: > > > RISC-V should also generate the SPCR in a manner similar to ARM. > > > Therefore, instead of replicating the code, relocate this function > > > to the common AML build. > > > > > > Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com> > > > --- > > > hw/acpi/aml-build.c | 51 ++++++++++++++++++++++++++++ > > > hw/arm/virt-acpi-build.c | 68 +++++++++++++++---------------------- > > > include/hw/acpi/acpi-defs.h | 33 ++++++++++++++++++ > > > include/hw/acpi/aml-build.h | 4 +++ > > > 4 files changed, 115 insertions(+), 41 deletions(-) > > > > > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > > > index af66bde0f5..1efa534aa8 100644 > > > --- a/hw/acpi/aml-build.c > > > +++ b/hw/acpi/aml-build.c > > > @@ -1994,6 +1994,57 @@ static void build_processor_hierarchy_node(GArray *tbl, uint32_t flags, > > > } > > > } > > > > > > +void build_spcr(GArray *table_data, BIOSLinker *linker, > > > + const AcpiSpcrData *f, const char *oem_id, > > > + const char *oem_table_id) > > > +{ > > > + AcpiTable table = { .sig = "SPCR", .rev = 2, .oem_id = oem_id, > > > + .oem_table_id = oem_table_id }; > > > + > > > + acpi_table_begin(&table, table_data); > > > + /* Interface type */ > > > + build_append_int_noprefix(table_data, f->interface_type, 1); > > > + /* Reserved */ > > > + build_append_int_noprefix(table_data, 0, 3); > > > + /* Base Address */ > > > + build_append_gas(table_data, f->base_addr.id, f->base_addr.width, > > > + f->base_addr.offset, f->base_addr.size, > > > + f->base_addr.addr); > > > + /* Interrupt type */ > > > + build_append_int_noprefix(table_data, f->interrupt_type, 1); > > > + /* IRQ */ > > > + build_append_int_noprefix(table_data, f->pc_interrupt, 1); > > > + /* Global System Interrupt */ > > > + build_append_int_noprefix(table_data, f->interrupt, 4); > > > + /* Baud Rate */ > > > + build_append_int_noprefix(table_data, f->baud_rate, 1); > > > + /* Parity */ > > > + build_append_int_noprefix(table_data, f->parity, 1); > > > + /* Stop Bits */ > > > + build_append_int_noprefix(table_data, f->stop_bits, 1); > > > + /* Flow Control */ > > > + build_append_int_noprefix(table_data, f->flow_control, 1); > > > + /* Terminal Type */ > > > + build_append_int_noprefix(table_data, f->terminal_type, 1); > > > + /* PCI Device ID */ > > > + build_append_int_noprefix(table_data, f->pci_device_id, 2); > > > + /* PCI Vendor ID */ > > > + build_append_int_noprefix(table_data, f->pci_vendor_id, 2); > > > + /* PCI Bus Number */ > > > + build_append_int_noprefix(table_data, f->pci_bus, 1); > > > + /* PCI Device Number */ > > > + build_append_int_noprefix(table_data, f->pci_device, 1); > > > + /* PCI Function Number */ > > > + build_append_int_noprefix(table_data, f->pci_function, 1); > > > + /* PCI Flags */ > > > + 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); > > > + > > > + acpi_table_end(linker, &table); > > > +} > > > /* > > > * ACPI spec, Revision 6.3 > > > * 5.2.29 Processor Properties Topology Table (PPTT) > > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > > > index 510ab0dcca..a31f736d1a 100644 > > > --- a/hw/arm/virt-acpi-build.c > > > +++ b/hw/arm/virt-acpi-build.c > > > @@ -431,48 +431,34 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > > > * Rev: 1.07 > > > */ > > > static void > > > -build_spcr(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > > > +build_spcr_v2(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > > > > Nit: I don't understand the '_v2' in the name of this function. Is it just to not collide > > with the now public build_spcr()? Or does it have to do with the SPCR table being > > '.rev = 2'? Because if it's the latter, you can name the common helper 'build_spcr_rev2' > > (since both ARM and RISC-V use SPCR rev 2), keep this local build_spcr() initializing > > the AcpiSpcrData struct with ARM attributes and then call the common build_spcr_rev2(). > Sorry for the confusion. 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(). > > > > > > Code looks good othewise. Thanks, Btw, as Sunil's code [1] has been merged into Alistair's riscv-to-apply.next, I will remove the word "RFC" from the next submission. Hope you guys are okay. > > > > > > Daniel > > > > > > > > > { > > > - AcpiTable table = { .sig = "SPCR", .rev = 2, .oem_id = vms->oem_id, > > > - .oem_table_id = vms->oem_table_id }; > > > - > > > - acpi_table_begin(&table, table_data); > > > - > > > - /* Interface Type */ > > > - build_append_int_noprefix(table_data, 3, 1); /* ARM PL011 UART */ > > > - build_append_int_noprefix(table_data, 0, 3); /* Reserved */ > > > - /* Base Address */ > > > - build_append_gas(table_data, AML_AS_SYSTEM_MEMORY, 32, 0, 3, > > > - vms->memmap[VIRT_UART].base); > > > - /* Interrupt Type */ > > > - build_append_int_noprefix(table_data, > > > - (1 << 3) /* Bit[3] ARMH GIC interrupt */, 1); > > > - build_append_int_noprefix(table_data, 0, 1); /* IRQ */ > > > - /* Global System Interrupt */ > > > - build_append_int_noprefix(table_data, > > > - vms->irqmap[VIRT_UART] + ARM_SPI_BASE, 4); > > > - build_append_int_noprefix(table_data, 3 /* 9600 */, 1); /* Baud Rate */ > > > - build_append_int_noprefix(table_data, 0 /* No Parity */, 1); /* Parity */ > > > - /* Stop Bits */ > > > - build_append_int_noprefix(table_data, 1 /* 1 Stop bit */, 1); > > > - /* Flow Control */ > > > - build_append_int_noprefix(table_data, > > > - (1 << 1) /* RTS/CTS hardware flow control */, 1); > > > - /* Terminal Type */ > > > - build_append_int_noprefix(table_data, 0 /* VT100 */, 1); > > > - build_append_int_noprefix(table_data, 0, 1); /* Language */ > > > - /* PCI Device ID */ > > > - build_append_int_noprefix(table_data, 0xffff /* not a PCI device*/, 2); > > > - /* PCI Vendor ID */ > > > - build_append_int_noprefix(table_data, 0xffff /* not a PCI device*/, 2); > > > - build_append_int_noprefix(table_data, 0, 1); /* PCI Bus Number */ > > > - build_append_int_noprefix(table_data, 0, 1); /* PCI Device Number */ > > > - build_append_int_noprefix(table_data, 0, 1); /* PCI Function Number */ > > > - build_append_int_noprefix(table_data, 0, 4); /* PCI Flags */ > > > - build_append_int_noprefix(table_data, 0, 1); /* PCI Segment */ > > > - build_append_int_noprefix(table_data, 0, 4); /* Reserved */ > > > + AcpiSpcrData serial = { > > > + .interface_type = 3, /* ARM PL011 UART */ > > > + .base_addr.id = AML_AS_SYSTEM_MEMORY, > > > + .base_addr.width = 32, > > > + .base_addr.offset = 0, > > > + .base_addr.size = 3, > > > + .base_addr.addr = vms->memmap[VIRT_UART].base, > > > + .interrupt_type = (1 << 3),/* Bit[3] ARMH GIC interrupt*/ > > > + .pc_interrupt = 0, /* IRQ */ > > > + .interrupt = (vms->irqmap[VIRT_UART] + ARM_SPI_BASE), > > > + .baud_rate = 3, /* 9600 */ > > > + .parity = 0, /* No Parity */ > > > + .stop_bits = 1, /* 1 Stop bit */ > > > + .flow_control = 1 << 1, /* RTS/CTS hardware flow control */ > > > + .terminal_type = 0, /* VT100 */ > > > + .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, > > > + }; > > > > > > - acpi_table_end(linker, &table); > > > + build_spcr(table_data, linker, &serial, vms->oem_id, vms->oem_table_id); > > > } > > > > > > /* > > > @@ -930,7 +916,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables) > > > } > > > > > > acpi_add_table(table_offsets, tables_blob); > > > - build_spcr(tables_blob, tables->linker, vms); > > > + build_spcr_v2(tables_blob, tables->linker, vms); > > > > > > acpi_add_table(table_offsets, tables_blob); > > > build_dbg2(tables_blob, tables->linker, vms); > > > diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h > > > index 2b42e4192b..0e6e82b339 100644 > > > --- a/include/hw/acpi/acpi-defs.h > > > +++ b/include/hw/acpi/acpi-defs.h > > > @@ -90,6 +90,39 @@ typedef struct AcpiFadtData { > > > unsigned *xdsdt_tbl_offset; > > > } AcpiFadtData; > > > > > > +typedef struct AcpiGas { > > > + uint8_t id; /* Address space ID */ > > > + uint8_t width; /* Register bit width */ > > > + uint8_t offset; /* Register bit offset */ > > > + uint8_t size; /* Access size */ > > > + uint64_t addr; /* Address */ > > > +} AcpiGas; > > > + > > > +/* SPCR (Serial Port Console Redirection table) */ > > > +typedef struct AcpiSpcrData { > > > + uint8_t interface_type; > > > + uint8_t reserved[3]; > > > + struct AcpiGas base_addr; > > > + uint8_t interrupt_type; > > > + uint8_t pc_interrupt; > > > + uint32_t interrupt; /* Global system interrupt */ > > > + uint8_t baud_rate; > > > + uint8_t parity; > > > + uint8_t stop_bits; > > > + 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; > > > + uint8_t pci_device; > > > + uint8_t pci_function; > > > + uint32_t pci_flags; > > > + uint8_t pci_segment; > > > + uint32_t reserved2; > > > +} AcpiSpcrData; > > > + > > > #define ACPI_FADT_ARM_PSCI_COMPLIANT (1 << 0) > > > #define ACPI_FADT_ARM_PSCI_USE_HVC (1 << 1) > > > > > > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h > > > index ff2a310270..44e03e76bd 100644 > > > --- a/include/hw/acpi/aml-build.h > > > +++ b/include/hw/acpi/aml-build.h > > > @@ -497,4 +497,8 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f, > > > > > > void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog, > > > const char *oem_id, const char *oem_table_id); > > > + > > > +void build_spcr(GArray *table_data, BIOSLinker *linker, > > > + const AcpiSpcrData *f, const char *oem_id, > > > + const char *oem_table_id); > > > #endif ^ permalink raw reply [flat|nested] 10+ messages in thread
* [RESEND RFC v1 2/2] hw/riscv/virt-acpi-build.c: Generate SPCR table 2024-01-05 9:06 [RESEND RFC v1 0/2] RISC-V: ACPI: Enable SPCR Sia Jee Heng 2024-01-05 9:06 ` [RESEND RFC v1 1/2] hw/arm/virt-acpi-build.c: Migrate SPCR creation to common location Sia Jee Heng @ 2024-01-05 9:06 ` Sia Jee Heng 2024-01-05 12:27 ` Daniel Henrique Barboza 1 sibling, 1 reply; 10+ messages in thread From: Sia Jee Heng @ 2024-01-05 9:06 UTC (permalink / raw) To: qemu-arm, qemu-devel, qemu-riscv Cc: mst, imammedo, anisinha, peter.maydell, jeeheng.sia, shannon.zhaosl, sunilvl, palmer, alistair.francis, bin.meng, liwei1518, dbarboza, zhiwei_liu 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 + */ + +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 = { -- 2.34.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [RESEND RFC v1 2/2] hw/riscv/virt-acpi-build.c: Generate SPCR table 2024-01-05 9:06 ` [RESEND RFC v1 2/2] hw/riscv/virt-acpi-build.c: Generate SPCR table Sia Jee Heng @ 2024-01-05 12:27 ` Daniel Henrique Barboza 2024-01-10 8:00 ` JeeHeng Sia 0 siblings, 1 reply; 10+ messages in thread From: Daniel Henrique Barboza @ 2024-01-05 12:27 UTC (permalink / raw) To: Sia Jee Heng, qemu-arm, qemu-devel, qemu-riscv Cc: mst, imammedo, anisinha, peter.maydell, shannon.zhaosl, sunilvl, palmer, alistair.francis, bin.meng, liwei1518, zhiwei_liu 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 + 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 = { ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [RESEND RFC v1 2/2] hw/riscv/virt-acpi-build.c: Generate SPCR table 2024-01-05 12:27 ` Daniel Henrique Barboza @ 2024-01-10 8:00 ` JeeHeng Sia 0 siblings, 0 replies; 10+ messages in thread From: JeeHeng Sia @ 2024-01-10 8:00 UTC (permalink / raw) To: Daniel Henrique Barboza, 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 > -----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 = { ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-01-10 9:24 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-01-05 9:06 [RESEND RFC v1 0/2] RISC-V: ACPI: Enable SPCR Sia Jee Heng 2024-01-05 9:06 ` [RESEND RFC v1 1/2] hw/arm/virt-acpi-build.c: Migrate SPCR creation to common location Sia Jee Heng 2024-01-05 12:19 ` Daniel Henrique Barboza 2024-01-08 11:34 ` Sunil V L 2024-01-10 8:03 ` JeeHeng Sia 2024-01-10 8:02 ` JeeHeng Sia 2024-01-10 9:23 ` JeeHeng Sia 2024-01-05 9:06 ` [RESEND RFC v1 2/2] hw/riscv/virt-acpi-build.c: Generate SPCR table Sia Jee Heng 2024-01-05 12:27 ` Daniel Henrique Barboza 2024-01-10 8:00 ` JeeHeng Sia
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).