[Qemu-devel] Virtual Machine Generation ID

[Qemu-devel] Virtual Machine Generation ID

[Ed Swierk]

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

It appears all of the designs explored through the 19 iterations were
problematic in some way. Is any of them vaguely acceptable to all
involved in the discussions? Or do we need to start from square one?

--Ed

Re: [Qemu-devel] Virtual Machine Generation ID

[Michael S. Tsirkin]

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
> 
> It appears all of the designs explored through the 19 iterations were
> problematic in some way. Is any of them vaguely acceptable to all
> involved in the discussions? Or do we need to start from square one?
> 
> --Ed

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.

-- 
MST

Re: [Qemu-devel] Virtual Machine Generation ID

[Ed Swierk]

On Thu, Sep 15, 2016 at 5:36 PM, Michael S. Tsirkin <mst@redhat.com> 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

Re: [Qemu-devel] Virtual Machine Generation ID

[Igor Mammedov]

On Tue, 4 Oct 2016 15:51:40 -0700
Ed Swierk <eswierk@skyportsystems.com> wrote:

> On Thu, Sep 15, 2016 at 5:36 PM, Michael S. Tsirkin <mst@redhat.com> 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

Eventually DMA method for allocating qemu<->guest memory prevailed
so you could use following commits as reference:

f7df22d nvdimm acpi: emulate dsm method
18c440e nvdimm acpi: let qemu handle _DSM method
b995141 nvdimm acpi: introduce patched dsm memory
5fe7938 nvdimm acpi: initialize the resource used by NVDIMM ACPI

Re: [Qemu-devel] Virtual Machine Generation ID

[Michael S. Tsirkin]

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 <mst@redhat.com> 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
- 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
- whenever vm gen id changes, update file, if we have address
  of blob update that as well
- use linker to patch address of blob into acpi as well
  it needs to be in a separate ssdt at top level
  otherwise it's hard to figure out the offset.

This can be reused from one of the previus versions:
- command line, interrupts and commands to update vm gen id

I can help with acpi if all the rest is clear.

-- 
MST

Re: [Qemu-devel] Virtual Machine Generation ID

[Ben Warren]

[-- Attachment #1: Type: text/plain, Size: 5051 bytes --]

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 <mst@redhat.com> 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 <mst@redhat.com> 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”

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?
> - whenever vm gen id changes, update file, if we have address
>  of blob update that as well
Will do that once I understand the other parts
> - use linker to patch address of blob into acpi as well
>  it needs to be in a separate ssdt at top level
>  otherwise it's hard to figure out the offset.
I have this going into a separate SSDT and can decode it using iasl on the guest:
—————
[root@playground ~]# cat SSDT.dsl 
/*
 * Intel ACPI Component Architecture
 * AML/ASL+ Disassembler version 20150717-64
 * Copyright (c) 2000 - 2015 Intel Corporation
 * 
 * Disassembling to symbolic ASL+ operators
 *
 * Disassembly of SSDT, Tue Dec  6 16:35:14 2016
 *
 * Original Table Header:
 *     Signature        "SSDT"
 *     Length           0x0000007C (124)
 *     Revision         0x01
 *     Checksum         0x29
 *     OEM ID           "BOCHS "
 *     OEM Table ID     "VMGENID"
 *     OEM Revision     0x00000001 (1)
 *     Compiler ID      "BXPC"
 *     Compiler Version 0x00000001 (1)
 */
DefinitionBlock ("SSDT.aml", "SSDT", 1, "BOCHS ", "VMGENID", 0x00000001)
{
    Scope (_SB)
    {
        Device (VGEN)
        {
            Name (_HID, "QEMU0003")  // _HID: Hardware ID
            Name (_CID, "VM_Gen_Counter")  // _CID: Compatible ID
            Name (_DDN, "VM_Gen_Counter")  // _DDN: DOS Device Name
            Name (ADDR, Package (0x02)
            {
                0xE7AAD010, 
                0x559F
            })
        }
    }
}
—————
The address you see is the address of the host memory where I have the GUID stored, so is obviously incorrect.

> 
> This can be reused from one of the previus versions:
> - command line, interrupts and commands to update vm gen id
> 
I have these all applied and can set/get the GUID via the QMP shell, and can pass GUID via QEMU command line.  As for parsing the input, I have it modeled as a device right on the sysbus.  Is this how it should be done, or would I pass the GUID in via the ‘fw_cfg’ command-line options, or something else?  It’s a bit problematic as-is because it depends on machine types that allow dynamic-sysbus.
> I can help with acpi if all the rest is clear.
> 


> -- 
> MST
> 
> 
Sorry for being so dense - I think once I get the main concepts the rest of this should be straightforward.  I’m happy to post a patch of the current work if that would help.  Thanks in advance for your help.

—Ben

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3583 bytes --]

Re: [Qemu-devel] Virtual Machine Generation ID

[Michael S. Tsirkin]

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 <mst@redhat.com> 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 <mst@redhat.com> 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.



> > - whenever vm gen id changes, update file, if we have address
> >  of blob update that as well
> Will do that once I understand the other parts
> > - use linker to patch address of blob into acpi as well
> >  it needs to be in a separate ssdt at top level
> >  otherwise it's hard to figure out the offset.
> I have this going into a separate SSDT and can decode it using iasl on the guest:
> —————
> [root@playground ~]# cat SSDT.dsl 
> /*
>  * Intel ACPI Component Architecture
>  * AML/ASL+ Disassembler version 20150717-64
>  * Copyright (c) 2000 - 2015 Intel Corporation
>  * 
>  * Disassembling to symbolic ASL+ operators
>  *
>  * Disassembly of SSDT, Tue Dec  6 16:35:14 2016
>  *
>  * Original Table Header:
>  *     Signature        "SSDT"
>  *     Length           0x0000007C (124)
>  *     Revision         0x01
>  *     Checksum         0x29
>  *     OEM ID           "BOCHS "
>  *     OEM Table ID     "VMGENID"
>  *     OEM Revision     0x00000001 (1)
>  *     Compiler ID      "BXPC"
>  *     Compiler Version 0x00000001 (1)
>  */
> DefinitionBlock ("SSDT.aml", "SSDT", 1, "BOCHS ", "VMGENID", 0x00000001)
> {
>     Scope (_SB)
>     {
>         Device (VGEN)
>         {
>             Name (_HID, "QEMU0003")  // _HID: Hardware ID
>             Name (_CID, "VM_Gen_Counter")  // _CID: Compatible ID
>             Name (_DDN, "VM_Gen_Counter")  // _DDN: DOS Device Name
>             Name (ADDR, Package (0x02)
>             {
>                 0xE7AAD010, 
>                 0x559F
>             })
>         }
>     }
> }
> —————
> The address you see is the address of the host memory where I have the GUID stored, so is obviously incorrect.
> 
> > 
> > This can be reused from one of the previus versions:
> > - command line, interrupts and commands to update vm gen id
> > 
> I have these all applied and can set/get the GUID via the QMP shell, and can pass GUID via QEMU command line.  As for parsing the input, I have it modeled as a device right on the sysbus.  Is this how it should be done, or would I pass the GUID in via the ‘fw_cfg’ command-line options, or something else?  It’s a bit problematic as-is because it depends on machine types that allow dynamic-sysbus.
> > I can help with acpi if all the rest is clear.
> > 
> 
> 
> > -- 
> > MST
> > 
> > 
> Sorry for being so dense - I think once I get the main concepts the rest of this should be straightforward.  I’m happy to post a patch of the current work if that would help.  Thanks in advance for your help.
> 
> —Ben

Re: [Qemu-devel] Virtual Machine Generation ID

[Ben Warren]

[-- Attachment #1: Type: text/plain, Size: 3551 bytes --]

Hi Michael,

> On Dec 10, 2016, at 7:28 PM, Michael S. Tsirkin <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3583 bytes --]

Re: [Qemu-devel] Virtual Machine Generation ID

[Igor Mammedov]

On Sat, 14 Jan 2017 22:17:53 -0800
Ben Warren <ben@skyportsystems.com> wrote:

> Hi Michael,
> 
> > On Dec 10, 2016, at 7:28 PM, Michael S. Tsirkin <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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.
post here a link to your current repo so I could check it

> 
> thanks,
> Ben

Re: [Qemu-devel] Virtual Machine Generation ID

[Michael S. Tsirkin]

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 <mst@redhat.com> 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 <mst@redhat.com>
>             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 <
>                 mst@redhat.com> 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

-- 
MST

Re: [Qemu-devel] Virtual Machine Generation ID

[Ben Warren]

[-- Attachment #1: Type: text/plain, Size: 5354 bytes --]


> On Jan 16, 2017, at 6:21 AM, Michael S. Tsirkin <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com>
>>            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 <
>>                mst@redhat.com> 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.

> -- 
> MST

regards,
Ben


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3583 bytes --]

Re: [Qemu-devel] Virtual Machine Generation ID

[Igor Mammedov]

On Mon, 16 Jan 2017 10:57:42 -0800
Ben Warren <ben@skyportsystems.com> wrote:

> > On Jan 16, 2017, at 6:21 AM, Michael S. Tsirkin <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com>
> >>            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 <  
> >>                mst@redhat.com> 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
> 

Re: [Qemu-devel] Virtual Machine Generation ID

[Ed Swierk]

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 <imammedo@redhat.com> wrote:
> On Mon, 16 Jan 2017 10:57:42 -0800
> Ben Warren <ben@skyportsystems.com> wrote:
>
>> > On Jan 16, 2017, at 6:21 AM, Michael S. Tsirkin <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com>
>> >>            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 <
>> >>                mst@redhat.com> 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
>>
>

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 <imammedo@redhat.com> wrote:
> > On Mon, 16 Jan 2017 10:57:42 -0800
> > Ben Warren <ben@skyportsystems.com> wrote:
> >
> >> > On Jan 16, 2017, at 6:21 AM, Michael S. Tsirkin <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com>
> >> >>            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 <
> >> >>                mst@redhat.com> 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
> >>
> >

Re: [Qemu-devel] Virtual Machine Generation ID

[Ed Swierk]

You mean what causes the guest to re-read the vmgenid guid? The
vmgenid ACPI table defines a notify method, and when the guest
receives the corresponding event, it re-reads the guid. (Also it
appears that with Windows Server 2012 at least, if no notify method is
defined, as is the case with Xen, the guest just re-reads it on
demand.)

Wouldn't it be sufficient for the qmp set-vmgenid command to call
acpi_ram_update() with the new guid, and acpi_send_event() to notify
the guest?

--Ed


On Tue, Jan 17, 2017 at 6:33 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> 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 <imammedo@redhat.com> wrote:
>> > On Mon, 16 Jan 2017 10:57:42 -0800
>> > Ben Warren <ben@skyportsystems.com> wrote:
>> >
>> >> > On Jan 16, 2017, at 6:21 AM, Michael S. Tsirkin <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com>
>> >> >>            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 <
>> >> >>                mst@redhat.com> 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
>> >>
>> >

Re: [Qemu-devel] Virtual Machine Generation ID

[Michael S. Tsirkin]

Let's not top-post anymore pls.

On Tue, Jan 17, 2017 at 07:01:27AM -0800, Ed Swierk wrote:
> You mean what causes the guest to re-read the vmgenid guid? The
> vmgenid ACPI table defines a notify method, and when the guest
> receives the corresponding event, it re-reads the guid. (Also it
> appears that with Windows Server 2012 at least, if no notify method is
> defined, as is the case with Xen, the guest just re-reads it on
> demand.)
> 
> Wouldn't it be sufficient for the qmp set-vmgenid command to call
> acpi_ram_update() with the new guid, and acpi_send_event() to notify
> the guest?
> 
> --Ed
> 

Not if you want to reliably be able to know that gen ID
did not change.

Consider an application that sends a transaction to
a database. It should be able to read gen ID and if that
is unchanged then you know you only sent it once.
If that is changed you may have sent it twice,
and may need to recover.

If guest updates gen id in memory after getting a notify,
there's a window after receiving a notify and before
updating it.



-- 
MST

Re: [Qemu-devel] Virtual Machine Generation ID

[Ben Warren]

[-- Attachment #1: Type: text/plain, Size: 1706 bytes --]


> On Jan 17, 2017, at 7:21 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> 
> Let's not top-post anymore pls.
> 
> On Tue, Jan 17, 2017 at 07:01:27AM -0800, Ed Swierk wrote:
>> You mean what causes the guest to re-read the vmgenid guid? The
>> vmgenid ACPI table defines a notify method, and when the guest
>> receives the corresponding event, it re-reads the guid. (Also it
>> appears that with Windows Server 2012 at least, if no notify method is
>> defined, as is the case with Xen, the guest just re-reads it on
>> demand.)
>> 
>> Wouldn't it be sufficient for the qmp set-vmgenid command to call
>> acpi_ram_update() with the new guid, and acpi_send_event() to notify
>> the guest?
>> 
>> --Ed
>> 
> 
> Not if you want to reliably be able to know that gen ID
> did not change.
> 
> Consider an application that sends a transaction to
> a database. It should be able to read gen ID and if that
> is unchanged then you know you only sent it once.
> If that is changed you may have sent it twice,
> and may need to recover.
> 
> If guest updates gen id in memory after getting a notify,
> there's a window after receiving a notify and before
> updating it.
> 
> 
Guests don’t update gen ID, the value is read-only to Windows.  The spec states:

“Put the generation ID in an 8-byte aligned buffer in guest RAM, ROM or device memory space, which is guaranteed not to be used by the operating system”

The host is in complete control of the data, so as long as the notify event follows setting of the data, there should be no race.  In practice, the only time the value will change in a running guest is when recovering a snapshot.
> 
> -- 
> MST
—Ben


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3583 bytes --]

Re: [Qemu-devel] Virtual Machine Generation ID

[Igor Mammedov]

On Tue, 17 Jan 2017 07:01:27 -0800
Ed Swierk <eswierk@skyportsystems.com> wrote:

> You mean what causes the guest to re-read the vmgenid guid? The
> vmgenid ACPI table defines a notify method, and when the guest
> receives the corresponding event, it re-reads the guid. (Also it
> appears that with Windows Server 2012 at least, if no notify method is
> defined, as is the case with Xen, the guest just re-reads it on
> demand.)
> 
> Wouldn't it be sufficient for the qmp set-vmgenid command to call
> acpi_ram_update() with the new guid, and acpi_send_event() to notify
> the guest?
pls note that acpi_ram_update() updates only memory on qemu side and
it's not mapped into guest. Updated memory will be re-read by firmware
when guest reboots.

But with vmgenid, QEMU might want to update guest RAM which
contains GUID without reboot, and for this it needs to know GPA
of GUID buffer and only after updating it send ACPI event.

BTW in-place update is racy anyways as OS could be reading it
at the same time, but we can't do anything about it as vmgenid
spec didn't provide means for atomic update.

> --Ed
> 
> 
> On Tue, Jan 17, 2017 at 6:33 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > 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 <imammedo@redhat.com> wrote:  
> >> > On Mon, 16 Jan 2017 10:57:42 -0800
> >> > Ben Warren <ben@skyportsystems.com> wrote:
> >> >  
> >> >> > On Jan 16, 2017, at 6:21 AM, Michael S. Tsirkin <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com>
> >> >> >>            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 <  
> >> >> >>                mst@redhat.com> 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
> >> >>  
> >> >  

Re: [Qemu-devel] Virtual Machine Generation ID

[Michael S. Tsirkin]

On Tue, Jan 17, 2017 at 05:24:04PM +0100, Igor Mammedov wrote:
> On Tue, 17 Jan 2017 07:01:27 -0800
> Ed Swierk <eswierk@skyportsystems.com> wrote:
> 
> > You mean what causes the guest to re-read the vmgenid guid? The
> > vmgenid ACPI table defines a notify method, and when the guest
> > receives the corresponding event, it re-reads the guid. (Also it
> > appears that with Windows Server 2012 at least, if no notify method is
> > defined, as is the case with Xen, the guest just re-reads it on
> > demand.)
> > 
> > Wouldn't it be sufficient for the qmp set-vmgenid command to call
> > acpi_ram_update() with the new guid, and acpi_send_event() to notify
> > the guest?
> pls note that acpi_ram_update() updates only memory on qemu side and
> it's not mapped into guest. Updated memory will be re-read by firmware
> when guest reboots.
> 
> But with vmgenid, QEMU might want to update guest RAM which
> contains GUID without reboot, and for this it needs to know GPA
> of GUID buffer and only after updating it send ACPI event.

+1 thanks, thanks is what I was trying to say.

> BTW in-place update is racy anyways as OS could be reading it
> at the same time,

That would be up to the applications to use correctly.
E.g.

	check id
	send transaction
	---> racy

	send transaction
	check id
	---> not racy

I can't of course answer for it that all applications use the id correctly.
Some of them might decide to accept the risk of a race condition.

> but we can't do anything about it as vmgenid
> spec didn't provide means for atomic update.

I agree it's not up to us to fix.

> 
> > --Ed
> > 
> > 
> > On Tue, Jan 17, 2017 at 6:33 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > > 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 <imammedo@redhat.com> wrote:  
> > >> > On Mon, 16 Jan 2017 10:57:42 -0800
> > >> > Ben Warren <ben@skyportsystems.com> wrote:
> > >> >  
> > >> >> > On Jan 16, 2017, at 6:21 AM, Michael S. Tsirkin <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com>
> > >> >> >>            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 <  
> > >> >> >>                mst@redhat.com> 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
> > >> >>  
> > >> >  

Re: [Qemu-devel] Virtual Machine Generation ID

[Michael S. Tsirkin]

On Mon, Jan 16, 2017 at 10:57:42AM -0800, Ben Warren wrote:
> 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.

Yes, I think I gathered this much from the discussion. This is what
I'm saying - don't. Have guest loader reserve guest memory and write the
address into a fw cfg blob, and have qemu write the id at that address.
This way you can update guest memory at any time.

-- 
MST

Re: [Qemu-devel] Virtual Machine Generation ID

[Ben Warren]

[-- Attachment #1: Type: text/plain, Size: 1792 bytes --]

Hi Michael,
> On Jan 17, 2017, at 9:45 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> 
> On Mon, Jan 16, 2017 at 10:57:42AM -0800, Ben Warren wrote:
>> 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.
> 
> Yes, I think I gathered this much from the discussion. This is what
> I'm saying - don't. Have guest loader reserve guest memory and write the
> address into a fw cfg blob, and have qemu write the id at that address.
> This way you can update guest memory at any time.
> 
So I’ve gone down the path of creating a writeable fw_cfg blob to hold the VGID address, but it doesn’t seem to be getting updated.

Here’s the code I’ve added:

#define VMGENID_FW_CFG_FILE      "etc/vmgenid"
#define VMGENID_FW_CFG_ADDR_FILE      "etc/vmgenid_addr”

// Create writeable fw_cfg blob, vas->vgia is a GArray of size 8 and element size 1
fw_cfg_add_file_callback(s, VMGENID_FW_CFG_ADDR_FILE, NULL, NULL, vms->vgia->data, 8, false);

// Request BIOS to allocate memory for the read-only DATA file:
bios_linker_loader_alloc(linker, VMGENID_FW_CFG_FILE, guid, 0,false);

// Request BIOS to allocate memory for the writeable ADDRESS file:
bios_linker_loader_alloc(linker, VMGENID_FW_CFG_ADDR_FILE, s->vgia, 0, false);

// Request BIOS to write the address of the DATA file into the ADDRESS file:
bios_linker_loader_add_pointer(linker, VMGENID_FW_CFG_ADDR_FILE, 0, sizeof(uint64_t), VMGENID_FW_CFG_FILE, 0);

I’ve instrumented SeaBIOS and see the requests being made and memcpy to the file happening, but don’t see any changes in QEMU in the memory pointed to by VMGENID_FW_CFG_ADDR_FILE.  Is this how writeable fw_cfg is supposed to work?

thanks,
Ben

> -- 
> MST


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3583 bytes --]

Re: [Qemu-devel] Virtual Machine Generation ID

[Ben Warren]

[-- Attachment #1: Type: text/plain, Size: 2446 bytes --]


> On Jan 18, 2017, at 4:02 PM, Ben Warren <ben@skyportsystems.com> wrote:
> 
> Hi Michael,
>> On Jan 17, 2017, at 9:45 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> 
>> On Mon, Jan 16, 2017 at 10:57:42AM -0800, Ben Warren wrote:
>>> 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.
>> 
>> Yes, I think I gathered this much from the discussion. This is what
>> I'm saying - don't. Have guest loader reserve guest memory and write the
>> address into a fw cfg blob, and have qemu write the id at that address.
>> This way you can update guest memory at any time.
>> 
> So I’ve gone down the path of creating a writeable fw_cfg blob to hold the VGID address, but it doesn’t seem to be getting updated.
> 
> Here’s the code I’ve added:
> 
> #define VMGENID_FW_CFG_FILE      "etc/vmgenid"
> #define VMGENID_FW_CFG_ADDR_FILE      "etc/vmgenid_addr”
> 
> // Create writeable fw_cfg blob, vas->vgia is a GArray of size 8 and element size 1
> fw_cfg_add_file_callback(s, VMGENID_FW_CFG_ADDR_FILE, NULL, NULL, vms->vgia->data, 8, false);
> 
> // Request BIOS to allocate memory for the read-only DATA file:
> bios_linker_loader_alloc(linker, VMGENID_FW_CFG_FILE, guid, 0,false);
> 
> // Request BIOS to allocate memory for the writeable ADDRESS file:
> bios_linker_loader_alloc(linker, VMGENID_FW_CFG_ADDR_FILE, s->vgia, 0, false);
> 
> // Request BIOS to write the address of the DATA file into the ADDRESS file:
> bios_linker_loader_add_pointer(linker, VMGENID_FW_CFG_ADDR_FILE, 0, sizeof(uint64_t), VMGENID_FW_CFG_FILE, 0);
> 
> I’ve instrumented SeaBIOS and see the requests being made and memcpy to the file happening, but don’t see any changes in QEMU in the memory pointed to by VMGENID_FW_CFG_ADDR_FILE.  Is this how writeable fw_cfg is supposed to work?
> 
Ed explained it to me and I think I get it now.  I realize that you and Igor have explained this throughout the e-mail chain, but I’m a little bit slower.

Anyway, is this understanding correct?  BIOS is in fact patching fw_cfg data, but it’s in a copy of the fw_cfg file that is in guest space.  In order to get this to work we would need to add a new command to the linker/loader protocol to write back changes to QEMU after patching, and of course implement the change in BIOS and UEFI projects.
> thanks,
> Ben
> 
>> -- 
>> MST
> 


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3583 bytes --]

Re: [Qemu-devel] Virtual Machine Generation ID

[Laszlo Ersek]

On 01/19/17 08:09, Ben Warren wrote:
>
>> On Jan 18, 2017, at 4:02 PM, Ben Warren <ben@skyportsystems.com>
>> wrote:
>>
>> Hi Michael,
>>> On Jan 17, 2017, at 9:45 AM, Michael S. Tsirkin <mst@redhat.com>
>>> wrote:
>>>
>>> On Mon, Jan 16, 2017 at 10:57:42AM -0800, Ben Warren wrote:
>>>> 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.
>>>
>>> Yes, I think I gathered this much from the discussion. This is what
>>> I'm saying - don't. Have guest loader reserve guest memory and write
>>> the address into a fw cfg blob, and have qemu write the id at that
>>> address. This way you can update guest memory at any time.
>>>
>> So I've gone down the path of creating a writeable fw_cfg blob to
>> hold the VGID address, but it doesn't seem to be getting updated.
>>
>> Here's the code I've added:
>>
>> #define VMGENID_FW_CFG_FILE      "etc/vmgenid"
>> #define VMGENID_FW_CFG_ADDR_FILE      "etc/vmgenid_addr"
>>
>> // Create writeable fw_cfg blob, vas->vgia is a GArray of size 8 and element size 1
>> fw_cfg_add_file_callback(s, VMGENID_FW_CFG_ADDR_FILE, NULL, NULL, vms->vgia->data, 8, false);
>>
>> // Request BIOS to allocate memory for the read-only DATA file:
>> bios_linker_loader_alloc(linker, VMGENID_FW_CFG_FILE, guid, 0,false);
>>
>> // Request BIOS to allocate memory for the writeable ADDRESS file:
>> bios_linker_loader_alloc(linker, VMGENID_FW_CFG_ADDR_FILE, s->vgia, 0, false);
>>
>> // Request BIOS to write the address of the DATA file into the ADDRESS file:
>> bios_linker_loader_add_pointer(linker, VMGENID_FW_CFG_ADDR_FILE, 0, sizeof(uint64_t), VMGENID_FW_CFG_FILE, 0);
>>
>> I've instrumented SeaBIOS and see the requests being made and memcpy
>> to the file happening, but don't see any changes in QEMU in the
>> memory pointed to by VMGENID_FW_CFG_ADDR_FILE.  Is this how writeable
>> fw_cfg is supposed to work?
>>
> Ed explained it to me and I think I get it now.  I realize that you
> and Igor have explained this throughout the e-mail chain, but I'm a
> little bit slower.
>
> Anyway, is this understanding correct?  BIOS is in fact patching
> fw_cfg data, but it's in a copy of the fw_cfg file that is in guest
> space.  In order to get this to work we would need to add a new
> command to the linker/loader protocol to write back changes to QEMU
> after patching, and of course implement the change in BIOS and UEFI
> projects.

(1) It's not enough to create just the "etc/vmgenid_addr" file; the
"etc/vmgenid" file must exist too, so that the firmware can download it.

(2) I don't understand why you need the ADD_POINTER command here. I
think it's unnecessary.

(3) The ALLOCATE command for "etc/vmgenid_addr" is also unnecessary.

(4) A new ALLOCATE_RETURN_ADDR command type should be introduced, with
the following contents:

        struct {
            char file[BIOS_LINKER_LOADER_FILESZ];
            uint32_t align;
            uint8_t zone;
            char addr_file[BIOS_LINKER_LOADER_FILESZ];
        } alloc_return_addr;

The first three fields have identical meanings to those of the current
ALLOCATE command. The last field instructs the firmware as to what
fw_cfg file to write the 8-byte physical address back to, in little
endian byte order, of the actual allocation.

(5) The linker/loader script should then use one command in total,
namely ALLOCATE_RETURN_ADDR, with

  file = etc/vmgenid
  addr_file = etc/vmgenid_addr

This will allow both SeaBIOS and OVMF to do the right thing.

In summary:
- create the read-only "etc/vmgenid" fw_cfg file
- create the writeable "etc/vmgenid_addr" fw_cfg file
- use one ALLOCATE_RETURN_ADDR command, and nothing else.

I hereby volunteer to write the OVMF patch for the ALLOCATE_RETURN_ADDR
command type.

If someone is interested, I'll remind us that EFI_ACPI_TABLE_PROTOCOL
installs *copies* of ACPI tables, out of the fw_cfg blobs that were
downloaded. Therefore OVMF tracks all ADD_POINTER commands that point
into fw_cfg blobs, and if an fw_cfg blob is not pointed-to by *any*
ADD_POINTER command, or else *all* ADD_POINTER commands that point into
it point to ACPI table headers, then OVMF will ultimately free the
originally downloaded fw_cfg blob. If there is at least one referring
ADD_POINTER command that points to non-ACPI-table data within the blob,
then OVMF marks the blob to be preserved permanently, in AcpiNVS type
memory.

By introducing the above ALLOCATE_RETURN_ADDR command type, OVMF can do
two things:

(a) immediately mark the blob for permanent preservation (i.e., it won't
    be released after the script processing is complete),
(b) write the actual allocation address back to the appropriate fw_cfg
    file.

For SeaBIOS, only (b) matters -- because it doesn't install *copies* of
ACPI tables; it rather keeps everything in place, as originally
allocated, and it just links things together --, but
ALLOCATE_RETURN_ADDR as proposed above should be suitable for SeaBIOS
just as well.

Thanks
Laszlo

Re: [Qemu-devel] Virtual Machine Generation ID

[Ben Warren]

[-- Attachment #1: Type: text/plain, Size: 6052 bytes --]

Thanks Laszlo!
> On Jan 19, 2017, at 1:25 AM, Laszlo Ersek <lersek@redhat.com> wrote:
> 
> On 01/19/17 08:09, Ben Warren wrote:
>> 
>>> On Jan 18, 2017, at 4:02 PM, Ben Warren <ben@skyportsystems.com>
>>> wrote:
>>> 
>>> Hi Michael,
>>>> On Jan 17, 2017, at 9:45 AM, Michael S. Tsirkin <mst@redhat.com>
>>>> wrote:
>>>> 
>>>> On Mon, Jan 16, 2017 at 10:57:42AM -0800, Ben Warren wrote:
>>>>> 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.
>>>> 
>>>> Yes, I think I gathered this much from the discussion. This is what
>>>> I'm saying - don't. Have guest loader reserve guest memory and write
>>>> the address into a fw cfg blob, and have qemu write the id at that
>>>> address. This way you can update guest memory at any time.
>>>> 
>>> So I've gone down the path of creating a writeable fw_cfg blob to
>>> hold the VGID address, but it doesn't seem to be getting updated.
>>> 
>>> Here's the code I've added:
>>> 
>>> #define VMGENID_FW_CFG_FILE      "etc/vmgenid"
>>> #define VMGENID_FW_CFG_ADDR_FILE      "etc/vmgenid_addr"
>>> 
>>> // Create writeable fw_cfg blob, vas->vgia is a GArray of size 8 and element size 1
>>> fw_cfg_add_file_callback(s, VMGENID_FW_CFG_ADDR_FILE, NULL, NULL, vms->vgia->data, 8, false);
>>> 
>>> // Request BIOS to allocate memory for the read-only DATA file:
>>> bios_linker_loader_alloc(linker, VMGENID_FW_CFG_FILE, guid, 0,false);
>>> 
>>> // Request BIOS to allocate memory for the writeable ADDRESS file:
>>> bios_linker_loader_alloc(linker, VMGENID_FW_CFG_ADDR_FILE, s->vgia, 0, false);
>>> 
>>> // Request BIOS to write the address of the DATA file into the ADDRESS file:
>>> bios_linker_loader_add_pointer(linker, VMGENID_FW_CFG_ADDR_FILE, 0, sizeof(uint64_t), VMGENID_FW_CFG_FILE, 0);
>>> 
>>> I've instrumented SeaBIOS and see the requests being made and memcpy
>>> to the file happening, but don't see any changes in QEMU in the
>>> memory pointed to by VMGENID_FW_CFG_ADDR_FILE.  Is this how writeable
>>> fw_cfg is supposed to work?
>>> 
>> Ed explained it to me and I think I get it now.  I realize that you
>> and Igor have explained this throughout the e-mail chain, but I'm a
>> little bit slower.
>> 
>> Anyway, is this understanding correct?  BIOS is in fact patching
>> fw_cfg data, but it's in a copy of the fw_cfg file that is in guest
>> space.  In order to get this to work we would need to add a new
>> command to the linker/loader protocol to write back changes to QEMU
>> after patching, and of course implement the change in BIOS and UEFI
>> projects.
> 
> (1) It's not enough to create just the "etc/vmgenid_addr" file; the
> "etc/vmgenid" file must exist too, so that the firmware can download it.
> 
I do have that file too, just didn’t show it.
> (2) I don't understand why you need the ADD_POINTER command here. I
> think it's unnecessary.
> 
> (3) The ALLOCATE command for "etc/vmgenid_addr" is also unnecessary.
> 
For both of these, I was working within the confines of the existing API, which we now know is inadequate.
> (4) A new ALLOCATE_RETURN_ADDR command type should be introduced, with
> the following contents:
> 
>        struct {
>            char file[BIOS_LINKER_LOADER_FILESZ];
>            uint32_t align;
>            uint8_t zone;
>            char addr_file[BIOS_LINKER_LOADER_FILESZ];
>        } alloc_return_addr;
> 
> The first three fields have identical meanings to those of the current
> ALLOCATE command. The last field instructs the firmware as to what
> fw_cfg file to write the 8-byte physical address back to, in little
> endian byte order, of the actual allocation.
> 
> (5) The linker/loader script should then use one command in total,
> namely ALLOCATE_RETURN_ADDR, with
> 
>  file = etc/vmgenid
>  addr_file = etc/vmgenid_addr
> 
> This will allow both SeaBIOS and OVMF to do the right thing.
> 
> In summary:
> - create the read-only "etc/vmgenid" fw_cfg file
> - create the writeable "etc/vmgenid_addr" fw_cfg file
> - use one ALLOCATE_RETURN_ADDR command, and nothing else.
> 
> I hereby volunteer to write the OVMF patch for the ALLOCATE_RETURN_ADDR
> command type.
> 
Sounds great.  We use SeaBIOS so I’ll try to do the same there.

So, just to make sure everything’s covered: this new mechanism will allow QEMU to have the guest allocate an arbitrary blob of memory (in the form of a fw_cfg file), and will get an offset within guest memory in return (via another fw_cfg file).  It can then translate the guest offset into a host address and update the blob at will.  Is this correct, because that’s what we need for VM Generation ID?

> If someone is interested, I'll remind us that EFI_ACPI_TABLE_PROTOCOL
> installs *copies* of ACPI tables, out of the fw_cfg blobs that were
> downloaded. Therefore OVMF tracks all ADD_POINTER commands that point
> into fw_cfg blobs, and if an fw_cfg blob is not pointed-to by *any*
> ADD_POINTER command, or else *all* ADD_POINTER commands that point into
> it point to ACPI table headers, then OVMF will ultimately free the
> originally downloaded fw_cfg blob. If there is at least one referring
> ADD_POINTER command that points to non-ACPI-table data within the blob,
> then OVMF marks the blob to be preserved permanently, in AcpiNVS type
> memory.
> 
> By introducing the above ALLOCATE_RETURN_ADDR command type, OVMF can do
> two things:
> 
> (a) immediately mark the blob for permanent preservation (i.e., it won't
>    be released after the script processing is complete),
> (b) write the actual allocation address back to the appropriate fw_cfg
>    file.
> 
> For SeaBIOS, only (b) matters -- because it doesn't install *copies* of
> ACPI tables; it rather keeps everything in place, as originally
> allocated, and it just links things together --, but
> ALLOCATE_RETURN_ADDR as proposed above should be suitable for SeaBIOS
> just as well.
> 
> Thanks
> Laszlo
—Ben


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3583 bytes --]

Re: [Qemu-devel] Virtual Machine Generation ID

[Laszlo Ersek]

On 01/19/17 18:47, Ben Warren wrote:
> Thanks Laszlo!
>> On Jan 19, 2017, at 1:25 AM, Laszlo Ersek <lersek@redhat.com
>> <mailto:lersek@redhat.com>> wrote:
>>
>> On 01/19/17 08:09, Ben Warren wrote:
>>>
>>>> On Jan 18, 2017, at 4:02 PM, Ben Warren <ben@skyportsystems.com
>>>> <mailto:ben@skyportsystems.com>>
>>>> wrote:
>>>>
>>>> Hi Michael,
>>>>> On Jan 17, 2017, at 9:45 AM, Michael S. Tsirkin <mst@redhat.com
>>>>> <mailto:mst@redhat.com>>
>>>>> wrote:
>>>>>
>>>>> On Mon, Jan 16, 2017 at 10:57:42AM -0800, Ben Warren wrote:
>>>>>> 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.
>>>>>
>>>>> Yes, I think I gathered this much from the discussion. This is what
>>>>> I'm saying - don't. Have guest loader reserve guest memory and write
>>>>> the address into a fw cfg blob, and have qemu write the id at that
>>>>> address. This way you can update guest memory at any time.
>>>>>
>>>> So I've gone down the path of creating a writeable fw_cfg blob to
>>>> hold the VGID address, but it doesn't seem to be getting updated.
>>>>
>>>> Here's the code I've added:
>>>>
>>>> #define VMGENID_FW_CFG_FILE      "etc/vmgenid"
>>>> #define VMGENID_FW_CFG_ADDR_FILE      "etc/vmgenid_addr"
>>>>
>>>> // Create writeable fw_cfg blob, vas->vgia is a GArray of size 8 and
>>>> element size 1
>>>> fw_cfg_add_file_callback(s, VMGENID_FW_CFG_ADDR_FILE, NULL, NULL,
>>>> vms->vgia->data, 8, false);
>>>>
>>>> // Request BIOS to allocate memory for the read-only DATA file:
>>>> bios_linker_loader_alloc(linker, VMGENID_FW_CFG_FILE, guid, 0,false);
>>>>
>>>> // Request BIOS to allocate memory for the writeable ADDRESS file:
>>>> bios_linker_loader_alloc(linker, VMGENID_FW_CFG_ADDR_FILE, s->vgia,
>>>> 0, false);
>>>>
>>>> // Request BIOS to write the address of the DATA file into the
>>>> ADDRESS file:
>>>> bios_linker_loader_add_pointer(linker, VMGENID_FW_CFG_ADDR_FILE, 0,
>>>> sizeof(uint64_t), VMGENID_FW_CFG_FILE, 0);
>>>>
>>>> I've instrumented SeaBIOS and see the requests being made and memcpy
>>>> to the file happening, but don't see any changes in QEMU in the
>>>> memory pointed to by VMGENID_FW_CFG_ADDR_FILE.  Is this how writeable
>>>> fw_cfg is supposed to work?
>>>>
>>> Ed explained it to me and I think I get it now.  I realize that you
>>> and Igor have explained this throughout the e-mail chain, but I'm a
>>> little bit slower.
>>>
>>> Anyway, is this understanding correct?  BIOS is in fact patching
>>> fw_cfg data, but it's in a copy of the fw_cfg file that is in guest
>>> space.  In order to get this to work we would need to add a new
>>> command to the linker/loader protocol to write back changes to QEMU
>>> after patching, and of course implement the change in BIOS and UEFI
>>> projects.
>>
>> (1) It's not enough to create just the "etc/vmgenid_addr" file; the
>> "etc/vmgenid" file must exist too, so that the firmware can download it.
>>
> I do have that file too, just didn’t show it.
>> (2) I don't understand why you need the ADD_POINTER command here. I
>> think it's unnecessary.
>>
>> (3) The ALLOCATE command for "etc/vmgenid_addr" is also unnecessary.
>>
> For both of these, I was working within the confines of the existing
> API, which we now know is inadequate.
>> (4) A new ALLOCATE_RETURN_ADDR command type should be introduced, with
>> the following contents:
>>
>>        struct {
>>            char file[BIOS_LINKER_LOADER_FILESZ];
>>            uint32_t align;
>>            uint8_t zone;
>>            char addr_file[BIOS_LINKER_LOADER_FILESZ];
>>        } alloc_return_addr;
>>
>> The first three fields have identical meanings to those of the current
>> ALLOCATE command. The last field instructs the firmware as to what
>> fw_cfg file to write the 8-byte physical address back to, in little
>> endian byte order, of the actual allocation.
>>
>> (5) The linker/loader script should then use one command in total,
>> namely ALLOCATE_RETURN_ADDR, with
>>
>>  file = etc/vmgenid
>>  addr_file = etc/vmgenid_addr
>>
>> This will allow both SeaBIOS and OVMF to do the right thing.
>>
>> In summary:
>> - create the read-only "etc/vmgenid" fw_cfg file
>> - create the writeable "etc/vmgenid_addr" fw_cfg file
>> - use one ALLOCATE_RETURN_ADDR command, and nothing else.
>>
>> I hereby volunteer to write the OVMF patch for the ALLOCATE_RETURN_ADDR
>> command type.
>>
> Sounds great.  We use SeaBIOS so I’ll try to do the same there.
> 
> So, just to make sure everything’s covered: this new mechanism will
> allow QEMU to have the guest allocate an arbitrary blob of memory (in
> the form of a fw_cfg file),

Yes.

> and will get an offset within guest memory
> in return (via another fw_cfg file).

Yes.

The information that QEMU will receive is the LE-encoded 8-byte GPA
(guest-physical address) of the allocation carried out by the firmware.

> It can then translate the guest
> offset into a host address

Yes.

I'm a bit rusty on the exact QEMU memory APIs here, but I think that
ram_addr_t is *not* the right source address type to convert from (to
HVA, host virtual address).

I.e., what the firmware returns should *not* be considered a ram_addr_t;
IIRC, ram_addr_t is a linear offset into RAMBlocks that does not take,
for example, the 32-bit PCI MMIO hole into account.

Instead, the value written by the firmware should be considered a
hwaddr. (See "include/exec/hwaddr.h".) Therefore you'll have to find a
translation from hwaddr to HVA.

(I'm sure seasoned QEMU developers will correct me on the APIs / src
address type if necessary.)

> and update the blob at will.

Yes.

(And then Igor said something about dirtying that memory after the QEMU
write...)

> Is this
> correct, because that’s what we need for VM Generation ID?

Yes, it seems correct to me.

Thanks
Laszlo

> 
>> If someone is interested, I'll remind us that EFI_ACPI_TABLE_PROTOCOL
>> installs *copies* of ACPI tables, out of the fw_cfg blobs that were
>> downloaded. Therefore OVMF tracks all ADD_POINTER commands that point
>> into fw_cfg blobs, and if an fw_cfg blob is not pointed-to by *any*
>> ADD_POINTER command, or else *all* ADD_POINTER commands that point into
>> it point to ACPI table headers, then OVMF will ultimately free the
>> originally downloaded fw_cfg blob. If there is at least one referring
>> ADD_POINTER command that points to non-ACPI-table data within the blob,
>> then OVMF marks the blob to be preserved permanently, in AcpiNVS type
>> memory.
>>
>> By introducing the above ALLOCATE_RETURN_ADDR command type, OVMF can do
>> two things:
>>
>> (a) immediately mark the blob for permanent preservation (i.e., it won't
>>    be released after the script processing is complete),
>> (b) write the actual allocation address back to the appropriate fw_cfg
>>    file.
>>
>> For SeaBIOS, only (b) matters -- because it doesn't install *copies* of
>> ACPI tables; it rather keeps everything in place, as originally
>> allocated, and it just links things together --, but
>> ALLOCATE_RETURN_ADDR as proposed above should be suitable for SeaBIOS
>> just as well.
>>
>> Thanks
>> Laszlo
> —Ben
>