diff mbox series

[04/10] hw/riscv/virt: virt-acpi-build.c: Add basic ACPI tables

Message ID 20230202045223.2594627-5-sunilvl@ventanamicro.com (mailing list archive)
State New, archived
Headers show
Series Add basic ACPI support for risc-v virt | expand

Commit Message

Sunil V L Feb. 2, 2023, 4:52 a.m. UTC
Add few basic ACPI tables and DSDT with few devices in a
new file virt-acpi-build.c.

These are mostly leveraged from arm64.

Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
---
 hw/riscv/virt-acpi-build.c | 292 +++++++++++++++++++++++++++++++++++++
 include/hw/riscv/virt.h    |   1 +
 2 files changed, 293 insertions(+)
 create mode 100644 hw/riscv/virt-acpi-build.c

Comments

Bin Meng Feb. 6, 2023, 10:17 a.m. UTC | #1
On Thu, Feb 2, 2023 at 12:54 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
>
> Add few basic ACPI tables and DSDT with few devices in a
> new file virt-acpi-build.c.
>
> These are mostly leveraged from arm64.

There are lots of same ACPI codes existing in x86/arm/riscv. I believe
some refactoring work is needed before ACPI support fully lands on
RISC-V.
For example, we can extract the common part among x86/arm/riscv into a
separate file, like hw/acpi/acpi-build.c?

>
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> ---
>  hw/riscv/virt-acpi-build.c | 292 +++++++++++++++++++++++++++++++++++++
>  include/hw/riscv/virt.h    |   1 +
>  2 files changed, 293 insertions(+)
>  create mode 100644 hw/riscv/virt-acpi-build.c
>
> diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
> new file mode 100644
> index 0000000000..0410b955bd
> --- /dev/null
> +++ b/hw/riscv/virt-acpi-build.c
> @@ -0,0 +1,292 @@
> +/*
> + * 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/riscv/virt.h"
> +#include "hw/riscv/numa.h"
> +#include "hw/acpi/pci.h"
> +#include "hw/acpi/utils.h"
> +#include "sysemu/reset.h"
> +#include "hw/pci-host/gpex.h"
> +#include "qapi/error.h"
> +#include "migration/vmstate.h"
> +
> +#define ACPI_BUILD_TABLE_SIZE             0x20000
> +
> +typedef struct AcpiBuildState {
> +    /* Copy of table in RAM (for patching). */

nits: removing the ending .

> +    MemoryRegion *table_mr;
> +    MemoryRegion *rsdp_mr;
> +    MemoryRegion *linker_mr;
> +    /* Is table patched? */
> +    bool patched;
> +} AcpiBuildState;
> +
> +static void

nits: please put above in the same line

> +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 *vms)

QEMU convention is to use 's' for the model state, so:

s/vms/s/g

> +{
> +    MachineState *ms = MACHINE(vms);
> +    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 *vms, unsigned dsdt_tbl_offset)
> +{
> +    /* ACPI v5.1 */
> +    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, vms->oem_id, vms->oem_table_id);
> +}
> +
> +/* DSDT */
> +static void
> +build_dsdt(GArray *table_data, BIOSLinker *linker, RISCVVirtState *vms)
> +{
> +    Aml *scope, *dsdt;
> +    const MemMapEntry *memmap = vms->memmap;
> +    AcpiTable table = { .sig = "DSDT", .rev = 2, .oem_id = vms->oem_id,
> +                        .oem_table_id = vms->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, vms);
> +
> +    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 *vms, 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, vms);
> +
> +    /* FADT and others pointed to by RSDT */
> +    acpi_add_table(table_offsets, tables_blob);
> +    build_fadt_rev6(tables_blob, tables->linker, vms, dsdt);
> +
> +    acpi_add_table(table_offsets, tables_blob);
> +    build_madt(tables_blob, tables->linker, vms);
> +
> +    acpi_add_table(table_offsets, tables_blob);
> +    build_rhct(tables_blob, tables->linker, vms);
> +
> +    /* XSDT is pointed to by RSDP */
> +    xsdt = tables_blob->len;
> +    build_xsdt(tables_blob, tables->linker, table_offsets, vms->oem_id,
> +                vms->oem_table_id);
> +
> +    /* RSDP is in FSEG memory, so allocate it separately */
> +    {
> +        AcpiRsdpData rsdp_data = {
> +            .revision = 2,
> +            .oem_id = vms->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 CPUs, NUMA nodes, memory slots"
> +                     " or PCI bridges.");
> +    }
> +    acpi_align_size(tables_blob, ACPI_BUILD_TABLE_SIZE);
> +
> +
> +    /* Cleanup memory that's no longer used. */

Clean up

nits: removing the ending .

> +    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 *vms)
> +{
> +    AcpiBuildTables tables;
> +    AcpiBuildState *build_state;
> +
> +    build_state = g_malloc0(sizeof *build_state);
> +
> +    acpi_build_tables_init(&tables);
> +    virt_acpi_build(vms, &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);
> +
> +    /*
> +     * Cleanup tables but don't free the memory: we track it

s/Cleanup/Clean up

> +     * 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
> --

Regards,
Bin
Sunil V L Feb. 6, 2023, 1:24 p.m. UTC | #2
On Mon, Feb 06, 2023 at 06:17:50PM +0800, Bin Meng wrote:
> On Thu, Feb 2, 2023 at 12:54 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
> >
> > Add few basic ACPI tables and DSDT with few devices in a
> > new file virt-acpi-build.c.
> >
> > These are mostly leveraged from arm64.
> 
> There are lots of same ACPI codes existing in x86/arm/riscv. I believe
> some refactoring work is needed before ACPI support fully lands on
> RISC-V.
> For example, we can extract the common part among x86/arm/riscv into a
> separate file, like hw/acpi/acpi-build.c?
> 

While it appears like there is same code in multiple places, those
functions take architecture specific MachineState parameter. Some tables
like MADT though with same name, will have different contents for
different architectures.

Only one function which Daniel also pointed is the wrapper
acpi_align_size() which can be made common. I am not
sure whether it is worth of refactoring.

> >
> > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> > ---
> >  hw/riscv/virt-acpi-build.c | 292 +++++++++++++++++++++++++++++++++++++
> >  include/hw/riscv/virt.h    |   1 +
> >  2 files changed, 293 insertions(+)
> >  create mode 100644 hw/riscv/virt-acpi-build.c
> >
> > diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
> > new file mode 100644
> > index 0000000000..0410b955bd
> > --- /dev/null
> > +++ b/hw/riscv/virt-acpi-build.c
> > @@ -0,0 +1,292 @@
> > +/*
> > + * 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/riscv/virt.h"
> > +#include "hw/riscv/numa.h"
> > +#include "hw/acpi/pci.h"
> > +#include "hw/acpi/utils.h"
> > +#include "sysemu/reset.h"
> > +#include "hw/pci-host/gpex.h"
> > +#include "qapi/error.h"
> > +#include "migration/vmstate.h"
> > +
> > +#define ACPI_BUILD_TABLE_SIZE             0x20000
> > +
> > +typedef struct AcpiBuildState {
> > +    /* Copy of table in RAM (for patching). */
> 
> nits: removing the ending .
> 
> > +    MemoryRegion *table_mr;
> > +    MemoryRegion *rsdp_mr;
> > +    MemoryRegion *linker_mr;
> > +    /* Is table patched? */
> > +    bool patched;
> > +} AcpiBuildState;
> > +
> > +static void
> 
> nits: please put above in the same line
> 
> > +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 *vms)
> 
> QEMU convention is to use 's' for the model state, so:
> 
> s/vms/s/g
> 
Sure, Thanks!. Will update it when I send the next revision.

> > +{
> > +    MachineState *ms = MACHINE(vms);
> > +    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 *vms, unsigned dsdt_tbl_offset)
> > +{
> > +    /* ACPI v5.1 */
> > +    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, vms->oem_id, vms->oem_table_id);
> > +}
> > +
> > +/* DSDT */
> > +static void
> > +build_dsdt(GArray *table_data, BIOSLinker *linker, RISCVVirtState *vms)
> > +{
> > +    Aml *scope, *dsdt;
> > +    const MemMapEntry *memmap = vms->memmap;
> > +    AcpiTable table = { .sig = "DSDT", .rev = 2, .oem_id = vms->oem_id,
> > +                        .oem_table_id = vms->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, vms);
> > +
> > +    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 *vms, 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, vms);
> > +
> > +    /* FADT and others pointed to by RSDT */
> > +    acpi_add_table(table_offsets, tables_blob);
> > +    build_fadt_rev6(tables_blob, tables->linker, vms, dsdt);
> > +
> > +    acpi_add_table(table_offsets, tables_blob);
> > +    build_madt(tables_blob, tables->linker, vms);
> > +
> > +    acpi_add_table(table_offsets, tables_blob);
> > +    build_rhct(tables_blob, tables->linker, vms);
> > +
> > +    /* XSDT is pointed to by RSDP */
> > +    xsdt = tables_blob->len;
> > +    build_xsdt(tables_blob, tables->linker, table_offsets, vms->oem_id,
> > +                vms->oem_table_id);
> > +
> > +    /* RSDP is in FSEG memory, so allocate it separately */
> > +    {
> > +        AcpiRsdpData rsdp_data = {
> > +            .revision = 2,
> > +            .oem_id = vms->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 CPUs, NUMA nodes, memory slots"
> > +                     " or PCI bridges.");
> > +    }
> > +    acpi_align_size(tables_blob, ACPI_BUILD_TABLE_SIZE);
> > +
> > +
> > +    /* Cleanup memory that's no longer used. */
> 
> Clean up
> 
> nits: removing the ending .
> 
Okay.

> > +    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 *vms)
> > +{
> > +    AcpiBuildTables tables;
> > +    AcpiBuildState *build_state;
> > +
> > +    build_state = g_malloc0(sizeof *build_state);
> > +
> > +    acpi_build_tables_init(&tables);
> > +    virt_acpi_build(vms, &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);
> > +
> > +    /*
> > +     * Cleanup tables but don't free the memory: we track it
> 
> s/Cleanup/Clean up

Okay.

Thanks!
Sunil
> 
> > +     * 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
> > --
> 
> Regards,
> Bin
Bin Meng Feb. 7, 2023, 4:10 p.m. UTC | #3
On Mon, Feb 6, 2023 at 9:24 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
>
> On Mon, Feb 06, 2023 at 06:17:50PM +0800, Bin Meng wrote:
> > On Thu, Feb 2, 2023 at 12:54 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
> > >
> > > Add few basic ACPI tables and DSDT with few devices in a
> > > new file virt-acpi-build.c.
> > >
> > > These are mostly leveraged from arm64.
> >
> > There are lots of same ACPI codes existing in x86/arm/riscv. I believe
> > some refactoring work is needed before ACPI support fully lands on
> > RISC-V.
> > For example, we can extract the common part among x86/arm/riscv into a
> > separate file, like hw/acpi/acpi-build.c?
> >
>
> While it appears like there is same code in multiple places, those
> functions take architecture specific MachineState parameter. Some tables
> like MADT though with same name, will have different contents for
> different architectures.
>
> Only one function which Daniel also pointed is the wrapper
> acpi_align_size() which can be made common. I am not
> sure whether it is worth of refactoring.
>

It's more than that. For example,

With acpi_dsdt_add_cpus(), the only meaningful parameter is the number
of cpus, so there is no need to pass the architecture specific
MachineState as the parameter.

Likewise, acpi_dsdt_add_fw_cfg() can be made generic as well.

Regards,
Bin
Sunil V L Feb. 7, 2023, 6:15 p.m. UTC | #4
On Wed, Feb 08, 2023 at 12:10:55AM +0800, Bin Meng wrote:
> On Mon, Feb 6, 2023 at 9:24 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
> >
> > On Mon, Feb 06, 2023 at 06:17:50PM +0800, Bin Meng wrote:
> > > On Thu, Feb 2, 2023 at 12:54 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
> > > >
> > > > Add few basic ACPI tables and DSDT with few devices in a
> > > > new file virt-acpi-build.c.
> > > >
> > > > These are mostly leveraged from arm64.
> > >
> > > There are lots of same ACPI codes existing in x86/arm/riscv. I believe
> > > some refactoring work is needed before ACPI support fully lands on
> > > RISC-V.
> > > For example, we can extract the common part among x86/arm/riscv into a
> > > separate file, like hw/acpi/acpi-build.c?
> > >
> >
> > While it appears like there is same code in multiple places, those
> > functions take architecture specific MachineState parameter. Some tables
> > like MADT though with same name, will have different contents for
> > different architectures.
> >
> > Only one function which Daniel also pointed is the wrapper
> > acpi_align_size() which can be made common. I am not
> > sure whether it is worth of refactoring.
> >
> 
> It's more than that. For example,
> 
> With acpi_dsdt_add_cpus(), the only meaningful parameter is the number
> of cpus, so there is no need to pass the architecture specific
> MachineState as the parameter.
> 
I would not make this function common. The reason is, an architecture may
choose to attach different ACPI objects under the CPU node.

> Likewise, acpi_dsdt_add_fw_cfg() can be made generic as well.
> 
The issue is, these things are not exactly common across all architectures.
x86 has bit different data in these objects. While today it appears they
are same for riscv and arm, in future things may change for an architecture.
It doesn't look like it is a standard practice to build files under
hw/acpi for specific architectures. Hence, IMO, it is better to keep these
things in architecture specific folders to allow them to do differently in
future.

But I look forward for the feedback from other architecture maintainers on
this topic. My experience in qemu is very limited. So, I need help from
experts.

Thanks!
Sunil
Bin Meng Feb. 8, 2023, 1:06 a.m. UTC | #5
On Wed, Feb 8, 2023 at 2:15 AM Sunil V L <sunilvl@ventanamicro.com> wrote:
>
> On Wed, Feb 08, 2023 at 12:10:55AM +0800, Bin Meng wrote:
> > On Mon, Feb 6, 2023 at 9:24 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
> > >
> > > On Mon, Feb 06, 2023 at 06:17:50PM +0800, Bin Meng wrote:
> > > > On Thu, Feb 2, 2023 at 12:54 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
> > > > >
> > > > > Add few basic ACPI tables and DSDT with few devices in a
> > > > > new file virt-acpi-build.c.
> > > > >
> > > > > These are mostly leveraged from arm64.
> > > >
> > > > There are lots of same ACPI codes existing in x86/arm/riscv. I believe
> > > > some refactoring work is needed before ACPI support fully lands on
> > > > RISC-V.
> > > > For example, we can extract the common part among x86/arm/riscv into a
> > > > separate file, like hw/acpi/acpi-build.c?
> > > >
> > >
> > > While it appears like there is same code in multiple places, those
> > > functions take architecture specific MachineState parameter. Some tables
> > > like MADT though with same name, will have different contents for
> > > different architectures.
> > >
> > > Only one function which Daniel also pointed is the wrapper
> > > acpi_align_size() which can be made common. I am not
> > > sure whether it is worth of refactoring.
> > >
> >
> > It's more than that. For example,
> >
> > With acpi_dsdt_add_cpus(), the only meaningful parameter is the number
> > of cpus, so there is no need to pass the architecture specific
> > MachineState as the parameter.
> >
> I would not make this function common. The reason is, an architecture may
> choose to attach different ACPI objects under the CPU node.
>

Is it possible to insert architect specific CPU nodes after this
common API builds the basic CPU node in DSDT?

> > Likewise, acpi_dsdt_add_fw_cfg() can be made generic as well.
> >
> The issue is, these things are not exactly common across all architectures.
> x86 has bit different data in these objects. While today it appears they
> are same for riscv and arm, in future things may change for an architecture.
> It doesn't look like it is a standard practice to build files under
> hw/acpi for specific architectures. Hence, IMO, it is better to keep these
> things in architecture specific folders to allow them to do differently in
> future.
>

It looks like hw/acpi/ghes.c is for Arm from MAINTAINERS.

> But I look forward for the feedback from other architecture maintainers on
> this topic. My experience in qemu is very limited. So, I need help from
> experts.

+ Michael and Igor

Regards,
Bin
Sunil V L Feb. 8, 2023, 4:49 a.m. UTC | #6
On Wed, Feb 08, 2023 at 09:06:48AM +0800, Bin Meng wrote:
> On Wed, Feb 8, 2023 at 2:15 AM Sunil V L <sunilvl@ventanamicro.com> wrote:
> >
> > On Wed, Feb 08, 2023 at 12:10:55AM +0800, Bin Meng wrote:
> > > On Mon, Feb 6, 2023 at 9:24 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
> > > >
> > > > On Mon, Feb 06, 2023 at 06:17:50PM +0800, Bin Meng wrote:
> > > > > On Thu, Feb 2, 2023 at 12:54 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
> > > > > >
> > > > > > Add few basic ACPI tables and DSDT with few devices in a
> > > > > > new file virt-acpi-build.c.
> > > > > >
> > > > > > These are mostly leveraged from arm64.
> > > > >
> > > > > There are lots of same ACPI codes existing in x86/arm/riscv. I believe
> > > > > some refactoring work is needed before ACPI support fully lands on
> > > > > RISC-V.
> > > > > For example, we can extract the common part among x86/arm/riscv into a
> > > > > separate file, like hw/acpi/acpi-build.c?
> > > > >
> > > >
> > > > While it appears like there is same code in multiple places, those
> > > > functions take architecture specific MachineState parameter. Some tables
> > > > like MADT though with same name, will have different contents for
> > > > different architectures.
> > > >
> > > > Only one function which Daniel also pointed is the wrapper
> > > > acpi_align_size() which can be made common. I am not
> > > > sure whether it is worth of refactoring.
> > > >
> > >
> > > It's more than that. For example,
> > >
> > > With acpi_dsdt_add_cpus(), the only meaningful parameter is the number
> > > of cpus, so there is no need to pass the architecture specific
> > > MachineState as the parameter.
> > >
> > I would not make this function common. The reason is, an architecture may
> > choose to attach different ACPI objects under the CPU node.
> >
> 
> Is it possible to insert architect specific CPU nodes after this
> common API builds the basic CPU node in DSDT?
> 
No. They need to be added in the same loop. Otherwise, it will cause
issues to the AML interpreter.

> > > Likewise, acpi_dsdt_add_fw_cfg() can be made generic as well.
> > >
> > The issue is, these things are not exactly common across all architectures.
> > x86 has bit different data in these objects. While today it appears they
> > are same for riscv and arm, in future things may change for an architecture.
> > It doesn't look like it is a standard practice to build files under
> > hw/acpi for specific architectures. Hence, IMO, it is better to keep these
> > things in architecture specific folders to allow them to do differently in
> > future.
> >
> 
> It looks like hw/acpi/ghes.c is for Arm from MAINTAINERS.
> 
APEI is a standard feature but it is up to the architecture to use it or
not. Probably, it is maintained by ARM since they adopted first. But if
you look at hw/acpi/meson.build, it is not architecture specific.

> > But I look forward for the feedback from other architecture maintainers on
> > this topic. My experience in qemu is very limited. So, I need help from
> > experts.
> 
> + Michael and Igor
> 
Thanks!
Sunil
Igor Mammedov Feb. 24, 2023, 4:14 p.m. UTC | #7
On Wed, 8 Feb 2023 09:06:48 +0800
Bin Meng <bmeng.cn@gmail.com> wrote:

> On Wed, Feb 8, 2023 at 2:15 AM Sunil V L <sunilvl@ventanamicro.com> wrote:
> >
> > On Wed, Feb 08, 2023 at 12:10:55AM +0800, Bin Meng wrote:  
> > > On Mon, Feb 6, 2023 at 9:24 PM Sunil V L <sunilvl@ventanamicro.com> wrote:  
> > > >
> > > > On Mon, Feb 06, 2023 at 06:17:50PM +0800, Bin Meng wrote:  
> > > > > On Thu, Feb 2, 2023 at 12:54 PM Sunil V L <sunilvl@ventanamicro.com> wrote:  
> > > > > >
> > > > > > Add few basic ACPI tables and DSDT with few devices in a
> > > > > > new file virt-acpi-build.c.
> > > > > >
> > > > > > These are mostly leveraged from arm64.  
> > > > >
> > > > > There are lots of same ACPI codes existing in x86/arm/riscv. I believe
> > > > > some refactoring work is needed before ACPI support fully lands on
> > > > > RISC-V.
> > > > > For example, we can extract the common part among x86/arm/riscv into a
> > > > > separate file, like hw/acpi/acpi-build.c?
> > > > >  
> > > >
> > > > While it appears like there is same code in multiple places, those
> > > > functions take architecture specific MachineState parameter. Some tables
> > > > like MADT though with same name, will have different contents for
> > > > different architectures.
> > > >
> > > > Only one function which Daniel also pointed is the wrapper
> > > > acpi_align_size() which can be made common. I am not
> > > > sure whether it is worth of refactoring.
> > > >  
> > >
> > > It's more than that. For example,
> > >
> > > With acpi_dsdt_add_cpus(), the only meaningful parameter is the number
> > > of cpus, so there is no need to pass the architecture specific
> > > MachineState as the parameter.
> > >  
> > I would not make this function common. The reason is, an architecture may
> > choose to attach different ACPI objects under the CPU node.
> >  
> 
> Is it possible to insert architect specific CPU nodes after this
> common API builds the basic CPU node in DSDT?

What do you mean by "architect specific CPU", any examples?

> 
> > > Likewise, acpi_dsdt_add_fw_cfg() can be made generic as well.
> > >  
> > The issue is, these things are not exactly common across all architectures.
> > x86 has bit different data in these objects. While today it appears they
> > are same for riscv and arm, in future things may change for an architecture.
> > It doesn't look like it is a standard practice to build files under
> > hw/acpi for specific architectures. Hence, IMO, it is better to keep these
> > things in architecture specific folders to allow them to do differently in
> > future.
> >  
> 
> It looks like hw/acpi/ghes.c is for Arm from MAINTAINERS.
> 
> > But I look forward for the feedback from other architecture maintainers on
> > this topic. My experience in qemu is very limited. So, I need help from
> > experts.  
> 
> + Michael and Igor
> 
> Regards,
> Bin
>
diff mbox series

Patch

diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
new file mode 100644
index 0000000000..0410b955bd
--- /dev/null
+++ b/hw/riscv/virt-acpi-build.c
@@ -0,0 +1,292 @@ 
+/*
+ * 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/riscv/virt.h"
+#include "hw/riscv/numa.h"
+#include "hw/acpi/pci.h"
+#include "hw/acpi/utils.h"
+#include "sysemu/reset.h"
+#include "hw/pci-host/gpex.h"
+#include "qapi/error.h"
+#include "migration/vmstate.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 *vms)
+{
+    MachineState *ms = MACHINE(vms);
+    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 *vms, unsigned dsdt_tbl_offset)
+{
+    /* ACPI v5.1 */
+    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, vms->oem_id, vms->oem_table_id);
+}
+
+/* DSDT */
+static void
+build_dsdt(GArray *table_data, BIOSLinker *linker, RISCVVirtState *vms)
+{
+    Aml *scope, *dsdt;
+    const MemMapEntry *memmap = vms->memmap;
+    AcpiTable table = { .sig = "DSDT", .rev = 2, .oem_id = vms->oem_id,
+                        .oem_table_id = vms->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, vms);
+
+    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 *vms, 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, vms);
+
+    /* FADT and others pointed to by RSDT */
+    acpi_add_table(table_offsets, tables_blob);
+    build_fadt_rev6(tables_blob, tables->linker, vms, dsdt);
+
+    acpi_add_table(table_offsets, tables_blob);
+    build_madt(tables_blob, tables->linker, vms);
+
+    acpi_add_table(table_offsets, tables_blob);
+    build_rhct(tables_blob, tables->linker, vms);
+
+    /* XSDT is pointed to by RSDP */
+    xsdt = tables_blob->len;
+    build_xsdt(tables_blob, tables->linker, table_offsets, vms->oem_id,
+                vms->oem_table_id);
+
+    /* RSDP is in FSEG memory, so allocate it separately */
+    {
+        AcpiRsdpData rsdp_data = {
+            .revision = 2,
+            .oem_id = vms->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 CPUs, NUMA nodes, memory slots"
+                     " or PCI bridges.");
+    }
+    acpi_align_size(tables_blob, ACPI_BUILD_TABLE_SIZE);
+
+
+    /* Cleanup 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 *vms)
+{
+    AcpiBuildTables tables;
+    AcpiBuildState *build_state;
+
+    build_state = g_malloc0(sizeof *build_state);
+
+    acpi_build_tables_init(&tables);
+    virt_acpi_build(vms, &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);
+
+    /*
+     * Cleanup 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