> On Feb 6, 2017, at 10:17 AM, Michael S. Tsirkin <m...@redhat.com> wrote:
>
> 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?
>
I don’t think it’s an issue because BIOS is not a consumer of the GUID. I
suppose there’s a window where another GUID could be written via monitor prior
to bios writing back, but the change wouldn’t be applied to memory in
vmgenid_update_guest() because vgia=0. That would be solved by a write
callback and we could remove the GUID copying from vmgenid_build_acpi(). It’s
a very unlikely scenario, though.
> --
> MST
smime.p7s
Description: S/MIME cryptographic signature
