Re: [Qemu-devel] [PATCH v5 05/10] ACPI: Add Virtual Machine Generation ID support

[Michael S. Tsirkin]

On Mon, Feb 06, 2017 at 09:59:55AM -0800, Ben Warren wrote:
>                +void vmgenid_add_fw_cfg(FWCfgState *s, GArray *guid)
>                +{
>                +    Object *obj = find_vmgenid_dev(NULL);
>                +    assert(obj);
>                +    VmGenIdState *vms = VMGENID(obj);
>                +
>                +    /* Create a read-only fw_cfg file for GUID */
>                +    fw_cfg_add_file(s, VMGENID_GUID_FW_CFG_FILE, guid->data,
>                +                    VMGENID_FW_CFG_SIZE);
>                +    /* Create a read-write fw_cfg file for Address */
>                +    fw_cfg_add_file_callback(s, VMGENID_ADDR_FW_CFG_FILE, 
> NULL,
>         NULL,
> 
> 
>            Seems wrong. What if guest updates the address after command line
>            set it? You want a callback to copy guid there.
> 
> 
>         Sure, I can do that.  My understanding was that this is a read
>         callback, but if
>         it also is called upon a write, we should do what you suggest.
> 
> 
>     Hmm you are right. But we really need to call something
>     on write though - unlike read, it must be called after write.
>     Otherwise I don't see how it can work if you set gen id before
>     guest boots.
> 
>     I guess this means we need yet another callback per file.
>     FWCfgWriteCallback ?
> 
>     Can you implement this in hw/nvram/fw_cfg.c?
>     It's rather straight-forward to do.
> 
> 
> The reason it works is that we put the initial contents of the GUID (as
> supplied by command-line) into the GUID fw_cfg in the ‘vmgenid_build_acpi()’
> function, which is guaranteed to happen before the guest boots.  The only time
> QEMU needs to know VGIA is on later updates to the GUID (via monitor) or when
> restoring.  If you really think this extra complexity is needed, I can do so,
> but it seems to work very well as-is.

I see. So it's a race condition I think.  It works unless you change the
gen id after bios read the guid from the guid file but before it wrote
out the address.

Or do I miss something?

-- 
MST