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

[Laszlo Ersek]

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.

I'll let Gabriel answer your other question below. :)

Thanks
Laszlo


> 
> 
>> +        scope = aml_scope("\\_SB");
>> +        dev = aml_device("FWCF");
>> +
>> +        aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
>> +        /* device present, functioning, decoding, not shown in UI */
>> +        aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
>> +
>> +        crs = aml_resource_template();
>> +        /* when using port i/o, the 8-bit data register *always* overlaps
>> +         * with half of the 16-bit control register. Hence, the total size
>> +         * of the i/o region used is FW_CFG_CTL_SIZE */
>> +        aml_append(crs,
>> +            aml_io(AML_DECODE16, FW_CFG_IO_BASE, FW_CFG_IO_BASE,
>> +                   0x01, FW_CFG_CTL_SIZE)
>> +        );
> could you check/dump this _CRS and PCI0._CRS to see if they are intersecting
> in any way?
> 
> 
>> +        aml_append(dev, aml_name_decl("_CRS", crs));
>> +
>> +        aml_append(scope, dev);
>> +        aml_append(ssdt, scope);
>> +    }
>> +
>>      if (misc->applesmc_io_base) {
>>          scope = aml_scope("\\_SB.PCI0.ISA");
>>          dev = aml_device("SMC");
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index 3ffb05f..7f5e5d9 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -482,6 +482,7 @@ static void pc_i440fx_2_4_machine_options(MachineClass 
>> *m)
>>      m->alias = NULL;
>>      m->is_default = 0;
>>      pcmc->broken_reserved_end = true;
>> +    pcmc->acpi_no_fw_cfg_node = true;
>>      SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
>>  }
>>  
>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>> index 1b7d3b6..7180ca3 100644
>> --- a/hw/i386/pc_q35.c
>> +++ b/hw/i386/pc_q35.c
>> @@ -385,6 +385,7 @@ static void pc_q35_2_4_machine_options(MachineClass *m)
>>      pc_q35_2_5_machine_options(m);
>>      m->alias = NULL;
>>      pcmc->broken_reserved_end = true;
>> +    pcmc->acpi_no_fw_cfg_node = true;
>>      SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
>>  }
>>  
>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>> index 86007e3..6d0f1bd 100644
>> --- a/include/hw/i386/pc.h
>> +++ b/include/hw/i386/pc.h
>> @@ -62,6 +62,7 @@ struct PCMachineClass {
>>      bool broken_reserved_end;
>>      HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
>>                                             DeviceState *dev);
>> +    bool acpi_no_fw_cfg_node;
>>  };
>>  
>>  #define TYPE_PC_MACHINE "generic-pc-machine"
>