On 17 February 2016 at 16:11, Laszlo Ersek <[email protected]> wrote: > On 02/17/16 15:48, Ard Biesheuvel wrote: >> AARCH64 systems never require compatibility with legacy ACPI OSes, and >> may not have any 32-bit addressable system RAM. To support ACPI on these >> systems, we need to be able to relax the 4 GB allocation restriction. >> >> So add a PCD PcdAcpiAllocateTablesBelow4GB defaulting to TRUE, and wire >> it up to the memory allocation calls in AcpiTableDxe/AcpiTableProtocol.c >> >> Note that this will inhibit the publishing of any tables that carry only >> 32-bit addresses, i.e., RSDPv1, RSDTv1 and RSDTv3. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Ard Biesheuvel <[email protected]> >> --- >> MdeModulePkg/MdeModulePkg.dec | 7 + >> MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf | 2 + >> MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 447 >> ++++++++++++-------- >> 3 files changed, 270 insertions(+), 186 deletions(-) > > Somewhat independently of your patch, I agree 100% that the interface > exposed by EFI_ACPI_TABLE_PROTOCOL is seriously lacking. It doesn't > allow the caller to control the allocation limit (--> see your patch), > nor the UEFI memory type that the copy of the table being installed will > be placed into (it would be valid for the caller to choose between > AcpiReclaim vs. AcpiNVS), nor the question whether the table being > installed should be linked into the RSDT, XSDT, or both. > > I don't find this an implementation problem; this is a problem with the > specification. In particular, in October last year I cross-posted an > email to the USWG and ASWG lists, entitled > > LoadTable vs. EFI_ACPI_TABLE_PROTOCOL > > In that email, I pointed out that the current edk2 implementation of > EFI_ACPI_TABLE_PROTOCOL makes it impossible to support the LoadTable > ACPI operator. Namely, the LoadTable operator requires the definition > block (= AML code) being loaded dynamically to come *from* AcpiNVS type > memory, but edk2's implementation doesn't allow the caller to install > tables into AcpiNVS type memory, *except* those with the signature > "UEFI" -- those however are *data* tables, not definition blocks (= AML > code, like SSDT or DSDT). > > For this reason, I proposed EFI_ACPI_TABLE2_PROTOCOL: > >> - same set of member functions >> - UninstallAcpiTable() works the same >> - InstallAcpiTable() gets a new parameter called MemoryType: >> >> typedef >> EFI_STATUS >> (EFIAPI *EFI_ACPI_TABLE2_INSTALL_ACPI_TABLE) ( >> IN EFI_ACPI_TABLE2_PROTOCOL *This, >> IN VOID *AcpiTableBuffer, >> IN UINTN AcpiTableBufferSize, >> IN EFI_MEMORY_TYPE *MemoryType OPTIONAL, >> OUT UINTN *TableKey >> ); >> >> - If MemoryType is NULL, then the new interface works identically to >> the preexistent one. >> - Otherwise, if EFI_ACPI_TABLE2_PROTOCOL can prove that *MemoryType is >> invalid for the signature detected in AcpiTableBuffer, then >> EFI_INVALID_PARAMETER is returned. >> - Otherwise, the copy of AcpiTableBuffer is allocated from *MemoryType >> memory. >> >> This would allow tables with any (non-standard) signatures to be >> placed in reserved or AcpiNVS memory, regardless of contents. > > My query to the USWG & ASWG garnered exactly *zero* replies. I guess > nobody cared about (or understood) my problem. > > Now I absolutely view your use case as a further parameter that > "EFI_ACPI_TABLE2_PROTOCOL" should expose. From that POV, I support your > use case. > > Regarding the implementation, I guess you are not enthusiastic about > turning this question into a standardization effort. (I tried and > failed.) So, whatever works for you -- I won't review the code, but as > long as it can remain compatible with current clients, given the right > PCD settings, I absolutely support the idea. >
You may be right about the lacking EFI_ACPI_PROTOCOL interface, but the hardcoded 4 GB limit is simply an implementation detail, and should be treated as such imo. > (QEMU currently depends on tables being allocated under 4GB, even for > aarch64 guests, as far as I recall.) > Yes, I wondered about that. After the tables are installed, a 16 KB reserved region remains in the UEFI memory map, Does that contain any live payload? > Digressing a bit, I'll also mention that I found out (and reported) the > hard way that Microsoft's ACPI.SYS does not implement the > DataTableRegion operator. (ACPI CA does.) That operator is supposed to > be extremely powerful, it is not optional, and it dates back to the ACPI > 2.0 specification -- which Microsoft co-authored. > I remember that thread :-) Thanks, Ard. _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
