Re: [Qemu-devel] [PATCH v4 3/5] acpi: pc: add fw_cfg device node to ssdt

[Gabriel L. Somlo]

On Thu, Oct 01, 2015 at 01:33:50PM +0200, Igor Mammedov wrote:
> On Thu, 1 Oct 2015 10:27:15 +0200
> Laszlo Ersek <address@hidden> wrote:
> 
> > On 10/01/15 09:02, Igor Mammedov wrote:
> > > On Sun, 27 Sep 2015 17:29:00 -0400
> > > "Gabriel L. Somlo" <address@hidden> wrote:
> > > 
> > >> Add a fw_cfg device node to the ACPI SSDT, on machine types
> > >> pc-*-2.5 and up. While the guest-side BIOS can't utilize
> > >> this information (since it has to access the hard-coded
> > >> fw_cfg device to extract ACPI tables to begin with), having
> > >> fw_cfg listed in ACPI will help the guest kernel keep a more
> > >> accurate inventory of in-use IO port regions.
> > >>
> > >> Signed-off-by: Gabriel Somlo <address@hidden>
> > >> ---
> > >>  hw/i386/acpi-build.c | 23 +++++++++++++++++++++++
> > >>  hw/i386/pc_piix.c    |  1 +
> > >>  hw/i386/pc_q35.c     |  1 +
> > >>  include/hw/i386/pc.h |  1 +
> > >>  4 files changed, 26 insertions(+)
> > >>
> > >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > >> index 95e0c65..ece2710 100644
> > >> --- a/hw/i386/acpi-build.c
> > >> +++ b/hw/i386/acpi-build.c
> > >> @@ -906,6 +906,7 @@ build_ssdt(GArray *table_data, GArray *linker,
> > >>             PcPciInfo *pci, PcGuestInfo *guest_info)
> > >>  {
> > >>      MachineState *machine = MACHINE(qdev_get_machine());
> > >> +    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine);
> > >>      uint32_t nr_mem = machine->ram_slots;
> > >>      unsigned acpi_cpus = guest_info->apic_id_limit;
> > >>      Aml *ssdt, *sb_scope, *scope, *pkg, *dev, *method, *crs, *field, 
> > >> *ifctx;
> > >> @@ -1071,6 +1072,28 @@ build_ssdt(GArray *table_data, GArray *linker,
> > >>      aml_append(scope, aml_name_decl("_S5", pkg));
> > >>      aml_append(ssdt, scope);
> > >>  
> > >> +    if (!pcmc->acpi_no_fw_cfg_node) {
> > > we don't really care about SSDT size changes since during
> > > migration ACPI blobs will be migrated from source, so
> > > machine compat bloat is excessive here. It would be better
> > > to just remove it.
> > 
> > This was Eduardo's suggestion, if I recall correctly:
> > 
> > http://thread.gmane.org/gmane.comp.emulators.qemu/361930/focus=361983
> > 
> > The idea being, if you move a guest from older QEMU to newer QEMU, but
> > keep the machine type (or more precisely, the full machine config, like
> > the domain XML) intact, the ACPI device node should not appear out of
> > the blue.
> This ACPI device is an always used resource declaration regardless
> of machine type so it makes sense to tell guest about used resource.
> 
> The only reason for machine compat code would be if guest suddenly
> started to ask for a driver but as Gabriel showed with _STA(0xB)
> it doesn't, so I'm trying to keep ACPI code machine compat agnostic
> as much as possible.

Eduardo, what do you think about this ? I'm hoping to do a v5 over the
weekend or early next week, and which way this should go is one of the
couple of decisions that I still have open.

Thanks much,
--Gabriel