Re: [Qemu-devel] [PATCH v2 09/14] pc: Remove compat fields from PcGuestInfo

[Marcel Apfelbaum <marcel@redhat.com>]

On 12/11/2015 08:42 PM, Eduardo Habkost wrote:
> Remove the fields: legacy_acpi_table_size, has_acpi_build,
> has_reserved_memory, and rsdp_in_ram from PcGuestInfo, and let
> the existing code use the PCMachineClass fields directly.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>   hw/i386/acpi-build.c | 10 ++++++----
>   hw/i386/pc.c         |  6 +++---
>   hw/i386/pc_piix.c    |  5 -----
>   hw/i386/pc_q35.c     |  8 --------
>   include/hw/i386/pc.h |  4 ----
>   5 files changed, 9 insertions(+), 24 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 82d55bb..46b83ad 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1689,6 +1689,7 @@ static
>   void acpi_build(AcpiBuildTables *tables)
>   {
>       PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
> +    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>       PcGuestInfo *guest_info = &pcms->acpi_guest_info;
>       GArray *table_offsets;
>       unsigned facs, ssdt, dsdt, rsdt;
> @@ -1802,12 +1803,12 @@ void acpi_build(AcpiBuildTables *tables)
>        *
>        * All this is for PIIX4, since QEMU 2.0 didn't support Q35 migration.
>        */
> -    if (guest_info->legacy_acpi_table_size) {
> +    if (pcmc->legacy_acpi_table_size) {
>           /* Subtracting aml_len gives the size of fixed tables.  Then add the
>            * size of the PIIX4 DSDT/SSDT in QEMU 2.0.
>            */
>           int legacy_aml_len =
> -            guest_info->legacy_acpi_table_size +
> +            pcmc->legacy_acpi_table_size +
>               ACPI_BUILD_LEGACY_CPU_AML_SIZE * max_cpus;
>           int legacy_table_size =
>               ROUND_UP(tables_blob->len - aml_len + legacy_aml_len,
> @@ -1900,6 +1901,7 @@ static const VMStateDescription vmstate_acpi_build = {
>   void acpi_setup(void)
>   {
>       PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
> +    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>       PcGuestInfo *guest_info = &pcms->acpi_guest_info;
>       AcpiBuildTables tables;
>       AcpiBuildState *build_state;
> @@ -1909,7 +1911,7 @@ void acpi_setup(void)
>           return;
>       }
>
> -    if (!guest_info->has_acpi_build) {
> +    if (!pcmc->has_acpi_build) {
>           ACPI_BUILD_DPRINTF("ACPI build disabled. Bailing out.\n");
>           return;
>       }
> @@ -1938,7 +1940,7 @@ void acpi_setup(void)
>       fw_cfg_add_file(guest_info->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
>                       tables.tcpalog->data, acpi_data_len(tables.tcpalog));
>
> -    if (!guest_info->rsdp_in_ram) {
> +    if (!pcmc->rsdp_in_ram) {
>           /*
>            * Keep for compatibility with old machine types.
>            * Though RSDP is small, its contents isn't immutable, so
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 14c0116..f8d4531 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1316,7 +1316,7 @@ void pc_memory_init(PCMachineState *pcms,
>           e820_add_entry(0x100000000ULL, pcms->above_4g_mem_size, E820_RAM);
>       }
>
> -    if (!guest_info->has_reserved_memory &&
> +    if (!pcmc->has_reserved_memory &&
>           (machine->ram_slots ||
>            (machine->maxram_size > machine->ram_size))) {
>           MachineClass *mc = MACHINE_GET_CLASS(machine);
> @@ -1327,7 +1327,7 @@ void pc_memory_init(PCMachineState *pcms,
>       }
>
>       /* initialize hotplug memory address space */
> -    if (guest_info->has_reserved_memory &&
> +    if (pcmc->has_reserved_memory &&
>           (machine->ram_size < machine->maxram_size)) {
>           ram_addr_t hotplug_mem_size =
>               machine->maxram_size - machine->ram_size;
> @@ -1382,7 +1382,7 @@ void pc_memory_init(PCMachineState *pcms,
>
>       rom_set_fw(fw_cfg);
>
> -    if (guest_info->has_reserved_memory && pcms->hotplug_memory.base) {
> +    if (pcmc->has_reserved_memory && pcms->hotplug_memory.base) {
>           uint64_t *val = g_malloc(sizeof(*val));
>           PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>           uint64_t res_mem_end = pcms->hotplug_memory.base;
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index f39c086..fe00086 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -142,12 +142,7 @@ static void pc_init1(MachineState *machine,
>
>       guest_info = pc_guest_info_init(pcms);
>
> -    guest_info->has_acpi_build = pcmc->has_acpi_build;
> -    guest_info->legacy_acpi_table_size = pcmc->legacy_acpi_table_size;
> -
>       guest_info->isapc_ram_fw = !pcmc->pci_enabled;
> -    guest_info->has_reserved_memory = pcmc->has_reserved_memory;
> -    guest_info->rsdp_in_ram = pcmc->rsdp_in_ram;
>
>       if (pcmc->smbios_defaults) {
>           MachineClass *mc = MACHINE_GET_CLASS(machine);
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index bb1436e..1f29943 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -133,14 +133,6 @@ static void pc_q35_init(MachineState *machine)
>
>       guest_info = pc_guest_info_init(pcms);
>       guest_info->isapc_ram_fw = false;
> -    guest_info->has_acpi_build = pcmc->has_acpi_build;
> -    guest_info->has_reserved_memory = pcmc->has_reserved_memory;
> -    guest_info->rsdp_in_ram = pcmc->rsdp_in_ram;
> -
> -    /* Migration was not supported in 2.0 for Q35, so do not bother
> -     * with this hack (see hw/i386/acpi-build.c).
> -     */
> -    guest_info->legacy_acpi_table_size = 0;
>
>       if (pcmc->smbios_defaults) {
>           /* These values are guest ABI, do not change */
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 24362ef..d24682f 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -30,10 +30,6 @@ struct PcGuestInfo {
>       uint64_t *node_mem;
>       uint64_t *node_cpu;
>       FWCfgState *fw_cfg;
> -    int legacy_acpi_table_size;
> -    bool has_acpi_build;
> -    bool has_reserved_memory;
> -    bool rsdp_in_ram;
>   };
>
>   /**
>

Looks OK to me

Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>