[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH V4 5/8] hw/riscv/virt: virt-acpi-build.c: Add RINTC in MADT
From: |
Sunil V L |
Subject: |
Re: [PATCH V4 5/8] hw/riscv/virt: virt-acpi-build.c: Add RINTC in MADT |
Date: |
Tue, 28 Feb 2023 13:04:36 +0530 |
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 <sunilvl@ventanamicro.com> 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 <sunilvl@ventanamicro.com> wrote:
> > >
> > > > Add Multiple APIC Description Table (MADT) with the
> > > > RINTC structure for each cpu.
> > > >
> > > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> > > > Acked-by: Alistair Francis <alistair.francis@wdc.com>
> > > > Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> > > > ---
> > > > 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!
> >
> > > > +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!
- [PATCH V4 0/8] Add basic ACPI support for risc-v virt, Sunil V L, 2023/02/24
- [PATCH V4 1/8] hw/riscv/virt: Add OEM_ID and OEM_TABLE_ID fields, Sunil V L, 2023/02/24
- [PATCH V4 2/8] hw/riscv/virt: Add a switch to disable ACPI, Sunil V L, 2023/02/24
- [PATCH V4 3/8] hw/riscv/virt: Add memmap pointer to RiscVVirtState, Sunil V L, 2023/02/24
- [PATCH V4 4/8] hw/riscv/virt: Enable basic ACPI infrastructure, Sunil V L, 2023/02/24
- [PATCH V4 5/8] hw/riscv/virt: virt-acpi-build.c: Add RINTC in MADT, Sunil V L, 2023/02/24
[PATCH V4 6/8] hw/riscv/virt: virt-acpi-build.c: Add RHCT Table, Sunil V L, 2023/02/24
[PATCH V4 7/8] hw/riscv/virt.c: Initialize the ACPI tables, Sunil V L, 2023/02/24
[PATCH V4 8/8] MAINTAINERS: Add entry for RISC-V ACPI, Sunil V L, 2023/02/24