qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Ben Warren
Subject: Re: [Qemu-devel] [PATCH v5 05/10] ACPI: Add Virtual Machine Generation ID support
Date: Thu, 9 Feb 2017 12:02:30 -0800

> On Feb 9, 2017, at 11:27 AM, Laszlo Ersek <address@hidden> wrote:
> 
> On 02/09/17 18:23, Igor Mammedov wrote:
>> On Wed, 8 Feb 2017 01:48:42 +0100
>> Laszlo Ersek <address@hidden> wrote:
>> 
>>> On 02/05/17 10:12, address@hidden wrote:
>>>> From: Ben Warren <address@hidden>
>> [...]
>> 
>>> (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.)
>>> 
>>> The consequence is that both the ADDR method and QEMU's guest memory
>>> access code have to add 40 manually.
>> The longer I look "suppress the OVMF ACPI SDT header detection",
>> the less I like approach.
>> 
>> It looks somewhat backwards where a firmware forces QEMU
>> to use non obvious offsets to workaround OVMF ACPI table detection
>> heuristics.
> 
> This is for historical reasons -- when the linker/loader commands were
> invented, it wasn't considered that in UEFI, ACPI tables cannot just be
> linked together in-place, instead they'd have to be passed to
> EFI_ACPI_TABLE_PROTOCOL.InstallAcpiTable() one by one, for copying. The
> commands didn't provide dedicated means for identifying individual
> tables in blobs. Hence the heuristics built upon ADD_POINTER.
> 
> And once you have heuristics, you want to suppress them occasionally, if
> you can find a way.
> 
>> Can we add and use explicit flag to mark blobs as ACPI tables,
>> so that OVMF won't have to guess whether to hand off table
>> as ACPI to UEFI stack or just keep it to yourself?
> 
> The ADD_POINTER-based heuristics cannot be turned off for ACPI table
> identification, because a single fw_cfg blob can (and does) contain
> multiple ACPI tables, and OVMF needs to figure out, somehow, where each
> of those tables start. Blob-level hints won't help with this.
> 
> The following could be an improvement though: a blob-level hint (perhaps
> in the ALLOCATE command) that the blob contains *no* ACPI tables. In
> this case, OVMF could turn off the table recognition heuristics for
> those ADD_POINTER commands that point into such blobs. (Plus mark the
> blob for permanent preservation at once, and not only when an
> ADD_POINTER "probe" into the blob fails.)
> 
> OVMF currently ignores the Zone field in the ALLOCATE command, so that
> could be extended / abused for such a hint, without breaking
> compatibility with OVMF. (Not sure about SeaBIOS.)
> 
Overloading the ALLOCATE command in theory could be done with a simple change 
to SeaBIOS.  Here’s where the zone is handled:

    switch (entry->alloc_zone) {
        case ROMFILE_LOADER_ALLOC_ZONE_HIGH:
            zone = &ZoneHigh;
            break;
        case ROMFILE_LOADER_ALLOC_ZONE_FSEG:
            zone = &ZoneFSeg;
            break;
        default:
            goto err;
    }
 
ZONE_HIGH = 1, and ZONE_FSEG = 2 and alloc_zone is 8 bits.  Stealing the MSB 
and masking it off would be dirty, but could work.


> Otherwise, a new allocation command will be necessary. (Which should
> embed the current ALLOCATE command structure fully, at some offset.)
> 
> Thanks
> Laszlo

Attachment: smime.p7s
Description: S/MIME cryptographic signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]