On 11/14/2013 07:38 AM, Michael S. Tsirkin wrote:
> On Thu, Nov 14, 2013 at 07:28:00AM -0600, Corey Minyard wrote:
>> 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 <cminy...@mvsita.com>
>>> 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.
> Where? Which patch does this? It would be a strange thing to do
> because FW_CFG_ACPI_TABLES is normally intended for user-supplied tables
> (though q35 misused it for other purposes).
I don't have that patch out yet. I had written some code to dynamically
generate ACPI tables, but that was rejected and from what I understand
something was done recently so these tables can be dynamically generated
some other way. I haven't had time to look into this, but I wanted to
get the main patch set out first.
I know that these tables are generally fixed, but the entry for IPMI
should only be present if the device is specified, and its contents
depend on the configuration supplied,
-corey
>
>> 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
