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