| Message ID | 1384273995-16486-15-git-send-email-cminyard@mvista.com |
|---|---|
| State | New |
| Headers | show |
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 <cminyard@mvsita.com> 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
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 <cminyard@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. 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
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 <cminyard@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
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 <cminyard@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
On Thu, Nov 14, 2013 at 07:40:31AM -0600, Corey Minyard wrote: > 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 <cminyard@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. OK so for review, it might be a good idea to split out the changes required for necessary functionality as opposed to groundwork that will be used down the line. > > 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 I see. These tables are generated in hw/i386/acpi-build.c This is invoked in machine done callback so there shouldn't in an issue to probe for IPMI and it's configuration using QOM. > > > >> 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
On Thu, Nov 14, 2013 at 07:40:31AM -0600, Corey Minyard wrote: > 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 <cminyard@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 OK need some help with that? Main options are - write code in ASL, supply entry in ACPI always, patch some fields to enable/disable it dynamically see how pvpanic entry is disabled for an example - write code in ASL append it to ACPI if necessary see how CPU entries are added depending on number of CPUs for an example - generate code in AML directly see acpi based pci hotplug on pci branch in my tree for an example > > > >> 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
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; }
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 <cminyard@mvsita.com> --- hw/i386/pc.c | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-)