On Tue, 28 Feb 2023 13:04:36 +0530 Sunil V L <[email protected]> wrote:
> On Mon, Feb 27, 2023 at 04:41:21PM +0100, Igor Mammedov wrote: > > On Fri, 24 Feb 2023 19:56:58 +0530 > > Sunil V L <[email protected]> wrote: > > > > > Hi Igor, > > > > > > On Fri, Feb 24, 2023 at 01:53:43PM +0100, Igor Mammedov wrote: > > > > On Fri, 24 Feb 2023 14:06:58 +0530 > > > > Sunil V L <[email protected]> wrote: > > > > > > > > > Add Multiple APIC Description Table (MADT) with the > > > > > RINTC structure for each cpu. > > > > > > > > > > Signed-off-by: Sunil V L <[email protected]> > > > > > Acked-by: Alistair Francis <[email protected]> > > > > > Reviewed-by: Andrew Jones <[email protected]> > > > > > --- > > > > > hw/riscv/virt-acpi-build.c | 44 > > > > > ++++++++++++++++++++++++++++++++++++++ > > > > > 1 file changed, 44 insertions(+) > > > > > > > > > > diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c > > > > > index 3a5e2e6d53..8b85b34c55 100644 > > > > > --- a/hw/riscv/virt-acpi-build.c > > > > > +++ b/hw/riscv/virt-acpi-build.c > > > > > @@ -32,6 +32,7 @@ > > > > > #include "sysemu/reset.h" > > > > > #include "migration/vmstate.h" > > > > > #include "hw/riscv/virt.h" > > > > > +#include "hw/riscv/numa.h" > > > > > > > > > > #define ACPI_BUILD_TABLE_SIZE 0x20000 > > > > > > > > > > @@ -132,6 +133,46 @@ static void build_dsdt(GArray *table_data, > > > > > free_aml_allocator(); > > > > > } > > > > > > > > > > +/* MADT */ > > > > > > > > see build_srat() how this comment must look like > > > > > > > Currently, even though ECRs are approved, the ACPI spec is not released > > > for these MADT structures. I can add the spec version for the generic > > > MADT but not for the RINTC. Same issue with a new table RHCT. > > > What is the recommendation in such case? > > > > ther must be some draft variant of spec or a ticket where it was approved > > or a reference impl. somewhere. > > > Sure, I can add the ticket ID. Thanks! and a link, later when new spec is published we can update it to rev/chapter format. > > > > > > > > > +static void build_madt(GArray *table_data, > > > > > + BIOSLinker *linker, > > > > > + RISCVVirtState *s) > > > > > +{ > > > > > + MachineState *ms = MACHINE(s); > > > > > + int socket; > > > > > + uint16_t base_hartid = 0; > > > > > + uint32_t cpu_id = 0; > > > > > + > > > > > + AcpiTable table = { .sig = "APIC", .rev = 6, .oem_id = s->oem_id, > > > > > + .oem_table_id = s->oem_table_id }; > > > > > + > > > > > + acpi_table_begin(&table, table_data); > > > > > + /* Local Interrupt Controller Address */ > > > > > + build_append_int_noprefix(table_data, 0, 4); > > > > > + build_append_int_noprefix(table_data, 0, 4); /* MADT Flags */ > > > > > + > > > > > + /* RISC-V Local INTC structures per HART */ > > > > > + for (socket = 0; socket < riscv_socket_count(ms); socket++) { > > > > > + base_hartid = riscv_socket_first_hartid(ms, socket); > > > > > + > > > > > + for (int i = 0; i < s->soc[socket].num_harts; i++) { > > > > > + build_append_int_noprefix(table_data, 0x18, 1); /* > > > > > Type */ > > > > > + build_append_int_noprefix(table_data, 20, 1); /* > > > > > Length */ > > > > > + build_append_int_noprefix(table_data, 1, 1); /* > > > > > Version */ > > > > > + build_append_int_noprefix(table_data, 0, 1); /* > > > > > Reserved */ > > > > > + build_append_int_noprefix(table_data, 1, 4); /* > > > > > Flags */ > > > > > + build_append_int_noprefix(table_data, > > > > > + (base_hartid + i), 8); /* > > > > > Hart ID */ > > > > > + > > > > > + /* ACPI Processor UID */ > > > > > + build_append_int_noprefix(table_data, cpu_id, 4); > > > > > > > > cpu_id here seems to be unrelated to one in DSDT. > > > > Could you explain how socket/hartid and cpu_id are related to each > > > > other? > > > > > > > cpu_id should match the _UID. I needed two loops here to get the > > > base_hartid of the socket. hart_id is the unique ID for each hart > > > similar to MPIDR / APIC ID. I understand your point. Let me make DSDT > > > also created using two loops so that both match. > > > > Why not reuse possible CPUs to describe topology there and then > > use ids from it to build ACPI tables instead of inventing your own > > cpu topo all over the place? > > > > PS: look for possible_cpus and how it's used (virt-arm already uses it > > although partially). And I'd like to avoid adding new ad-hoc ways > > to describe CPU topology is current possible_cpu ca do the job. > > Okay, sure. Let me take a look at possible_cpus and use in cpu topology. > Thanks! maybe following could be of help: hw/i386/acpi-common.c:acpi_build_madt and see how madt_cpu is used.
