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

[Eduardo Habkost]

On Sat, Oct 10, 2015 at 12:00:16AM -0400, Gabriel L. Somlo wrote:
> 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.

What about non-live migration?

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

The general rule is that anything that's visible to the guest shouldn't
change on a QEMU upgrade if the machine-type is kept the same. If we
want to avoid the compat code, we need careful testing to ensure this
won't make any guest OS do something unexpected.

One of the things that may break if guest-visible bits of the machine
change is Windows license activation, but the rules Windows use to
trigger reactivation aren't very clear.

-- 
Eduardo