Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.

[Paolo Bonzini <pbonzini@redhat.com>]

Il 12/08/2014 10:02, Gal Hammer ha scritto:
> Hi,
> 
> On 10/08/2014 20:22, Paolo Bonzini wrote:
> 
>> Il 10/08/2014 13:32, Gal Hammer ha scritto:
>>> Based on Microsoft's sepecifications (paper can be dowloaded from
>>> http://go.microsoft.com/fwlink/?LinkId=260709), add a device
>>> description to the SSDT ACPI table.
>>>
>>> The GUID is set using a new "-vmgenid" command line parameter.
>>>
>>> Signed-off-by: Gal Hammer <ghammer@redhat.com>
>>> ---
>>>   hw/i386/acpi-build.c  | 23 +++++++++++++++++++++++
>>>   hw/i386/ssdt-misc.dsl | 33 +++++++++++++++++++++++++++++++++
>>>   qemu-options.hx       |  9 +++++++++
>>>   vl.c                  | 11 +++++++++++
>>>   4 files changed, 76 insertions(+)
>>
>> Please make this a new device (like pvpanic), instead of adding a new
>> command-line option.
> 
> There is a problem with this request. I don't want to use ISA because it
> is obsolete, PCI is overkill for such a device and a SYSBUS (like HPET)
> device doesn't effect the command line options.
> 
> Did I miss something in SYSBUS and that's was the reason it didn't
> appear in the "-device ?" list?

For a sysbus device, you can override the
cannot_instantiate_with_device_add_yet field of DeviceClass in your
class_init function.

>>>
>>> +    Scope(\_SB) {
>>> +
>>> +        Device(VMGI) {
>>> +            Name(_HID, "QEMU0002")
>>> +            Name(_CID, "VM_Gen_Counter")
>>> +            Name(_DDN, "VM_Gen_Counter")
>>> +
>>> +            ACPI_EXTRACT_NAME_DWORD_CONST ssdt_acpi_vm_gid_addr
>>> +            Name(VGIA, 0x12345678)
>>> +
>>> +            ACPI_EXTRACT_NAME_BUFFER16 ssdt_acpi_vm_gid
>>> +            Name(VGID, Buffer(16) {
>>> +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>>> +                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 })
>>> +
>>> +            Method(_STA, 0, NotSerialized) {
>>> +                Store(VGIA, Local0)
>>> +                If (LEqual(Local0, Zero)) {
>>> +                    Return (0x00)
>>> +                } Else {
>>> +                    Return (0x0F)
>>> +                }
>>> +            }
>>> +
>>> +            Method(ADDR, 0, Serialized) {
>>> +                Store(Package(2) { }, Local0)
>>> +                Store(VGIA, Index(Local0, 0))
>>> +                Store(0x0000, Index(Local0, 1))
>>> +                return (Local0)
>>> +            }
>>> +        }
>>> +    }
>>> +
>>
>> Please either put this in the DSDT, or omit the Device altogether if you
>> put it in the SSDT and there is no VMGID device.
> 
> I'm not sure I understand this comment. I've put the new device in the
> SSDT table (like pvpanic) and add a _STA method which disable the device
> if no GUID's address is set (VGIA). The device doesn't show in the
> Device Manager if it was not added using the command line.

We are still in the process of defining which devices/methods go in the
DSDT and which go in the SSDT.  We had bad experiences with ACPI table
migration in 2.1, and one plan to fix them is the following:

* the DSDT should always be the same size no matter what command line
options are there

* the SSDT should have the exact same content (byte-for-byte) for
different versions of QEMU, with the same command line options
(including the machine type).

Right now your code obeys the first rule, not the second rule, so it
should add the device to the DSDT.

BTW, which events would cause the ID to change?  How should live cloning
(or revert to a disk+RAM snapshot) be implemented by layers above QEMU
for the VM gen ID to be patched?  Can you add something to docs/ about it?

Also, how does this ID compare to the UUID in the DMI info (-uuid)?

Paolo