Re: [PULL 20/21] hw/loongarch: Add acpi ged support

[Peter Maydell]

On Tue, 19 Jul 2022 at 19:16, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> From: Xiaojuan Yang <yangxiaojuan@loongson.cn>
>
> Loongarch virt machine uses general hardware reduces acpi method, rather
> than LS7A acpi device. Now only power management function is used in
> acpi ged device, memory hotplug will be added later. Also acpi tables
> such as RSDP/RSDT/FADT etc.
>
> The acpi table has submited to acpi spec, and will release soon.
>
> Acked-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Xiaojuan Yang <yangxiaojuan@loongson.cn>
> Message-Id: <20220712083206.4187715-6-yangxiaojuan@loongson.cn>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Didn't notice this in the original CI, but this generates a compiler
warning on openbsd:

../src/hw/loongarch/acpi-build.c:416:12: warning: variable 'aml_len'
set but not used [-Wunused-but-set-variable]
    size_t aml_len = 0;
           ^

and we do indeed only ever seem to set and update that variable;
we never use it.

> +static void acpi_build(AcpiBuildTables *tables, MachineState *machine)
> +{
> +    LoongArchMachineState *lams = LOONGARCH_MACHINE(machine);
> +    GArray *table_offsets;
> +    AcpiFadtData fadt_data;
> +    unsigned facs, rsdt, fadt, dsdt;
> +    uint8_t *u;
> +    size_t aml_len = 0;
> +    GArray *tables_blob = tables->table_data;
> +
> +    init_common_fadt_data(&fadt_data);
> +
> +    table_offsets = g_array_new(false, true, sizeof(uint32_t));
> +    ACPI_BUILD_DPRINTF("init ACPI tables\n");
> +
> +    bios_linker_loader_alloc(tables->linker,
> +                             ACPI_BUILD_TABLE_FILE, tables_blob,
> +                             64, false);
> +
> +    /*
> +     * FACS is pointed to by FADT.
> +     * We place it first since it's the only table that has alignment
> +     * requirements.
> +     */
> +    facs = tables_blob->len;
> +    build_facs(tables_blob);
> +
> +    /* DSDT is pointed to by FADT */
> +    dsdt = tables_blob->len;
> +    build_dsdt(tables_blob, tables->linker, machine);
> +
> +    /*
> +     * Count the size of the DSDT, we will need it for
> +     * legacy sizing of ACPI tables.
> +     */
> +    aml_len += tables_blob->len - dsdt;

This comment claims we're going to use this value -- is it wrong ?

thanks
-- PMM