Re: [Qemu-devel] [PATCH v4 10/20] hw/arm/virt-acpi-build: Generate RSDT table

[Laszlo Ersek]

On 04/09/15 18:03, Peter Maydell wrote:
> On 9 April 2015 at 17:00, Laszlo Ersek <address@hidden> wrote:
>> On 04/09/15 15:59, Peter Maydell wrote:
>>> On 9 April 2015 at 14:51, Igor Mammedov <address@hidden> wrote:
>>>> On Thu, 9 Apr 2015 14:27:58 +0100
>>>> Peter Maydell <address@hidden> wrote:
>>>>
>>>>> On 9 April 2015 at 14:17, Igor Mammedov <address@hidden> wrote:
>>>>>> On Thu, 09 Apr 2015 13:50:52 +0100
>>>>>> Alex Bennée <address@hidden> wrote:
>>>>>>
>>>>>>>
>>>>>>> Shannon Zhao <address@hidden> writes:
>>>>>>>> +    for (i = 0; i < table_offsets->len; ++i) {
>>>>>>>> +        /* rsdt->table_offset_entry to be filled by Guest linker */
>>>>>>>> +        bios_linker_loader_add_pointer(linker,
>>>>>>>> +                                       ACPI_BUILD_TABLE_FILE,
>>>>>>>> +                                       ACPI_BUILD_TABLE_FILE,
>>>>>>>> +                                       table_data, 
>>>>>>>> &rsdt->table_offset_entry[i],
>>>>>>>> +                                       sizeof(uint32_t));
>>>>>>>
>>>>>>> Why are these pointers always 32 bit? Can they ever be 64 bit?
>>>>>> Laszlo, can you confirm that UEFI puts APCI tables below 4G address
>>>>>> space?
>>
>> I confirmed that before, in the v2 discussion:
>>
>> http://thread.gmane.org/gmane.comp.emulators.qemu/316670/focus=317560
>>
>> But in fact the RSDT / XSDT that QEMU exports for UEFI doesn't matter.
> 
> If this table is never used, presumably we should just
> not generate it at all, then?

Unfortunately, this is not the case. In order to identify ACPI tables *at all* 
in UEFI, I need "relocate pointer" commands for pointers that point to those 
tables. And those pointers must *reside* somewhere, in some blob.

Here's how the "relocate pointer" command is defined in edk2 
(OvmfPkg/AcpiPlatformDxe/QemuLoader.h):

//
// QemuLoaderCmdAddPointer: the bytes at
// [PointerOffset..PointerOffset+PointerSize) in the file PointerFile contain a
// relative pointer (an offset) into PointeeFile. Increment the relative
// pointer's value by the base address of where PointeeFile's contents have
// been placed (when QemuLoaderCmdAllocate has been executed for PointeeFile).
//
typedef struct {
  UINT8  PointerFile[QEMU_LOADER_FNAME_SIZE]; // NUL-terminated
  UINT8  PointeeFile[QEMU_LOADER_FNAME_SIZE]; // NUL-terminated
  UINT32 PointerOffset;
  UINT8  PointerSize;                         // one of 1, 2, 4, 8
} QEMU_LOADER_ADD_POINTER;

In the qemu tree, see COMMAND_ADD_POINTER in "hw/acpi/bios-linker-loader.c", 
for the same. (I rewrote the types and the comments in edk2 from scratch, both 
for coding style reasons and for clearer documentation.)

... To be clear: the top-level pointers must exist somewhere (in some blob), 
because that helps edk2 find the tables (in some other blobs). However, the 
top-level pointers themselves don't need to reside in any ACPI table (RSDT, 
XSDT); they can just live in an otherwise unreferenced portion of one of the 
blobs.

But, IMO, implementing that wouldn't be much easier (and it would certainly be 
uglier) than composing a correct RSDT or XSDT. The latter would also keep the 
similarity with the x86 SeaBIOS case (where the RSDT is a hard requirement).

Thanks
Laszlo