Re: [Xen-devel] [PATCH v2 03/23] acpi/hvmloader: Initialize vm_gid data outside ACPI code

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 09/08/16 15:31, Jan Beulich wrote:
>>>> On 09.08.16 at 15:50, <boris.ostrovsky@xxxxxxxxxx> wrote:
>> On 08/09/2016 09:11 AM, Jan Beulich wrote:
>>>>>> On 04.08.16 at 23:06, <boris.ostrovsky@xxxxxxxxxx> wrote:
>>>> --- a/tools/firmware/hvmloader/acpi/build.c
>>>> +++ b/tools/firmware/hvmloader/acpi/build.c
>>>> @@ -462,32 +462,26 @@ static int construct_secondary_tables(unsigned long 
>>>> *table_ptrs,
>>>>   *
>>>>   * Return 0 if memory failure, != 0 if success
>>>>   */
>>>> -static int new_vm_gid(struct acpi_info *acpi_info)
>>>> +static int new_vm_gid(struct acpi_config *config,
>>>> +                      struct acpi_info *info)
>>>>  {
>>>> -    uint64_t vm_gid[2], *buf;
>>>> -    const char * s;
>>>> -    char *end;
>>>> -
>>>> -    acpi_info->vm_gid_addr = 0;
>>>> -
>>>> -    /* read ID and check for 0 */
>>>> -    s = xenstore_read("platform/generation-id", "0:0");
>>>> -    vm_gid[0] = strtoll(s, &end, 0);
>>>> -    vm_gid[1] = 0;
>>>> -    if ( end && end[0] == ':' )
>>>> -        vm_gid[1] = strtoll(end+1, NULL, 0);
>>>> -    if ( !vm_gid[0] && !vm_gid[1] )
>>>> +    uint64_t *buf;
>>>> +
>>>> +    info->vm_gid_addr = 0;
>>>> +
>>>> +    /* check for 0 ID*/
>>>> +    if ( !config->vm_gid[0] && !config->vm_gid[1] )
>>>>          return 1;
>>>>  
>>>>      /* copy to allocate BIOS memory */
>>>> -    buf = (uint64_t *) mem_alloc(sizeof(vm_gid), 8);
>>>> +    buf = mem_alloc(sizeof(config->vm_gid), 8);
>>>>      if ( !buf )
>>>>          return 0;
>>>> -    memcpy(buf, vm_gid, sizeof(vm_gid));
>>>> +    memcpy(buf, config->vm_gid, sizeof(config->vm_gid));
>>>>  
>>>> -    /* set into ACPI table and HVM param the address */
>>>> -    acpi_info->vm_gid_addr = virt_to_phys(buf);
>>>> -    hvm_param_set(HVM_PARAM_VM_GENERATION_ID_ADDR, 
>>>> acpi_info->vm_gid_addr);
>>>> +    /* set the address into ACPI table and also pass it back to the 
>>>> caller */
>>>> +    info->vm_gid_addr = virt_to_phys(buf);
>>> What consumer of this value is left? Can't that field go away now?
>> In acpi_info? TBH, I don't know, I thought this was introduced to deal
>> with something in Windows (commit 978cc62b looks generic but then
>> c5a29a87 is for Windows).
> Sure, but the need for the field appears to go away with the
> introduction of the one in acpi_config.

The way genid needs to work is this:

1) Allocate some memory, reserving it in the e820
2) Represents that physical address in the ACPI tables for the guest to
consume
3) Update Xenstore with the physical address, for the toolstack to consume

So long as HVMLoader continues to function in that way, it is fine.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel