Re: [Qemu-devel] [PATCH 14/16] pc: Postpone adding ACPI and SMBIOS to fw_cfg

[Michael S. Tsirkin]

On Tue, Nov 12, 2013 at 10:33:13AM -0600, Corey Minyard wrote:
> Postpone the addition of the ACPI and SMBIOS tables until after
> device initialization.  This allows devices to add entries to these
> tables.
> 
> Signed-off-by: Corey Minyard <address@hidden>

Why delay adding FW_CFG_ACPI_TABLES?
These are normally specified by user so they are constant.

> ---
>  hw/i386/pc.c | 38 ++++++++++++++++++++++++++++++--------
>  1 file changed, 30 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index dee409d..765c95e 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -607,8 +607,6 @@ static unsigned int pc_apic_id_limit(unsigned int 
> max_cpus)
>  static FWCfgState *bochs_bios_init(void)
>  {
>      FWCfgState *fw_cfg;
> -    uint8_t *smbios_table;
> -    size_t smbios_len;
>      uint64_t *numa_fw_cfg;
>      int i, j;
>      unsigned int apic_id_limit = pc_apic_id_limit(max_cpus);
> @@ -631,14 +629,8 @@ static FWCfgState *bochs_bios_init(void)
>      fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)apic_id_limit);
>      fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
>      fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> -    fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES,
> -                     acpi_tables, acpi_tables_len);
>      fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, kvm_allows_irq0_override());
>  
> -    smbios_table = smbios_get_table(&smbios_len);
> -    if (smbios_table)
> -        fw_cfg_add_bytes(fw_cfg, FW_CFG_SMBIOS_ENTRIES,
> -                         smbios_table, smbios_len);
>      fw_cfg_add_bytes(fw_cfg, FW_CFG_E820_TABLE,
>                       &e820_table, sizeof(e820_table));
>  
> @@ -1127,6 +1119,31 @@ void pc_acpi_init(const char *default_dsdt)
>      }
>  }
>  
> +struct pc_bios_post_init {
> +    Notifier post_init;
> +    void *fw_cfg;
> +};
> +
> +/* Add the ACPI and SMBIOS tables after all the hardware has been 
> initialized.
> + * This gives devices a chance to add to those tables.
> + */
> +static void pc_bios_post_initfn(Notifier *n, void *opaque)
> +{
> +    struct pc_bios_post_init *p = container_of(n, struct pc_bios_post_init,
> +                                               post_init);
> +    uint8_t *smbios_table;
> +    size_t smbios_len;
> +
> +    fw_cfg_add_bytes(p->fw_cfg, FW_CFG_ACPI_TABLES,
> +                     acpi_tables, acpi_tables_len);
> +    smbios_table = smbios_get_table(&smbios_len);
> +    if (smbios_table)
> +        fw_cfg_add_bytes(p->fw_cfg, FW_CFG_SMBIOS_ENTRIES,
> +                         smbios_table, smbios_len);
> +}
> +
> +static struct pc_bios_post_init post_init;
> +
>  FWCfgState *pc_memory_init(MemoryRegion *system_memory,
>                             const char *kernel_filename,
>                             const char *kernel_cmdline,
> @@ -1196,6 +1213,11 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,
>          rom_add_option(option_rom[i].name, option_rom[i].bootindex);
>      }
>      guest_info->fw_cfg = fw_cfg;
> +
> +    post_init.fw_cfg = fw_cfg;
> +    post_init.post_init.notify = pc_bios_post_initfn;
> +    qemu_add_machine_init_done_notifier(&post_init.post_init);
> +
>      return fw_cfg;
>  }
>  
> -- 
> 1.8.3.1