Re: [Qemu-devel] [PATCH v5 05/10] ACPI: Add Virtual Machine Generation ID support

[Laszlo Ersek]

On 02/08/17 12:04, Igor Mammedov wrote:
> On Wed, 8 Feb 2017 01:48:42 +0100
> Laszlo Ersek <address@hidden> wrote:

[snip]

>> (2) Structure of the vmgenid fw_cfg blob, AKA "why that darn offset 40
>> decimal".
>>
>> I explained it under points (6) and (7) in the following message:
>>
>> Message-Id: <address@hidden>
>> URL: https://www.mail-archive.com/address@hidden/msg425226.html
>>
>> The story is that *wherever* an ADD_POINTER command points to -- that
>> is, at the *exact* target address --, OVMF will look for an ACPI table
>> header, somewhat heuristically. If it finds a byte pattern that (a) fits
>> into the remaining blob and (b) passes some superficial ACPI table
>> header checks, such as length and checksum, then OVMF assumes that the
>> blob contains an ACPI table there, and passes the address (again, the
>> exact, relocated, absolute target address of ADD_POINTER) to
>> EFI_ACPI_TABLE_PROTOCOL.InstallAcpiTable().
>>
>> We want to disable this heuristic for the vmgenid blob. *If* the blob
>> contained only 16 bytes (for the GUID), then the heuristic would
>> automatically disable itself, because the ACPI table header (36 bytes)
>> is larger than 16 bytes, so OVMF wouldn't even look. However, for the
>> caching and other VMGENID requirements, we need to allocate a full page
>> with the blob (4KB), hence OVMF *will* look. If we placed the GUID right
>> at the start of the page, then OVMF would sanity-check it as an ACPI
>> table header. The check would *most likely* not pass, so things would be
>> fine in practice, but we can do better than that: just put 40 zero bytes
>> at the front of the blob.
>>
>> And this is why the ADD_POINTER command has to point to the beginning of
>> the blob (i.e., 36 zero bytes), to "suppress the OVMF ACPI SDT header
>> detection". (The other 4 bytes are for arriving at an address divisible
>> by 8, which is a VMGENID requirement for the GUID.)
> that's deserves a comment in code/doc as it's non obvious to someone
> who is not familiar with OVMF.

It is mentioned in the document "docs/specs/vmgenid.txt", added in patch 3.

I didn't want to push for more details there, but if you think it's
helpful or even required, then Ben should please simply add a

Rationale for padding at the front (OVMF SDT Header probe suppressor)
---------------------------------------------------------------------

subsection under the

GUID Storage Format
-------------------

section, and just dump the above into it.

Maybe tone down the over-use of *bold*. :)

> 
> 
>> The consequence is that both the ADDR method and QEMU's guest memory
>> access code have to add 40 manually.
>>
>>
>> (3) Whether the VGIA named object should be a DWORD or a QWORD in ACPI.
>> Personally, I'm fine with either, but I see that DWORD is more
>> compatible with old OSes.
> It's not only old OSes, but newer ones as well (32 bit and to some degree 
> 64-bit).
> We should not put QWORD objects in global scope until we switch
> DSDT rev to 2.0 or higher AND ready to drop support for ACPI 1.0 guests.
> DSDT rev is important for 64-bit guests as well as OSPM chooses
> supported integer length based on it.

Good point. DWORD is fine with me.

[snipping the rest, nothing for me to add there, it seems]

Thanks!
Laszlo