* [PATCH V4 1/8] hw/riscv/virt: Add OEM_ID and OEM_TABLE_ID fields
2023-02-24 8:36 [PATCH V4 0/8] Add basic ACPI support for risc-v virt Sunil V L
@ 2023-02-24 8:36 ` Sunil V L
2023-02-24 8:36 ` [PATCH V4 2/8] hw/riscv/virt: Add a switch to disable ACPI Sunil V L
` (6 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Sunil V L @ 2023-02-24 8:36 UTC (permalink / raw)
To: Palmer Dabbelt, Alistair Francis, Bin Meng
Cc: qemu-riscv, qemu-devel, Andrew Jones, Anup Patel,
Atish Kumar Patra, Daniel Henrique Barboza, Sunil V L, Bin Meng
ACPI needs OEM_ID and OEM_TABLE_ID for the machine. Add these fields
in the RISCVVirtState structure and initialize with default values.
Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
Reviewed-by: Bin Meng <bmeng@tinylab.org>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
---
hw/riscv/virt.c | 5 +++++
include/hw/riscv/virt.h | 2 ++
2 files changed, 7 insertions(+)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index b81081c70b..4118d28468 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -49,6 +49,7 @@
#include "hw/pci/pci.h"
#include "hw/pci-host/gpex.h"
#include "hw/display/ramfb.h"
+#include "hw/acpi/aml-build.h"
/*
* The virt machine physical address space used by some of the devices
@@ -1521,6 +1522,10 @@ static void virt_machine_init(MachineState *machine)
static void virt_machine_instance_init(Object *obj)
{
+ RISCVVirtState *s = RISCV_VIRT_MACHINE(obj);
+
+ s->oem_id = g_strndup(ACPI_BUILD_APPNAME6, 6);
+ s->oem_table_id = g_strndup(ACPI_BUILD_APPNAME8, 8);
}
static char *virt_get_aia_guests(Object *obj, Error **errp)
diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
index b3d26135c0..6c7885bf89 100644
--- a/include/hw/riscv/virt.h
+++ b/include/hw/riscv/virt.h
@@ -56,6 +56,8 @@ struct RISCVVirtState {
bool have_aclint;
RISCVVirtAIAType aia_type;
int aia_guests;
+ char *oem_id;
+ char *oem_table_id;
};
enum {
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH V4 2/8] hw/riscv/virt: Add a switch to disable ACPI
2023-02-24 8:36 [PATCH V4 0/8] Add basic ACPI support for risc-v virt Sunil V L
2023-02-24 8:36 ` [PATCH V4 1/8] hw/riscv/virt: Add OEM_ID and OEM_TABLE_ID fields Sunil V L
@ 2023-02-24 8:36 ` Sunil V L
2023-02-24 8:36 ` [PATCH V4 3/8] hw/riscv/virt: Add memmap pointer to RiscVVirtState Sunil V L
` (5 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Sunil V L @ 2023-02-24 8:36 UTC (permalink / raw)
To: Palmer Dabbelt, Alistair Francis, Bin Meng
Cc: qemu-riscv, qemu-devel, Andrew Jones, Anup Patel,
Atish Kumar Patra, Daniel Henrique Barboza, Sunil V L
ACPI will be enabled by default. Add a switch to turn off
for testing and debug purposes.
Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
---
hw/riscv/virt.c | 29 +++++++++++++++++++++++++++++
include/hw/riscv/virt.h | 2 ++
2 files changed, 31 insertions(+)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 4118d28468..89d5b9d8aa 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -50,6 +50,7 @@
#include "hw/pci-host/gpex.h"
#include "hw/display/ramfb.h"
#include "hw/acpi/aml-build.h"
+#include "qapi/qapi-visit-common.h"
/*
* The virt machine physical address space used by some of the devices
@@ -1526,6 +1527,7 @@ static void virt_machine_instance_init(Object *obj)
s->oem_id = g_strndup(ACPI_BUILD_APPNAME6, 6);
s->oem_table_id = g_strndup(ACPI_BUILD_APPNAME8, 8);
+ s->acpi = ON_OFF_AUTO_AUTO;
}
static char *virt_get_aia_guests(Object *obj, Error **errp)
@@ -1600,6 +1602,28 @@ static void virt_set_aclint(Object *obj, bool value, Error **errp)
s->have_aclint = value;
}
+bool virt_is_acpi_enabled(RISCVVirtState *s)
+{
+ return s->acpi != ON_OFF_AUTO_OFF;
+}
+
+static void virt_get_acpi(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ RISCVVirtState *s = RISCV_VIRT_MACHINE(obj);
+ OnOffAuto acpi = s->acpi;
+
+ visit_type_OnOffAuto(v, name, &acpi, errp);
+}
+
+static void virt_set_acpi(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ RISCVVirtState *s = RISCV_VIRT_MACHINE(obj);
+
+ visit_type_OnOffAuto(v, name, &s->acpi, errp);
+}
+
static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
DeviceState *dev)
{
@@ -1671,6 +1695,11 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
sprintf(str, "Set number of guest MMIO pages for AIA IMSIC. Valid value "
"should be between 0 and %d.", VIRT_IRQCHIP_MAX_GUESTS);
object_class_property_set_description(oc, "aia-guests", str);
+ object_class_property_add(oc, "acpi", "OnOffAuto",
+ virt_get_acpi, virt_set_acpi,
+ NULL, NULL);
+ object_class_property_set_description(oc, "acpi",
+ "Enable ACPI");
}
static const TypeInfo virt_machine_typeinfo = {
diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
index 6c7885bf89..62efebaa32 100644
--- a/include/hw/riscv/virt.h
+++ b/include/hw/riscv/virt.h
@@ -58,6 +58,7 @@ struct RISCVVirtState {
int aia_guests;
char *oem_id;
char *oem_table_id;
+ OnOffAuto acpi;
};
enum {
@@ -123,4 +124,5 @@ enum {
#define FDT_APLIC_INT_MAP_WIDTH (FDT_PCI_ADDR_CELLS + FDT_PCI_INT_CELLS + \
1 + FDT_APLIC_INT_CELLS)
+bool virt_is_acpi_enabled(RISCVVirtState *s);
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH V4 3/8] hw/riscv/virt: Add memmap pointer to RiscVVirtState
2023-02-24 8:36 [PATCH V4 0/8] Add basic ACPI support for risc-v virt Sunil V L
2023-02-24 8:36 ` [PATCH V4 1/8] hw/riscv/virt: Add OEM_ID and OEM_TABLE_ID fields Sunil V L
2023-02-24 8:36 ` [PATCH V4 2/8] hw/riscv/virt: Add a switch to disable ACPI Sunil V L
@ 2023-02-24 8:36 ` Sunil V L
2023-02-24 8:36 ` [PATCH V4 4/8] hw/riscv/virt: Enable basic ACPI infrastructure Sunil V L
` (4 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Sunil V L @ 2023-02-24 8:36 UTC (permalink / raw)
To: Palmer Dabbelt, Alistair Francis, Bin Meng
Cc: qemu-riscv, qemu-devel, Andrew Jones, Anup Patel,
Atish Kumar Patra, Daniel Henrique Barboza, Sunil V L, Bin Meng
memmap needs to be exported outside of virt.c so that
modules like acpi can use it. Hence, add a pointer field
in RiscVVirtState structure and initialize it with the
memorymap.
Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
Reviewed-by: Bin Meng <bmeng@tinylab.org>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
---
hw/riscv/virt.c | 2 ++
include/hw/riscv/virt.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 89d5b9d8aa..bcbacf4e63 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1459,6 +1459,8 @@ static void virt_machine_init(MachineState *machine)
ROUND_UP(virt_high_pcie_memmap.base, virt_high_pcie_memmap.size);
}
+ s->memmap = virt_memmap;
+
/* register system main memory (actual RAM) */
memory_region_add_subregion(system_memory, memmap[VIRT_DRAM].base,
machine->ram);
diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
index 62efebaa32..379501edcc 100644
--- a/include/hw/riscv/virt.h
+++ b/include/hw/riscv/virt.h
@@ -59,6 +59,7 @@ struct RISCVVirtState {
char *oem_id;
char *oem_table_id;
OnOffAuto acpi;
+ const MemMapEntry *memmap;
};
enum {
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH V4 4/8] hw/riscv/virt: Enable basic ACPI infrastructure
2023-02-24 8:36 [PATCH V4 0/8] Add basic ACPI support for risc-v virt Sunil V L
` (2 preceding siblings ...)
2023-02-24 8:36 ` [PATCH V4 3/8] hw/riscv/virt: Add memmap pointer to RiscVVirtState Sunil V L
@ 2023-02-24 8:36 ` Sunil V L
2023-02-24 8:36 ` [PATCH V4 5/8] hw/riscv/virt: virt-acpi-build.c: Add RINTC in MADT Sunil V L
` (3 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Sunil V L @ 2023-02-24 8:36 UTC (permalink / raw)
To: Palmer Dabbelt, Alistair Francis, Bin Meng
Cc: qemu-riscv, qemu-devel, Andrew Jones, Anup Patel,
Atish Kumar Patra, Daniel Henrique Barboza, Sunil V L
Add basic ACPI infrastructure for RISC-V with below tables.
1) DSDT with below basic objects
- CPUs
- fw_cfg
2) FADT revision 6 with HW_REDUCED flag
3) XSDT
4) RSDP
Add this functionality in a new file virt-acpi-build.c and enable
building this infrastructure.
Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
---
hw/riscv/Kconfig | 1 +
hw/riscv/meson.build | 1 +
hw/riscv/virt-acpi-build.c | 276 +++++++++++++++++++++++++++++++++++++
include/hw/riscv/virt.h | 1 +
4 files changed, 279 insertions(+)
create mode 100644 hw/riscv/virt-acpi-build.c
diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
index 4550b3b938..6528ebfa3a 100644
--- a/hw/riscv/Kconfig
+++ b/hw/riscv/Kconfig
@@ -44,6 +44,7 @@ config RISCV_VIRT
select VIRTIO_MMIO
select FW_CFG_DMA
select PLATFORM_BUS
+ select ACPI
config SHAKTI_C
bool
diff --git a/hw/riscv/meson.build b/hw/riscv/meson.build
index ab6cae57ea..2f7ee81be3 100644
--- a/hw/riscv/meson.build
+++ b/hw/riscv/meson.build
@@ -9,5 +9,6 @@ riscv_ss.add(when: 'CONFIG_SIFIVE_E', if_true: files('sifive_e.c'))
riscv_ss.add(when: 'CONFIG_SIFIVE_U', if_true: files('sifive_u.c'))
riscv_ss.add(when: 'CONFIG_SPIKE', if_true: files('spike.c'))
riscv_ss.add(when: 'CONFIG_MICROCHIP_PFSOC', if_true: files('microchip_pfsoc.c'))
+riscv_ss.add(when: 'CONFIG_ACPI', if_true: files('virt-acpi-build.c'))
hw_arch += {'riscv': riscv_ss}
diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
new file mode 100644
index 0000000000..3a5e2e6d53
--- /dev/null
+++ b/hw/riscv/virt-acpi-build.c
@@ -0,0 +1,276 @@
+/*
+ * Support for generating ACPI tables and passing them to Guests
+ *
+ * RISC-V virt ACPI generation
+ *
+ * Copyright (C) 2008-2010 Kevin O'Connor <kevin@koconnor.net>
+ * Copyright (C) 2006 Fabrice Bellard
+ * Copyright (C) 2013 Red Hat Inc
+ * Copyright (c) 2015 HUAWEI TECHNOLOGIES CO.,LTD.
+ * Copyright (C) 2021-2023 Ventana Micro Systems Inc
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/acpi/acpi-defs.h"
+#include "hw/acpi/acpi.h"
+#include "hw/acpi/aml-build.h"
+#include "hw/acpi/utils.h"
+#include "qapi/error.h"
+#include "sysemu/reset.h"
+#include "migration/vmstate.h"
+#include "hw/riscv/virt.h"
+
+#define ACPI_BUILD_TABLE_SIZE 0x20000
+
+typedef struct AcpiBuildState {
+ /* Copy of table in RAM (for patching) */
+ MemoryRegion *table_mr;
+ MemoryRegion *rsdp_mr;
+ MemoryRegion *linker_mr;
+ /* Is table patched? */
+ bool patched;
+} AcpiBuildState;
+
+static void acpi_align_size(GArray *blob, unsigned align)
+{
+ /*
+ * Align size to multiple of given size. This reduces the chance
+ * we need to change size in the future (breaking cross version migration).
+ */
+ g_array_set_size(blob, ROUND_UP(acpi_data_len(blob), align));
+}
+
+static void acpi_dsdt_add_cpus(Aml *scope, RISCVVirtState *s)
+{
+ MachineState *ms = MACHINE(s);
+ uint16_t i;
+
+ for (i = 0; i < ms->smp.cpus; i++) {
+ Aml *dev = aml_device("C%.03X", i);
+ aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0007")));
+ aml_append(dev, aml_name_decl("_UID", aml_int(i)));
+ aml_append(scope, dev);
+ }
+}
+
+static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap)
+{
+ Aml *dev = aml_device("FWCF");
+ aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
+
+ /* device present, functioning, decoding, not shown in UI */
+ aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
+ aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
+
+ Aml *crs = aml_resource_template();
+ aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base,
+ fw_cfg_memmap->size, AML_READ_WRITE));
+ aml_append(dev, aml_name_decl("_CRS", crs));
+ aml_append(scope, dev);
+}
+
+/* FADT */
+static void build_fadt_rev6(GArray *table_data,
+ BIOSLinker *linker,
+ RISCVVirtState *s,
+ unsigned dsdt_tbl_offset)
+{
+ AcpiFadtData fadt = {
+ .rev = 6,
+ .minor_ver = 0,
+ .flags = 1 << ACPI_FADT_F_HW_REDUCED_ACPI,
+ .xdsdt_tbl_offset = &dsdt_tbl_offset,
+ };
+
+ build_fadt(table_data, linker, &fadt, s->oem_id, s->oem_table_id);
+}
+
+/* DSDT */
+static void build_dsdt(GArray *table_data,
+ BIOSLinker *linker,
+ RISCVVirtState *s)
+{
+ Aml *scope, *dsdt;
+ const MemMapEntry *memmap = s->memmap;
+ AcpiTable table = { .sig = "DSDT", .rev = 2, .oem_id = s->oem_id,
+ .oem_table_id = s->oem_table_id };
+
+
+ acpi_table_begin(&table, table_data);
+ dsdt = init_aml_allocator();
+
+ /*
+ * When booting the VM with UEFI, UEFI takes ownership of the RTC hardware.
+ * While UEFI can use libfdt to disable the RTC device node in the DTB that
+ * it passes to the OS, it cannot modify AML. Therefore, we won't generate
+ * the RTC ACPI device at all when using UEFI.
+ */
+ scope = aml_scope("\\_SB");
+ acpi_dsdt_add_cpus(scope, s);
+
+ acpi_dsdt_add_fw_cfg(scope, &memmap[VIRT_FW_CFG]);
+
+ aml_append(dsdt, scope);
+
+ /* copy AML table into ACPI tables blob and patch header there */
+ g_array_append_vals(table_data, dsdt->buf->data, dsdt->buf->len);
+
+ acpi_table_end(linker, &table);
+ free_aml_allocator();
+}
+
+static void virt_acpi_build(RISCVVirtState *s, AcpiBuildTables *tables)
+{
+ GArray *table_offsets;
+ unsigned dsdt, xsdt;
+ GArray *tables_blob = tables->table_data;
+
+ table_offsets = g_array_new(false, true,
+ sizeof(uint32_t));
+
+ bios_linker_loader_alloc(tables->linker,
+ ACPI_BUILD_TABLE_FILE, tables_blob,
+ 64, false);
+
+ /* DSDT is pointed to by FADT */
+ dsdt = tables_blob->len;
+ build_dsdt(tables_blob, tables->linker, s);
+
+ /* FADT and others pointed to by XSDT */
+ acpi_add_table(table_offsets, tables_blob);
+ build_fadt_rev6(tables_blob, tables->linker, s, dsdt);
+
+ /* XSDT is pointed to by RSDP */
+ xsdt = tables_blob->len;
+ build_xsdt(tables_blob, tables->linker, table_offsets, s->oem_id,
+ s->oem_table_id);
+
+ /* RSDP is in FSEG memory, so allocate it separately */
+ {
+ AcpiRsdpData rsdp_data = {
+ .revision = 2,
+ .oem_id = s->oem_id,
+ .xsdt_tbl_offset = &xsdt,
+ .rsdt_tbl_offset = NULL,
+ };
+ build_rsdp(tables->rsdp, tables->linker, &rsdp_data);
+ }
+
+ /*
+ * The align size is 128, warn if 64k is not enough therefore
+ * the align size could be resized.
+ */
+ if (tables_blob->len > ACPI_BUILD_TABLE_SIZE / 2) {
+ warn_report("ACPI table size %u exceeds %d bytes,"
+ " migration may not work",
+ tables_blob->len, ACPI_BUILD_TABLE_SIZE / 2);
+ error_printf("Try removing some objects.");
+ }
+
+ acpi_align_size(tables_blob, ACPI_BUILD_TABLE_SIZE);
+
+ /* Clean up memory that's no longer used */
+ g_array_free(table_offsets, true);
+}
+
+static void acpi_ram_update(MemoryRegion *mr, GArray *data)
+{
+ uint32_t size = acpi_data_len(data);
+
+ /*
+ * Make sure RAM size is correct - in case it got changed
+ * e.g. by migration
+ */
+ memory_region_ram_resize(mr, size, &error_abort);
+
+ memcpy(memory_region_get_ram_ptr(mr), data->data, size);
+ memory_region_set_dirty(mr, 0, size);
+}
+
+static void virt_acpi_build_update(void *build_opaque)
+{
+ AcpiBuildState *build_state = build_opaque;
+ AcpiBuildTables tables;
+
+ /* No state to update or already patched? Nothing to do. */
+ if (!build_state || build_state->patched) {
+ return;
+ }
+
+ build_state->patched = true;
+
+ acpi_build_tables_init(&tables);
+
+ virt_acpi_build(RISCV_VIRT_MACHINE(qdev_get_machine()), &tables);
+
+ acpi_ram_update(build_state->table_mr, tables.table_data);
+ acpi_ram_update(build_state->rsdp_mr, tables.rsdp);
+ acpi_ram_update(build_state->linker_mr, tables.linker->cmd_blob);
+
+ acpi_build_tables_cleanup(&tables, true);
+}
+
+static void virt_acpi_build_reset(void *build_opaque)
+{
+ AcpiBuildState *build_state = build_opaque;
+ build_state->patched = false;
+}
+
+static const VMStateDescription vmstate_virt_acpi_build = {
+ .name = "virt_acpi_build",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_BOOL(patched, AcpiBuildState),
+ VMSTATE_END_OF_LIST()
+ },
+};
+
+void virt_acpi_setup(RISCVVirtState *s)
+{
+ AcpiBuildTables tables;
+ AcpiBuildState *build_state;
+
+ build_state = g_malloc0(sizeof *build_state);
+
+ acpi_build_tables_init(&tables);
+ virt_acpi_build(s, &tables);
+
+ /* Now expose it all to Guest */
+ build_state->table_mr = acpi_add_rom_blob(virt_acpi_build_update,
+ build_state, tables.table_data,
+ ACPI_BUILD_TABLE_FILE);
+ assert(build_state->table_mr != NULL);
+
+ build_state->linker_mr = acpi_add_rom_blob(virt_acpi_build_update,
+ build_state,
+ tables.linker->cmd_blob,
+ ACPI_BUILD_LOADER_FILE);
+
+ build_state->rsdp_mr = acpi_add_rom_blob(virt_acpi_build_update,
+ build_state, tables.rsdp,
+ ACPI_BUILD_RSDP_FILE);
+
+ qemu_register_reset(virt_acpi_build_reset, build_state);
+ virt_acpi_build_reset(build_state);
+ vmstate_register(NULL, 0, &vmstate_virt_acpi_build, build_state);
+
+ /*
+ * Clean up tables but don't free the memory: we track it
+ * in build_state.
+ */
+ acpi_build_tables_cleanup(&tables, false);
+}
diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
index 379501edcc..e5c474b26e 100644
--- a/include/hw/riscv/virt.h
+++ b/include/hw/riscv/virt.h
@@ -126,4 +126,5 @@ enum {
1 + FDT_APLIC_INT_CELLS)
bool virt_is_acpi_enabled(RISCVVirtState *s);
+void virt_acpi_setup(RISCVVirtState *vms);
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH V4 5/8] hw/riscv/virt: virt-acpi-build.c: Add RINTC in MADT
2023-02-24 8:36 [PATCH V4 0/8] Add basic ACPI support for risc-v virt Sunil V L
` (3 preceding siblings ...)
2023-02-24 8:36 ` [PATCH V4 4/8] hw/riscv/virt: Enable basic ACPI infrastructure Sunil V L
@ 2023-02-24 8:36 ` Sunil V L
2023-02-24 12:53 ` Igor Mammedov
2023-02-24 8:36 ` [PATCH V4 6/8] hw/riscv/virt: virt-acpi-build.c: Add RHCT Table Sunil V L
` (2 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Sunil V L @ 2023-02-24 8:36 UTC (permalink / raw)
To: Palmer Dabbelt, Alistair Francis, Bin Meng
Cc: qemu-riscv, qemu-devel, Andrew Jones, Anup Patel,
Atish Kumar Patra, Daniel Henrique Barboza, Sunil V L
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 */
+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++;
+ }
+ }
+
+ acpi_table_end(linker, &table);
+}
+
static void virt_acpi_build(RISCVVirtState *s, AcpiBuildTables *tables)
{
GArray *table_offsets;
@@ -153,6 +194,9 @@ static void virt_acpi_build(RISCVVirtState *s, AcpiBuildTables *tables)
acpi_add_table(table_offsets, tables_blob);
build_fadt_rev6(tables_blob, tables->linker, s, dsdt);
+ acpi_add_table(table_offsets, tables_blob);
+ build_madt(tables_blob, tables->linker, s);
+
/* XSDT is pointed to by RSDP */
xsdt = tables_blob->len;
build_xsdt(tables_blob, tables->linker, table_offsets, s->oem_id,
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH V4 5/8] hw/riscv/virt: virt-acpi-build.c: Add RINTC in MADT
2023-02-24 8:36 ` [PATCH V4 5/8] hw/riscv/virt: virt-acpi-build.c: Add RINTC in MADT Sunil V L
@ 2023-02-24 12:53 ` Igor Mammedov
2023-02-24 14:26 ` Sunil V L
0 siblings, 1 reply; 16+ messages in thread
From: Igor Mammedov @ 2023-02-24 12:53 UTC (permalink / raw)
To: Sunil V L
Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, qemu-riscv,
qemu-devel, Andrew Jones, Anup Patel, Atish Kumar Patra,
Daniel Henrique Barboza
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
> +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++;
> + }
> + }
> +
> + acpi_table_end(linker, &table);
> +}
> +
> static void virt_acpi_build(RISCVVirtState *s, AcpiBuildTables *tables)
> {
> GArray *table_offsets;
> @@ -153,6 +194,9 @@ static void virt_acpi_build(RISCVVirtState *s, AcpiBuildTables *tables)
> acpi_add_table(table_offsets, tables_blob);
> build_fadt_rev6(tables_blob, tables->linker, s, dsdt);
>
> + acpi_add_table(table_offsets, tables_blob);
> + build_madt(tables_blob, tables->linker, s);
> +
> /* XSDT is pointed to by RSDP */
> xsdt = tables_blob->len;
> build_xsdt(tables_blob, tables->linker, table_offsets, s->oem_id,
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH V4 5/8] hw/riscv/virt: virt-acpi-build.c: Add RINTC in MADT
2023-02-24 12:53 ` Igor Mammedov
@ 2023-02-24 14:26 ` Sunil V L
2023-02-27 15:41 ` Igor Mammedov
0 siblings, 1 reply; 16+ messages in thread
From: Sunil V L @ 2023-02-24 14:26 UTC (permalink / raw)
To: Igor Mammedov
Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, qemu-riscv,
qemu-devel, Andrew Jones, Anup Patel, Atish Kumar Patra,
Daniel Henrique Barboza
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?
> > +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.
Thank you very much for your review!
Sunil
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH V4 5/8] hw/riscv/virt: virt-acpi-build.c: Add RINTC in MADT
2023-02-24 14:26 ` Sunil V L
@ 2023-02-27 15:41 ` Igor Mammedov
2023-02-28 7:34 ` Sunil V L
0 siblings, 1 reply; 16+ messages in thread
From: Igor Mammedov @ 2023-02-27 15:41 UTC (permalink / raw)
To: Sunil V L
Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, qemu-riscv,
qemu-devel, Andrew Jones, Anup Patel, Atish Kumar Patra,
Daniel Henrique Barboza
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.
>
> > > +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.
> Thank you very much for your review!
> Sunil
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH V4 5/8] hw/riscv/virt: virt-acpi-build.c: Add RINTC in MADT
2023-02-27 15:41 ` Igor Mammedov
@ 2023-02-28 7:34 ` Sunil V L
2023-02-28 16:52 ` Igor Mammedov
0 siblings, 1 reply; 16+ messages in thread
From: Sunil V L @ 2023-02-28 7:34 UTC (permalink / raw)
To: Igor Mammedov
Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, qemu-riscv,
qemu-devel, Andrew Jones, Anup Patel, Atish Kumar Patra,
Daniel Henrique Barboza
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!
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH V4 5/8] hw/riscv/virt: virt-acpi-build.c: Add RINTC in MADT
2023-02-28 7:34 ` Sunil V L
@ 2023-02-28 16:52 ` Igor Mammedov
0 siblings, 0 replies; 16+ messages in thread
From: Igor Mammedov @ 2023-02-28 16:52 UTC (permalink / raw)
To: Sunil V L
Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, qemu-riscv,
qemu-devel, Andrew Jones, Anup Patel, Atish Kumar Patra,
Daniel Henrique Barboza
On Tue, 28 Feb 2023 13:04:36 +0530
Sunil V L <sunilvl@ventanamicro.com> 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 <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!
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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH V4 6/8] hw/riscv/virt: virt-acpi-build.c: Add RHCT Table
2023-02-24 8:36 [PATCH V4 0/8] Add basic ACPI support for risc-v virt Sunil V L
` (4 preceding siblings ...)
2023-02-24 8:36 ` [PATCH V4 5/8] hw/riscv/virt: virt-acpi-build.c: Add RINTC in MADT Sunil V L
@ 2023-02-24 8:36 ` Sunil V L
2023-02-24 12:55 ` Igor Mammedov
2023-02-24 8:37 ` [PATCH V4 7/8] hw/riscv/virt.c: Initialize the ACPI tables Sunil V L
2023-02-24 8:37 ` [PATCH V4 8/8] MAINTAINERS: Add entry for RISC-V ACPI Sunil V L
7 siblings, 1 reply; 16+ messages in thread
From: Sunil V L @ 2023-02-24 8:36 UTC (permalink / raw)
To: Palmer Dabbelt, Alistair Francis, Bin Meng
Cc: qemu-riscv, qemu-devel, Andrew Jones, Anup Patel,
Atish Kumar Patra, Daniel Henrique Barboza, Sunil V L
RISC-V ACPI platforms need to provide RISC-V Hart Capabilities
Table (RHCT). Add this to the ACPI tables.
Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
---
hw/riscv/virt-acpi-build.c | 76 ++++++++++++++++++++++++++++++++++++++
1 file changed, 76 insertions(+)
diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
index 8b85b34c55..7037fe7634 100644
--- a/hw/riscv/virt-acpi-build.c
+++ b/hw/riscv/virt-acpi-build.c
@@ -33,6 +33,7 @@
#include "migration/vmstate.h"
#include "hw/riscv/virt.h"
#include "hw/riscv/numa.h"
+#include "hw/intc/riscv_aclint.h"
#define ACPI_BUILD_TABLE_SIZE 0x20000
@@ -83,6 +84,78 @@ static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap)
aml_append(scope, dev);
}
+#define RHCT_NODE_ARRAY_OFFSET 56
+static void build_rhct(GArray *table_data,
+ BIOSLinker *linker,
+ RISCVVirtState *s)
+{
+ MachineState *ms = MACHINE(s);
+ uint32_t acpi_proc_id = 0;
+ int i, socket;
+ RISCVCPU *cpu;
+ char *isa;
+ size_t len, aligned_len;
+ uint32_t isa_offset, num_rhct_nodes;
+
+ AcpiTable table = { .sig = "RHCT", .rev = 1, .oem_id = s->oem_id,
+ .oem_table_id = s->oem_table_id };
+
+ acpi_table_begin(&table, table_data);
+
+ build_append_int_noprefix(table_data, 0x0, 4); /* Reserved */
+
+ /* Time Base Frequency */
+ build_append_int_noprefix(table_data,
+ RISCV_ACLINT_DEFAULT_TIMEBASE_FREQ, 8);
+
+ /* ISA + N hart info */
+ num_rhct_nodes = 1 + ms->smp.cpus;
+
+ /* Number of RHCT nodes*/
+ build_append_int_noprefix(table_data, num_rhct_nodes, 4);
+
+ /* Offset to the RHCT node array */
+ build_append_int_noprefix(table_data, RHCT_NODE_ARRAY_OFFSET, 4);
+
+ /* ISA string node */
+ isa_offset = table_data->len - table.table_offset;
+ build_append_int_noprefix(table_data, 0, 2); /* Type 0 */
+
+ cpu = &s->soc[0].harts[0];
+ isa = riscv_isa_string(cpu);
+ len = 8 + strlen(isa) + 1;
+ aligned_len = (len % 2) ? (len + 1) : len;
+
+ build_append_int_noprefix(table_data, aligned_len, 2); /* Length */
+ build_append_int_noprefix(table_data, 0x1, 2); /* Revision */
+
+ /* ISA string length including NUL */
+ build_append_int_noprefix(table_data, strlen(isa) + 1, 2);
+ g_array_append_vals(table_data, isa, strlen(isa) + 1); /* ISA string */
+
+ if (aligned_len != len) {
+ build_append_int_noprefix(table_data, 0x0, 1); /* Optional Padding */
+ }
+
+ for (socket = 0; socket < riscv_socket_count(ms); socket++) {
+ for (i = 0; i < s->soc[socket].num_harts; i++) {
+ build_append_int_noprefix(table_data, 0xFFFF, 2); /* Type */
+ build_append_int_noprefix(table_data, 16, 2); /* Length */
+ build_append_int_noprefix(table_data, 0x1, 2); /* Revision */
+ build_append_int_noprefix(table_data, 1, 2); /* Number of offsets */
+
+ /* ACPI Processor UID */
+ build_append_int_noprefix(table_data, acpi_proc_id, 4);
+
+ /* Offsets[0] */
+ build_append_int_noprefix(table_data, isa_offset, 4);
+ acpi_proc_id++;
+ }
+ }
+
+ acpi_table_end(linker, &table);
+}
+
/* FADT */
static void build_fadt_rev6(GArray *table_data,
BIOSLinker *linker,
@@ -197,6 +270,9 @@ static void virt_acpi_build(RISCVVirtState *s, AcpiBuildTables *tables)
acpi_add_table(table_offsets, tables_blob);
build_madt(tables_blob, tables->linker, s);
+ acpi_add_table(table_offsets, tables_blob);
+ build_rhct(tables_blob, tables->linker, s);
+
/* XSDT is pointed to by RSDP */
xsdt = tables_blob->len;
build_xsdt(tables_blob, tables->linker, table_offsets, s->oem_id,
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH V4 6/8] hw/riscv/virt: virt-acpi-build.c: Add RHCT Table
2023-02-24 8:36 ` [PATCH V4 6/8] hw/riscv/virt: virt-acpi-build.c: Add RHCT Table Sunil V L
@ 2023-02-24 12:55 ` Igor Mammedov
0 siblings, 0 replies; 16+ messages in thread
From: Igor Mammedov @ 2023-02-24 12:55 UTC (permalink / raw)
To: Sunil V L
Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, qemu-riscv,
qemu-devel, Andrew Jones, Anup Patel, Atish Kumar Patra,
Daniel Henrique Barboza
On Fri, 24 Feb 2023 14:06:59 +0530
Sunil V L <sunilvl@ventanamicro.com> wrote:
> RISC-V ACPI platforms need to provide RISC-V Hart Capabilities
> Table (RHCT). Add this to the ACPI tables.
>
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> ---
> hw/riscv/virt-acpi-build.c | 76 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 76 insertions(+)
>
> diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
> index 8b85b34c55..7037fe7634 100644
> --- a/hw/riscv/virt-acpi-build.c
> +++ b/hw/riscv/virt-acpi-build.c
> @@ -33,6 +33,7 @@
> #include "migration/vmstate.h"
> #include "hw/riscv/virt.h"
> #include "hw/riscv/numa.h"
> +#include "hw/intc/riscv_aclint.h"
>
> #define ACPI_BUILD_TABLE_SIZE 0x20000
>
> @@ -83,6 +84,78 @@ static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap)
> aml_append(scope, dev);
> }
>
> +#define RHCT_NODE_ARRAY_OFFSET 56
same as previous patch, here should be proper comment
otherwise reviewer has no clue where to look for reference.
> +static void build_rhct(GArray *table_data,
> + BIOSLinker *linker,
> + RISCVVirtState *s)
> +{
> + MachineState *ms = MACHINE(s);
> + uint32_t acpi_proc_id = 0;
> + int i, socket;
> + RISCVCPU *cpu;
> + char *isa;
> + size_t len, aligned_len;
> + uint32_t isa_offset, num_rhct_nodes;
> +
> + AcpiTable table = { .sig = "RHCT", .rev = 1, .oem_id = s->oem_id,
> + .oem_table_id = s->oem_table_id };
> +
> + acpi_table_begin(&table, table_data);
> +
> + build_append_int_noprefix(table_data, 0x0, 4); /* Reserved */
> +
> + /* Time Base Frequency */
> + build_append_int_noprefix(table_data,
> + RISCV_ACLINT_DEFAULT_TIMEBASE_FREQ, 8);
> +
> + /* ISA + N hart info */
> + num_rhct_nodes = 1 + ms->smp.cpus;
> +
> + /* Number of RHCT nodes*/
> + build_append_int_noprefix(table_data, num_rhct_nodes, 4);
> +
> + /* Offset to the RHCT node array */
> + build_append_int_noprefix(table_data, RHCT_NODE_ARRAY_OFFSET, 4);
> +
> + /* ISA string node */
> + isa_offset = table_data->len - table.table_offset;
> + build_append_int_noprefix(table_data, 0, 2); /* Type 0 */
> +
> + cpu = &s->soc[0].harts[0];
> + isa = riscv_isa_string(cpu);
> + len = 8 + strlen(isa) + 1;
> + aligned_len = (len % 2) ? (len + 1) : len;
> +
> + build_append_int_noprefix(table_data, aligned_len, 2); /* Length */
> + build_append_int_noprefix(table_data, 0x1, 2); /* Revision */
> +
> + /* ISA string length including NUL */
> + build_append_int_noprefix(table_data, strlen(isa) + 1, 2);
> + g_array_append_vals(table_data, isa, strlen(isa) + 1); /* ISA string */
> +
> + if (aligned_len != len) {
> + build_append_int_noprefix(table_data, 0x0, 1); /* Optional Padding */
> + }
> +
> + for (socket = 0; socket < riscv_socket_count(ms); socket++) {
> + for (i = 0; i < s->soc[socket].num_harts; i++) {
> + build_append_int_noprefix(table_data, 0xFFFF, 2); /* Type */
> + build_append_int_noprefix(table_data, 16, 2); /* Length */
> + build_append_int_noprefix(table_data, 0x1, 2); /* Revision */
> + build_append_int_noprefix(table_data, 1, 2); /* Number of offsets */
> +
> + /* ACPI Processor UID */
> + build_append_int_noprefix(table_data, acpi_proc_id, 4);
> +
> + /* Offsets[0] */
> + build_append_int_noprefix(table_data, isa_offset, 4);
> + acpi_proc_id++;
> + }
> + }
> +
> + acpi_table_end(linker, &table);
> +}
> +
> /* FADT */
> static void build_fadt_rev6(GArray *table_data,
> BIOSLinker *linker,
> @@ -197,6 +270,9 @@ static void virt_acpi_build(RISCVVirtState *s, AcpiBuildTables *tables)
> acpi_add_table(table_offsets, tables_blob);
> build_madt(tables_blob, tables->linker, s);
>
> + acpi_add_table(table_offsets, tables_blob);
> + build_rhct(tables_blob, tables->linker, s);
> +
> /* XSDT is pointed to by RSDP */
> xsdt = tables_blob->len;
> build_xsdt(tables_blob, tables->linker, table_offsets, s->oem_id,
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH V4 7/8] hw/riscv/virt.c: Initialize the ACPI tables
2023-02-24 8:36 [PATCH V4 0/8] Add basic ACPI support for risc-v virt Sunil V L
` (5 preceding siblings ...)
2023-02-24 8:36 ` [PATCH V4 6/8] hw/riscv/virt: virt-acpi-build.c: Add RHCT Table Sunil V L
@ 2023-02-24 8:37 ` Sunil V L
2023-02-24 8:37 ` [PATCH V4 8/8] MAINTAINERS: Add entry for RISC-V ACPI Sunil V L
7 siblings, 0 replies; 16+ messages in thread
From: Sunil V L @ 2023-02-24 8:37 UTC (permalink / raw)
To: Palmer Dabbelt, Alistair Francis, Bin Meng
Cc: qemu-riscv, qemu-devel, Andrew Jones, Anup Patel,
Atish Kumar Patra, Daniel Henrique Barboza, Sunil V L, Bin Meng
Initialize the ACPI tables if the acpi option is not
disabled.
Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
Reviewed-by: Bin Meng <bmeng@tinylab.org>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
---
hw/riscv/virt.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index bcbacf4e63..126352d480 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1324,6 +1324,10 @@ static void virt_machine_done(Notifier *notifier, void *data)
if (kvm_enabled()) {
riscv_setup_direct_kernel(kernel_entry, fdt_load_addr);
}
+
+ if (virt_is_acpi_enabled(s)) {
+ virt_acpi_setup(s);
+ }
}
static void virt_machine_init(MachineState *machine)
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH V4 8/8] MAINTAINERS: Add entry for RISC-V ACPI
2023-02-24 8:36 [PATCH V4 0/8] Add basic ACPI support for risc-v virt Sunil V L
` (6 preceding siblings ...)
2023-02-24 8:37 ` [PATCH V4 7/8] hw/riscv/virt.c: Initialize the ACPI tables Sunil V L
@ 2023-02-24 8:37 ` Sunil V L
2023-02-24 10:29 ` Andrew Jones
7 siblings, 1 reply; 16+ messages in thread
From: Sunil V L @ 2023-02-24 8:37 UTC (permalink / raw)
To: Palmer Dabbelt, Alistair Francis, Bin Meng
Cc: qemu-riscv, qemu-devel, Andrew Jones, Anup Patel,
Atish Kumar Patra, Daniel Henrique Barboza, Sunil V L, Bin Meng
RISC-V ACPI related functionality for virt machine is added in
virt-acpi-build.c. Add the maintainer entry after moving the
ARM ACPI entry under the main ACPI entry.
Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
Reviewed-by: Bin Meng <bmeng@tinylab.org>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
---
MAINTAINERS | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 9adb628627..7a47c2c724 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -994,12 +994,6 @@ S: Maintained
F: hw/ssi/xlnx-versal-ospi.c
F: include/hw/ssi/xlnx-versal-ospi.h
-ARM ACPI Subsystem
-M: Shannon Zhao <shannon.zhaosl@gmail.com>
-L: qemu-arm@nongnu.org
-S: Maintained
-F: hw/arm/virt-acpi-build.c
-
STM32F100
M: Alexandre Iooss <erdnaxe@crans.org>
L: qemu-arm@nongnu.org
@@ -1907,6 +1901,18 @@ F: hw/acpi/ghes.c
F: include/hw/acpi/ghes.h
F: docs/specs/acpi_hest_ghes.rst
+ARM ACPI Subsystem
+M: Shannon Zhao <shannon.zhaosl@gmail.com>
+L: qemu-arm@nongnu.org
+S: Maintained
+F: hw/arm/virt-acpi-build.c
+
+RISC-V ACPI Subsystem
+M: Sunil V L <sunilvl@ventanamicro.com>
+L: qemu-riscv@nongnu.org
+S: Maintained
+F: hw/riscv/virt-acpi-build.c
+
ppc4xx
L: qemu-ppc@nongnu.org
S: Orphan
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH V4 8/8] MAINTAINERS: Add entry for RISC-V ACPI
2023-02-24 8:37 ` [PATCH V4 8/8] MAINTAINERS: Add entry for RISC-V ACPI Sunil V L
@ 2023-02-24 10:29 ` Andrew Jones
0 siblings, 0 replies; 16+ messages in thread
From: Andrew Jones @ 2023-02-24 10:29 UTC (permalink / raw)
To: Sunil V L
Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, qemu-riscv,
qemu-devel, Anup Patel, Atish Kumar Patra,
Daniel Henrique Barboza, Bin Meng
On Fri, Feb 24, 2023 at 02:07:01PM +0530, Sunil V L wrote:
> RISC-V ACPI related functionality for virt machine is added in
> virt-acpi-build.c. Add the maintainer entry after moving the
> ARM ACPI entry under the main ACPI entry.
>
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> Reviewed-by: Bin Meng <bmeng@tinylab.org>
> Acked-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> ---
> MAINTAINERS | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9adb628627..7a47c2c724 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -994,12 +994,6 @@ S: Maintained
> F: hw/ssi/xlnx-versal-ospi.c
> F: include/hw/ssi/xlnx-versal-ospi.h
>
> -ARM ACPI Subsystem
> -M: Shannon Zhao <shannon.zhaosl@gmail.com>
> -L: qemu-arm@nongnu.org
> -S: Maintained
> -F: hw/arm/virt-acpi-build.c
> -
> STM32F100
> M: Alexandre Iooss <erdnaxe@crans.org>
> L: qemu-arm@nongnu.org
> @@ -1907,6 +1901,18 @@ F: hw/acpi/ghes.c
> F: include/hw/acpi/ghes.h
> F: docs/specs/acpi_hest_ghes.rst
>
> +ARM ACPI Subsystem
> +M: Shannon Zhao <shannon.zhaosl@gmail.com>
> +L: qemu-arm@nongnu.org
> +S: Maintained
> +F: hw/arm/virt-acpi-build.c
> +
> +RISC-V ACPI Subsystem
> +M: Sunil V L <sunilvl@ventanamicro.com>
> +L: qemu-riscv@nongnu.org
> +S: Maintained
> +F: hw/riscv/virt-acpi-build.c
> +
> ppc4xx
> L: qemu-ppc@nongnu.org
> S: Orphan
> --
> 2.34.1
>
I probably would have put these sections directly under the main ACPI
section, "ACPI/SMBIOS", as these are "main" arch-specific sections,
but I wouldn't respin the series for that.
Maybe whichever maintainer picks up the patches can put the sections in
their preferred location.
Thanks,
drew
^ permalink raw reply [flat|nested] 16+ messages in thread