Re: [Qemu-devel] [PATCH 2/5] pc: Postpone SMBIOS table installation to post machine init

[Michael S. Tsirkin]

On Mon, Mar 14, 2016 at 03:16:46PM +0100, Paolo Bonzini wrote:
> 
> 
> On 14/03/2016 14:30, Corey Minyard wrote:
> > On 03/13/2016 09:03 PM, Michael S. Tsirkin wrote:
> >> On Wed, Feb 24, 2016 at 12:59:13PM -0600, miny...@acm.org wrote:
> >>> From: Corey Minyard 
> >>>
> >>> This is the same place that the ACPI SSDT table gets added, so that
> >>> devices can add themselves to the SMBIOS table.
> >>>
> >>> Signed-off-by: Corey Minyard 
> >> This changes the order of fw cfg files, which
> >> is guest visible.
> >> Need to make it depend on either machine type 
> 
> Didn't Gerd have a patch to sort fw_cfg files?
> 
> ... yes, here it is:
> https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg05238.html
> 
> Paolo

Yes but the messy part is the compatibility.
Specifically we need to find out the current list
of fw cfg files, and their order, and use that
existing order with old machine types.

-- 
MST

Re: [Qemu-devel] [PATCH 2/5] pc: Postpone SMBIOS table installation to post machine init

[Gerd Hoffmann]

  Hi,

> > Didn't Gerd have a patch to sort fw_cfg files?
> >
> > ... yes, here it is:
> > https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg05238.html
> 
> I could take that patch and modify it to add a new machine type...

Sure, go ahead.  Didn't submit the patch because I wasn't sure it is
worth it, especially the compat stuff, do it on new machine types only
etc.

But if we now have to change the order _anyway_ for new machine types it
is a good opportunity to move to a sorted order at the same time, so we
will not have that problem (init order changes reorders fw_cfg file
directory) again.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 2/5] pc: Postpone SMBIOS table installation to post machine init

[Corey Minyard]

On 03/14/2016 09:16 PM, Paolo Bonzini wrote:


On 14/03/2016 14:30, Corey Minyard wrote:

On 03/13/2016 09:03 PM, Michael S. Tsirkin wrote:

On Wed, Feb 24, 2016 at 12:59:13PM -0600, miny...@acm.org wrote:

From: Corey Minyard 

This is the same place that the ACPI SSDT table gets added, so that
devices can add themselves to the SMBIOS table.

Signed-off-by: Corey Minyard 

This changes the order of fw cfg files, which
is guest visible.
Need to make it depend on either machine type

Didn't Gerd have a patch to sort fw_cfg files?

... yes, here it is:
https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg05238.html


I could take that patch and modify it to add a new machine type...

-corey


Paolo

Re: [Qemu-devel] [PATCH 2/5] pc: Postpone SMBIOS table installation to post machine init

[Paolo Bonzini]

On 14/03/2016 14:30, Corey Minyard wrote:
> On 03/13/2016 09:03 PM, Michael S. Tsirkin wrote:
>> On Wed, Feb 24, 2016 at 12:59:13PM -0600, miny...@acm.org wrote:
>>> From: Corey Minyard 
>>>
>>> This is the same place that the ACPI SSDT table gets added, so that
>>> devices can add themselves to the SMBIOS table.
>>>
>>> Signed-off-by: Corey Minyard 
>> This changes the order of fw cfg files, which
>> is guest visible.
>> Need to make it depend on either machine type 

Didn't Gerd have a patch to sort fw_cfg files?

... yes, here it is:
https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg05238.html

Paolo

Re: [Qemu-devel] [PATCH 2/5] pc: Postpone SMBIOS table installation to post machine init

[Corey Minyard]

On 03/13/2016 09:03 PM, Michael S. Tsirkin wrote:

On Wed, Feb 24, 2016 at 12:59:13PM -0600, miny...@acm.org wrote:

From: Corey Minyard 

This is the same place that the ACPI SSDT table gets added, so that
devices can add themselves to the SMBIOS table.

Signed-off-by: Corey Minyard 

This changes the order of fw cfg files, which
is guest visible.
Need to make it depend on either machine type


Do you mean add a new machine type, like adding a
2.7 version of pc-i440fx and pc-q35?


or the presence of ipmi somehow.


You could pre-scan for ipmi options from early in the
pc code.  Kind of hackish.

One other option would be to just have ACPI support for
IPMI.  Older OSes might not work; I'm not sure how
important that is.

Does anyone have a preference here?  To me, adding a
new machine type looks easiest, but I'm not sure of the
effects on users.

-corey


---
  hw/i386/pc.c | 11 ++-
  1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0aeefd2..da8fc76 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -778,8 +778,6 @@ static FWCfgState *bochs_bios_init(AddressSpace *as)
   acpi_tables, acpi_tables_len);
  fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, kvm_allows_irq0_override());
  
-pc_build_smbios(fw_cfg);

-
  fw_cfg_add_bytes(fw_cfg, FW_CFG_E820_TABLE,
   _reserve, sizeof(e820_reserve));
  fw_cfg_add_file(fw_cfg, "etc/e820", e820_table,
@@ -1161,6 +1159,7 @@ void pc_machine_done(Notifier *notifier, void *data)
  {
  PCMachineState *pcms = container_of(notifier,
  PCMachineState, machine_done);
+FWCfgState *fw_cfg = pcms->fw_cfg;
  PCIBus *bus = pcms->bus;
  
  if (bus) {

@@ -1172,15 +1171,17 @@ void pc_machine_done(Notifier *notifier, void *data)
  extra_hosts++;
  }
  }
-if (extra_hosts && pcms->fw_cfg) {
+if (extra_hosts && fw_cfg) {
  uint64_t *val = g_malloc(sizeof(*val));
  *val = cpu_to_le64(extra_hosts);
-fw_cfg_add_file(pcms->fw_cfg,
-"etc/extra-pci-roots", val, sizeof(*val));
+fw_cfg_add_file(fw_cfg, "etc/extra-pci-roots", val, sizeof(*val));
  }
  }
  
  acpi_setup();

+if (fw_cfg) {
+pc_build_smbios(fw_cfg);
+}
  }
  
  void pc_guest_info_init(PCMachineState *pcms)

--
2.5.0

Re: [Qemu-devel] [PATCH 2/5] pc: Postpone SMBIOS table installation to post machine init

[Michael S. Tsirkin]

On Wed, Feb 24, 2016 at 12:59:13PM -0600, miny...@acm.org wrote:
> From: Corey Minyard 
> 
> This is the same place that the ACPI SSDT table gets added, so that
> devices can add themselves to the SMBIOS table.
> 
> Signed-off-by: Corey Minyard 

This changes the order of fw cfg files, which
is guest visible.
Need to make it depend on either machine type
or the presence of ipmi somehow.

> ---
>  hw/i386/pc.c | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 0aeefd2..da8fc76 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -778,8 +778,6 @@ static FWCfgState *bochs_bios_init(AddressSpace *as)
>   acpi_tables, acpi_tables_len);
>  fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, kvm_allows_irq0_override());
>  
> -pc_build_smbios(fw_cfg);
> -
>  fw_cfg_add_bytes(fw_cfg, FW_CFG_E820_TABLE,
>   _reserve, sizeof(e820_reserve));
>  fw_cfg_add_file(fw_cfg, "etc/e820", e820_table,
> @@ -1161,6 +1159,7 @@ void pc_machine_done(Notifier *notifier, void *data)
>  {
>  PCMachineState *pcms = container_of(notifier,
>  PCMachineState, machine_done);
> +FWCfgState *fw_cfg = pcms->fw_cfg;
>  PCIBus *bus = pcms->bus;
>  
>  if (bus) {
> @@ -1172,15 +1171,17 @@ void pc_machine_done(Notifier *notifier, void *data)
>  extra_hosts++;
>  }
>  }
> -if (extra_hosts && pcms->fw_cfg) {
> +if (extra_hosts && fw_cfg) {
>  uint64_t *val = g_malloc(sizeof(*val));
>  *val = cpu_to_le64(extra_hosts);
> -fw_cfg_add_file(pcms->fw_cfg,
> -"etc/extra-pci-roots", val, sizeof(*val));
> +fw_cfg_add_file(fw_cfg, "etc/extra-pci-roots", val, 
> sizeof(*val));
>  }
>  }
>  
>  acpi_setup();
> +if (fw_cfg) {
> +pc_build_smbios(fw_cfg);
> +}
>  }
>  
>  void pc_guest_info_init(PCMachineState *pcms)
> -- 
> 2.5.0

[Qemu-devel] [PATCH 2/5] pc: Postpone SMBIOS table installation to post machine init

[minyard]

From: Corey Minyard 

This is the same place that the ACPI SSDT table gets added, so that
devices can add themselves to the SMBIOS table.

Signed-off-by: Corey Minyard 
---
 hw/i386/pc.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0aeefd2..da8fc76 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -778,8 +778,6 @@ static FWCfgState *bochs_bios_init(AddressSpace *as)
  acpi_tables, acpi_tables_len);
 fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, kvm_allows_irq0_override());
 
-pc_build_smbios(fw_cfg);
-
 fw_cfg_add_bytes(fw_cfg, FW_CFG_E820_TABLE,
  _reserve, sizeof(e820_reserve));
 fw_cfg_add_file(fw_cfg, "etc/e820", e820_table,
@@ -1161,6 +1159,7 @@ void pc_machine_done(Notifier *notifier, void *data)
 {
 PCMachineState *pcms = container_of(notifier,
 PCMachineState, machine_done);
+FWCfgState *fw_cfg = pcms->fw_cfg;
 PCIBus *bus = pcms->bus;
 
 if (bus) {
@@ -1172,15 +1171,17 @@ void pc_machine_done(Notifier *notifier, void *data)
 extra_hosts++;
 }
 }
-if (extra_hosts && pcms->fw_cfg) {
+if (extra_hosts && fw_cfg) {
 uint64_t *val = g_malloc(sizeof(*val));
 *val = cpu_to_le64(extra_hosts);
-fw_cfg_add_file(pcms->fw_cfg,
-"etc/extra-pci-roots", val, sizeof(*val));
+fw_cfg_add_file(fw_cfg, "etc/extra-pci-roots", val, sizeof(*val));
 }
 }
 
 acpi_setup();
+if (fw_cfg) {
+pc_build_smbios(fw_cfg);
+}
 }
 
 void pc_guest_info_init(PCMachineState *pcms)
-- 
2.5.0