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

[Gal Hammer]

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 <address@hidden>
> > > > ---
> > > >    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.