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

[Gal Hammer <ghammer@redhat.com>]

On 17/08/2014 12:49, Paolo Bonzini wrote:
> 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.

Are you sure about selecting the DSDT? I don't see anyone else is using 
the ACPI_EXTRACT_NAME_* macros in the DSDT table (and I keep crashing my 
guest, but ignore it for now ;-)).

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

The VGID is expected to change when executing a VM with the -snapshot 
option, when a VM is restored from a backup or when it is imported, 
copied or cloned. So I would say it is a management's call.

I think that the Microsoft's document describes the requirements better 
than me :-).

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

The -uuid is not expected to change after the VM was created. Unlike the 
-vmgenid that is designed to give the guest OS a notification that a 
change has occurred. Microsoft, as an example, writes that is can be use 
for a safer cryptographic software.

> Paolo
>

     Gal.