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....@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