qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 18/26] hw: acpi: Initial hardware-reduced suppor


From: Paolo Bonzini
Subject: Re: [Qemu-devel] [PATCH 18/26] hw: acpi: Initial hardware-reduced support
Date: Wed, 24 Oct 2018 00:26:27 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0

On 22/10/2018 20:36, Samuel Ortiz wrote:
> +
> +static void acpi_reduced_build_update(void *build_opaque)
> +{
> +    MachineState *ms = MACHINE(build_opaque);
> +    AcpiBuildState *build_state = ms->firmware_build_state.acpi.state;
> +    AcpiConfiguration *conf = ms->firmware_build_state.acpi.conf;
> +    AcpiBuildTables tables;
> +
> +    /* No ACPI configuration? Nothing to do. */
> +    if (!conf) {
> +        return;
> +    }
> +
> +    /* 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);
> +
> +    acpi_reduced_build(ms, &tables, conf);
> +
> +    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);
> +}
> +

ms is not needed here; just pass the FirmwareBuildState as the opaque
value in rom_add_blob.

In fact, here:

> +    AcpiBuildState *build_state;
> +
> +    build_state = g_malloc0(sizeof(*build_state));
> +    machine->firmware_build_state.acpi.state = build_state;
> +    machine->firmware_build_state.acpi.conf = conf;
> +

I would say that you don't need FirmwareBuildState at all.  I cannot be
100% sure because I cannot see the caller of acpi_reduced_setup, but I
think you can add an AcpiConfiguration* field to AcpiBuildState and
encapsulate everything in AcpiBuildState.  In addition, the
AcpiBuildState need not be stored in the MachineState.

Instead, FirmwareBuildMethods should be a QOM interface (also please
refer explicitly to ACPI in the names, don't call it "firmware").  The
setup method of FirmwareBuildMethods can take that QOM interface, not
the MachineState, i.e. its prototype should be

void (*setup)(AcpiBuildMethods *acpibuild,
              AcpiConfiguration *conf);

so that pc_machine_done does

    if (pcms->acpi_build_enabled) {
        acpi_conf_pc_init(pcms);
        /* This calls the ->setup method.  */
        acpi_builder_setup(ACPI_BUILD_METHODS(pcms),
                           &pcms->acpi_configuration);
    }

This is because MachineClass is used for dozens of machines that have
nothing to with ACPI.  Instead, machines that use ACPI (either reduced
or normal) can define the AcpiBuildMethods interface, and invoke the
entry point firmware_build_methods->setup (either acpi_setup or
acpi_reduced_setup) through acpi_builder_setup.

Paolo



reply via email to

[Prev in Thread] Current Thread [Next in Thread]