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

[Corey Minyard]

On 11/14/2013 01:30 AM, Michael S. Tsirkin wrote:
> 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.

Because the IPMI device has to dynamically add an entry based on its
configuration.  As it is the tables get added to the BIOS access before
devices initialize.

-corey

>
>> ---
>>  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