Re: [Qemu-devel] Virtual Machine Generation ID

[Michael S. Tsirkin]

Because this relies on guest to run code to read out the new fw cfg.
Thus guest can not reliably detect that host didn't update the gen id -
new one might be there in fw cfg but not yet read.

RSDP never changes during guest lifetime so the issue does
not occur.

On Tue, Jan 17, 2017 at 06:26:57AM -0800, Ed Swierk wrote:
> Why is using fw_cfg for vmgenid preferable to the approach used for
> RSDP: call acpi_add_rom_blob() to allocate a MemoryRegion with the
> initial vmgenid guid, and call acpi_ram_update() with the new guid
> whenever the host needs to change it?
> 
> --Ed
> 
> 
> On Tue, Jan 17, 2017 at 5:26 AM, Igor Mammedov <address@hidden> wrote:
> > On Mon, 16 Jan 2017 10:57:42 -0800
> > Ben Warren <address@hidden> wrote:
> >
> >> > On Jan 16, 2017, at 6:21 AM, Michael S. Tsirkin <address@hidden> wrote:
> >> >
> >> > On Sat, Jan 14, 2017 at 10:17:53PM -0800, Ben Warren wrote:
> >> >> Hi Michael,
> >> >>
> >> >>
> >> >>    On Dec 10, 2016, at 7:28 PM, Michael S. Tsirkin <address@hidden> 
> >> >> wrote:
> >> >>
> >> >>    On Tue, Dec 06, 2016 at 06:15:34PM -0800, Ben Warren wrote:
> >> >>
> >> >>        Hi Michael,
> >> >>
> >> >>        I’m well on my way to implementing this, but I am really new to 
> >> >> the
> >> >>        QEMU code base and am struggling with some concepts.  Please see 
> >> >> below:
> >> >>
> >> >>            On Oct 5, 2016, at 6:29 PM, Michael S. Tsirkin 
> >> >> <address@hidden>
> >> >>            wrote:
> >> >>
> >> >>            On Tue, Oct 04, 2016 at 03:51:40PM -0700, Ed Swierk wrote:
> >> >>
> >> >>                On Thu, Sep 15, 2016 at 5:36 PM, Michael S. Tsirkin <
> >> >>                address@hidden> wrote:
> >> >>
> >> >>                    On Thu, Sep 15, 2016 at 05:23:28PM -0700, Ed Swierk 
> >> >> wrote:
> >> >>
> >> >>                        I'm wondering what it will take to finish up 
> >> >> work on
> >> >>                        vmgenid.
> >> >>
> >> >>                        
> >> >> https://lists.gnu.org/archive/html/qemu-devel/2016-01/
> >> >>                        msg05599.html
> >> >>
> >> >>
> >> >>                    We have ACPI_BUILD_TPMLOG_FILE in tree now and I 
> >> >> think it
> >> >>                    could be
> >> >>                    allocated in a similar way.
> >> >>                    Integrate patch "fw-cfg: support writeable blobs" to
> >> >>                    communicate the
> >> >>                    allocated address back to QEMU.
> >> >>
> >> >>
> >> >>                Starting with Igor's last version at
> >> >>                https://github.com/imammedo/qemu/commits/vmgen_wip , 
> >> >> it's not
> >> >>                clear to
> >> >>                me which changes need to be ported, which changes are 
> >> >> obsoleted
> >> >>                by
> >> >>                your new fw-cfg stuff and/or upstream churn in ACPI, 
> >> >> device
> >> >>                properties, etc. In particular ACPI is still a total 
> >> >> mystery to
> >> >>                me,
> >> >>                though passing a single address from guest to host can't 
> >> >> be
> >> >>                that hard,
> >> >>                can it?
> >> >>
> >> >>                Any clues would be appreciated.
> >> >>
> >> >>                --Ed
> >> >>
> >> >>
> >> >>            It might be best to just re-start from the beginning.
> >> >>            So the idea is that ACPI should be about supplying the 
> >> >> address
> >> >>            to guest. To supply address to host we'll use fw cfg.
> >> >>            This would be new I think:
> >> >>
> >> >>            - add support for writeable fw cfg blobs
> >> >>
> >> >>        patch applied
> >> >>
> >> >>            - add linker/loader command to write address of a blob into
> >> >>            such a fw cfg file
> >> >>            - add a new file used for vm gen id, use loader command above
> >> >>            to pass the address of a blob allocated for it to host
> >> >>
> >> >>        I don’t really understand the meaning of “file” in this context. 
> >> >>  It
> >> >>        seems to be a way of specifying individual fw_cfg entries without
> >> >>        explicitly giving an index, but is not something that is visible 
> >> >> in
> >> >>        either the host or guest file system.  Is this about right?  In 
> >> >> my code
> >> >>        I’m using “/etc/vmgenid”
> >> >>
> >> >>
> >> >>    yes
> >> >>
> >> >>
> >> >>        As for the blob, I’m thinking this is where my main problem is.  
> >> >> The
> >> >>        ‘fw_cfg_add_*()’ functions take a data pointer but doesn’t seem 
> >> >> to copy
> >> >>        the data anywhere.  We pass essentially a pointer via ACPI to the
> >> >>        guest, so what it points to needs to be in an accessible region. 
> >> >>  I
> >> >>        don’t get how to define the blob contents.  There are 
> >> >> command-line
> >> >>        ‘fw-cfg’ options where you can specify a file, but it’s not 
> >> >> clear to me
> >> >>        how to use them.  Maybe I reserve some IO memory or something?
> >> >>
> >> >>
> >> >>    Not sure I understand the question. fw cfg device will make
> >> >>    memory accessible to guest. Put the guest physical address there.
> >> >>    the address needs to be calculated by linker.
> >> >>
> >> >>
> >> >> I’m almost ready to submit a V2 of the patch set, but there’s still one 
> >> >> issue
> >> >> that I can’t figure out.  From the guest, I can read the contents of 
> >> >> the blob.
> >> >> If I make a change to the contents of the blob (via QMP) the guest does 
> >> >> not
> >> >> see the changes.  Is there something I need to do on the QEMU side to 
> >> >> “push”
> >> >> the updated fw_cfg contents to the guest?  I’ve noticed this both when 
> >> >> writing
> >> >> a qtest for the feature, and also in a Linux kernel module I wrote that 
> >> >> reads
> >> >> the ACPI contents in a guest.
> >> >>
> >> >> thanks,
> >> >> Ben
> >> >
> >> > fw cfg entities are assumed to be immutable. This week
> >> > I'll merge support for writeable fw cfg entries.
> >> > I don't see why you want to change fw cfg transparently
> >> > though - I think it should be like this
> >> > - guest writes GPA into fw cfg
> >> > - qemu writes gen id at this GPA
> >> >
> >> I’ve tried with your patch "fw-cfg-support-writeable-blobs”, setting the 
> >> ‘read-only’ flag on my blob to false:
> >>
> >> fw_cfg_add_file_callback(s, VMGENID_FW_CFG_FILE, NULL, NULL, 
> >> vms->guid.data, sizeof(vms->guid.data), false);
> >>
> >> and it doesn’t seem to make a difference.
> >>
> >> I think we have a misunderstanding here.  I’m storing the VM Generation ID 
> >> __data__ (a GUID) in a fw_cfg blob, not the address.  I’ll submit the 
> >> patch set ASAP so you will understand.
> > there  should be another fw_cfg file for address so
> > that guest would be able to write it back into QEMU
> >
> >>
> >> > --
> >> > MST
> >>
> >> regards,
> >> Ben
> >>
> >