On Mon, Feb 06, 2017 at 10:48:05AM -0800, Ben Warren wrote: > > > 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.
When people run 100000 VMs and up, every unlikely scenario tends to trigger. > > -- > > MST >