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 > > wrote:
>>
>> On 01/19/17 08:09, Ben Warren wrote:
>>>
 On Jan 18, 2017, at 4:02 PM, Ben Warren >
 wrote:

 Hi Michael,
> On Jan 17, 2017, at 9:45 AM, Michael S. Tsirkin  >
> 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 

Re: [Qemu-devel] Virtual Machine Generation ID

[Ben Warren]

Thanks Laszlo!
> On Jan 19, 2017, at 1:25 AM, Laszlo Ersek  wrote:
> 
> On 01/19/17 08:09, Ben Warren wrote:
>> 
>>> On Jan 18, 2017, at 4:02 PM, Ben Warren 
>>> wrote:
>>> 
>>> Hi Michael,
 On Jan 17, 2017, at 9:45 AM, Michael S. Tsirkin 
 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 

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 
>> wrote:
>>
>> Hi Michael,
>>> On Jan 17, 2017, at 9:45 AM, Michael S. Tsirkin 
>>> 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 

Re: [Qemu-devel] Virtual Machine Generation ID

[Ben Warren]

> On Jan 18, 2017, at 4:02 PM, Ben Warren  wrote:
> 
> Hi Michael,
>> On Jan 17, 2017, at 9:45 AM, Michael S. Tsirkin  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
> 



smime.p7s
Description: S/MIME cryptographic signature

Re: [Qemu-devel] Virtual Machine Generation ID

[Ben Warren]

Hi Michael,
> On Jan 17, 2017, at 9:45 AM, Michael S. Tsirkin  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



smime.p7s
Description: S/MIME cryptographic signature

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

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]

> On Jan 17, 2017, at 7:21 AM, Michael S. Tsirkin  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



smime.p7s
Description: S/MIME cryptographic signature

Re: [Qemu-devel] Virtual Machine Generation ID

[Igor Mammedov]

On Tue, 17 Jan 2017 07:01:27 -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?
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  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  
> >> wrote:  
> >> > On Mon, 16 Jan 2017 10:57:42 -0800
> >> > Ben Warren  wrote:
> >> >  
> >> >> > On Jan 16, 2017, at 6:21 AM, Michael S. Tsirkin  
> >> >> > 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  
> >> >> >> 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 
> >> >> >> 
> >> >> >>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 < 
> >> >> >>  
> >> >> >>m...@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 

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

[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  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  wrote:
>> > On Mon, 16 Jan 2017 10:57:42 -0800
>> > Ben Warren  wrote:
>> >
>> >> > On Jan 16, 2017, at 6:21 AM, Michael S. Tsirkin  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  
>> >> >> 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 
>> >> >> 
>> >> >>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 <
>> >> >>m...@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? 

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

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

Re: [Qemu-devel] Virtual Machine Generation ID

[Igor Mammedov]

On Mon, 16 Jan 2017 10:57:42 -0800
Ben Warren  wrote:

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

Re: [Qemu-devel] Virtual Machine Generation ID

[Ben Warren]

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

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  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 
> 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 <
> m...@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

[Igor Mammedov]

On Sat, 14 Jan 2017 22:17:53 -0800
Ben Warren  wrote:

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

[Ben Warren]

Hi Michael,

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

smime.p7s
Description: S/MIME cryptographic signature

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  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  
> >> 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   0x007C (124)
>  * Revision 0x01
>  * Checksum 0x29
>  * OEM ID   "BOCHS "
>  * OEM Table ID "VMGENID"
>  * OEM Revision 0x0001 (1)
>  * Compiler ID  "BXPC"
>  * Compiler Version 0x0001 (1)
>  */
> DefinitionBlock ("SSDT.aml", "SSDT", 1, "BOCHS ", "VMGENID", 0x0001)
> {
> 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?  
> 

Re: [Qemu-devel] Virtual Machine Generation ID

[Ben Warren]

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  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  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   0x007C (124)
 * Revision 0x01
 * Checksum 0x29
 * OEM ID   "BOCHS "
 * OEM Table ID "VMGENID"
 * OEM Revision 0x0001 (1)
 * Compiler ID  "BXPC"
 * Compiler Version 0x0001 (1)
 */
DefinitionBlock ("SSDT.aml", "SSDT", 1, "BOCHS ", "VMGENID", 0x0001)
{
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

smime.p7s
Description: S/MIME cryptographic signature

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  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

[Igor Mammedov]

On Tue, 4 Oct 2016 15:51:40 -0700
Ed Swierk  wrote:

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

[Ed Swierk]

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

[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

[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