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

[Igor Mammedov]

On Tue, 13 Oct 2015 16:10:03 -0300
Eduardo Habkost <address@hidden> wrote:

> 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?
I don't see any issues here, it should just work, since usually
original SSDT from source is used (copied as part of migrated ram).

> > > > 
> > > > 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.
Practice shows that changing ACPI tables doesn't affect MS Activation so far.