Re: [Qemu-devel] [PATCH v2 11/14] pc: Remove PcGuestInfo.isapc_ram_fw field

[Marcel Apfelbaum <marcel@redhat.com>]

On 12/11/2015 08:42 PM, Eduardo Habkost wrote:
> The code can use the PCMachineClass.pci_enabled field directly.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>   hw/i386/pc.c         | 2 +-
>   hw/i386/pc_piix.c    | 5 +----
>   hw/i386/pc_q35.c     | 4 +---
>   include/hw/i386/pc.h | 1 -
>   4 files changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 998fe4e..02d0e19 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1365,7 +1365,7 @@ void pc_memory_init(PCMachineState *pcms,
>       }
>
>       /* Initialize PC system firmware */
> -    pc_system_firmware_init(rom_memory, guest_info->isapc_ram_fw);
> +    pc_system_firmware_init(rom_memory, !pcmc->pci_enabled);
>
>       option_rom_mr = g_malloc(sizeof(*option_rom_mr));
>       memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE,
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index fe00086..f0c2dc8 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -83,7 +83,6 @@ static void pc_init1(MachineState *machine,
>       MemoryRegion *ram_memory;
>       MemoryRegion *pci_memory;
>       MemoryRegion *rom_memory;
> -    PcGuestInfo *guest_info;
>       ram_addr_t lowmem;
>
>       /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory).
> @@ -140,9 +139,7 @@ static void pc_init1(MachineState *machine,
>           rom_memory = system_memory;
>       }
>
> -    guest_info = pc_guest_info_init(pcms);
> -
> -    guest_info->isapc_ram_fw = !pcmc->pci_enabled;
> +    pc_guest_info_init(pcms);
>
>       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 1f29943..0907746 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -70,7 +70,6 @@ static void pc_q35_init(MachineState *machine)
>       int i;
>       ICH9LPCState *ich9_lpc;
>       PCIDevice *ahci;
> -    PcGuestInfo *guest_info;
>       ram_addr_t lowmem;
>       DriveInfo *hd[MAX_SATA_PORTS];
>       MachineClass *mc = MACHINE_GET_CLASS(machine);
> @@ -131,8 +130,7 @@ static void pc_q35_init(MachineState *machine)
>           rom_memory = get_system_memory();
>       }
>
> -    guest_info = pc_guest_info_init(pcms);
> -    guest_info->isapc_ram_fw = false;

This may not be an issue, I just want be sure.
For Q35 isapc_ram_fw is always false, but now we are always querying
!pcmc->pci_enabled.

Now we have a Q35 case when !pcmc->pci_enabled *can* be true.

Do we need to care?

Thanks,
Marcel

> +    pc_guest_info_init(pcms);
>
>       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 30c7a5b..20a425c 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -22,7 +22,6 @@
>
>   /* Machine info for ACPI build: */
>   struct PcGuestInfo {
> -    bool isapc_ram_fw;
>       unsigned apic_id_limit;
>       bool apic_xrupt_override;
>       uint64_t numa_nodes;
>