XSDT is introduced in ACPI2.0. ACPI1.0/ACPI1.0b only defined 32bit address range. ACPI2.0 resolved 4G issue immediately, but in order to keep compatibility, most platform produced both DSDT and XSDT, and with FADT1.0 and FADT2.0, at that time.
For now, I believe most OSes support ACPI2.0 and above. So it should be safe. Thank you Yao Jiewen -----Original Message----- From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] Sent: Thursday, February 18, 2016 11:15 PM To: Yao, Jiewen Cc: Tian, Feng; Graeme Gregory; edk2-devel@lists.01.org; Leif Lindholm; Gao, Liming; Laszlo Ersek; Zeng, Star Subject: Re: [edk2] [RFC PATCH] MdeModulePkg: AcpiTableDxe: make 4 GB table allocation limit optional On 18 February 2016 at 16:07, Yao, Jiewen <jiewen....@intel.com> wrote: > Or PcdAcpiSupportVersion, bit0 as ACPI1.0. > Bit0 set means allocation Below4G. > Bit0 clear means allocation from everywhere. > That is also fine with me. But I think it was ACPI 5.0 that introduced XSDT, right? So if you clear this bit, you would expect a ACPI system compatible with a version older than 5.0 to still work, but with allocations above 4 GB and no RSDT, only XSDT, would that be ok? So perhaps it should define the lowest supported ACPI level, with legal values of '1' and '5' for the moment, and we can always expand it later if we need to. > > -----Original Message----- > From: Yao, Jiewen > Sent: Thursday, February 18, 2016 11:05 PM > To: Ard Biesheuvel > Cc: Tian, Feng; Graeme Gregory; edk2-devel@lists.01.org; Leif > Lindholm; Gao, Liming; Laszlo Ersek; Zeng, Star; Yao, Jiewen > Subject: RE: [edk2] [RFC PATCH] MdeModulePkg: AcpiTableDxe: make 4 GB > table allocation limit optional > > Thanks to detail explain. I understand and I agree with you that > fragmentation is problem. > So basically, I agree on PCD solution to choose preferred location of > allocation. > > Maybe you can consider adding fragmentation issue as description to help more > people on why we need this PCD. > > I also think the PCD name - do we consider PcdAcpi1Support, or similar? > People need strong tech background to choose why and if he wants Below4GB or > not. Using ACPI1.0 as PCD might be better, because if ACPI1.0 is not POR, it > can be dropped. It is more clear. > > Just like SMBIOS table, we use PcdSmbiosEntryPointProvideMethod, instead of > Below4GB. > > Thank you > Yao Jiewen > > > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Ard Biesheuvel > Sent: Thursday, February 18, 2016 10:39 PM > To: Yao, Jiewen > Cc: Tian, Feng; Graeme Gregory; edk2-devel@lists.01.org; Leif > Lindholm; Gao, Liming; Laszlo Ersek; Zeng, Star > Subject: Re: [edk2] [RFC PATCH] MdeModulePkg: AcpiTableDxe: make 4 GB > table allocation limit optional > > On 18 February 2016 at 15:20, Yao, Jiewen <jiewen....@intel.com> wrote: >> Hi >> Thanks to bring this 4G issue again. I have several thought for your >> consideration. >> >> 1) At 12/16/2015, Samer El-Haj-Mahmoud <samer.el-haj-mahm...@hpe.com> >> submitted a patch: >> [edk2] [PATCH] IntelFrameworkModulePkg : Allow ACPI tables to get >> installed above 4GB >> >> Some ARM systems do not have available memory below 4GB, and still support >> ACPI. This patch allows the tables to get loaded above 4GB if the allocation >> below 4GB fails. >> >> I have given comments that: >> -- I guess we need similar fix for MdeModulePkg\Universal\Acpi\AcpiTableDxe. >> > > Indeed :-) > >> -- Can we create a new function to allocate <4G at first, if fail, allocate >> any memory? >> Then we can update all caller to consume this new function. >> We can avoid adding ERROR check anywhere. I also suggest we need add some >> debug message there, we can add into one place. >> >> It is just another option to resolve ARM issue. Currently, we only add PCD >> when necessary, because we got complain that we defined too many PCDs. >> If there is a way to avoid introducing a new one. It worth considering. >> > > No, this is not a good idea. This still assumes that allocating below > 4 GB is preferred if possible, and that is simply not true on AArch64. > > Allocating below 4 GB if there is space there will result in some UEFI > regions at the top of memory, and some regions below 4 GB. This needlessly > fragments the address space, and results in reduced TLB efficiency, since > ACPI tables are not mapped by default, and the [3 GB, 4 GB] interval can no > longer be mapped using a single 1 GB block entry. Also, memory below 4 GB > needs to be reserved for devices that can only do 32-bit DMA. > >> 2) I believe 4G table issue not only exist in ACPI table, but also exist in >> others. For example, Smbios table. >> SMBIOS 3.0 spec allows table be loaded >4G memory, while SMBIOS 2.X has 4G >> limitation. >> >> I know ACPI table and SMBIOS table are different. Some platforms care only >> one of them. >> Personally, I hope we can have a consistent way to resolve this generic 4G >> issue. >> >> I have seems SMBIOS table is using the way Samer submitted in last >> year. (Allocate <4G at first, if fail, allocate at any place) Do you see any >> limitation on this algorithm? >> > > This only happens if you have lower than version 3.0 or the > PcdSmbiosEntryPointProvideMethod has bit 0 set. On AARCH64 systems, we > usually only enable the v3.0 entry point, and clear bit 0 in this PCD > >> I am not insist on some solutions. I just want to bring the other way for >> discussion. >> >> BTW: I am not clear on memory map on >4G ARM platform. Can someone help me >> understand where is runtime code/runtime data/ACPI NVS/Reserved memory? >4G >> or <4G? >> > > Sorry to be pedantic, but AARCH64 is an architecture, not a platform. > Every AARCH64 platform has complete freedom as to where peripherals live and > where system RAM lives, and some platforms have their RAM starting at > 0x80_0000_0000 So every such region could be anywhere in the address space. > > >> -----Original Message----- >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf >> Of Ard Biesheuvel >> Sent: Thursday, February 18, 2016 1:42 AM >> To: Laszlo Ersek >> Cc: Tian, Feng; Graeme Gregory; edk2-devel@lists.01.org; Leif >> Lindholm; Gao, Liming; Zeng, Star >> Subject: Re: [edk2] [RFC PATCH] MdeModulePkg: AcpiTableDxe: make 4 GB >> table allocation limit optional >> >> On 17 February 2016 at 18:40, Laszlo Ersek <ler...@redhat.com> wrote: >>> On 02/17/16 18:23, Ard Biesheuvel wrote: >>>> On 17 February 2016 at 18:07, Graeme Gregory <graeme.greg...@linaro.org> >>>> wrote: >>>>> >>>>> >>>>> On 17/02/2016 16:04, Laszlo Ersek wrote: >>>>>> >>>>>> On 02/17/16 16:34, Ard Biesheuvel wrote: >>>>>>> >>>>>>> On 17 February 2016 at 16:11, Laszlo Ersek <ler...@redhat.com> 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 <ard.biesheu...@linaro.org> >>>>>>>>> --- >>>>>>>>> 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. >>>>>> >>>>>> Hm, maybe, maybe not. >>>>>> >>>>>> Namely, RSDT cannot reference tables that are located above 4GB, >>>>>> and the UEFI spec says, quote, >>>>>> >>>>>> EFI_ACPI_TABLE_PROTOCOL.InstallAcpiTable() >>>>>> >>>>>> [...] >>>>>> >>>>>> The InstallAcpiTable() function allows a caller to install an ACPI >>>>>> table. The ACPI table may either by a System Description Table or >>>>>> the FACS. For all tables except for the DSDT and FACS, a copy of >>>>>> the table will be linked by the RSDT/XSDT. For the FACS and DSDT, >>>>>> the pointer to a copy of the table will be updated in the FADT, if >>>>>> present. >>>>> >>>>> Surely this is a simple case of the spec not being clear that >>>>> RSDT/XDST means RSDT or XSDT as appropriate? >>>>> >>>>> RSDT is optional for systems in ACPI 5.1+ specs. >>>>> >>>> >>>> Indeed. >>>> >>>>>> [...] >>>>>> >>>>>> On successful output, the EFI_ACPI_TABLE_PROTOCOL will ensure that >>>>>> the checksum field is correct for both the RSDT/XSDT table and the >>>>>> copy of the table being installed that is linked by the RSDT/XSDT. >>>>>> >>>>>> [...] >>>>>> >>>>>> That is, following the *letter* of the spec, linkage into the >>>>>> RSDT is >>>>>> *not* optional. And that requirement can only be satisfied if the >>>>>> tables are located under 4GB. >>>>>> >>>> >>>> No, it is 'linkage into the RSDT/XSDT' that is not optional. In the >>>> quote above, '..both the RSDT/XDST and ...' does not refer to both >>>> RSDT and XSDT but to the RSDT/XSDT on the one hand and the copy of >>>> the table being installed on the other. >>> >>> I agree with you on the scope of the words "both ... and ...". >>> >>> I disagree with your interpretation of the slash character between >>> the words "RSDT" and "XSDT". I believe you say "RSDT and XSDT are >>> alternatives, and it's platform prerogative to pick one (or both)" >>> -- however I can't deduce that from the spec, and the current >>> reference implementation doesn't seem to support it either >>> (otherwise your patch might not be necessary). >>> >>> "Interpretation required", I guess. >>> >> >> Well, the wording is obviously not unambiguous :-) >> >>> Anyway, please be aware that I'm not trying to block this patch at all. >>> As long as the current behavior remains available, I'm all good. >>> >> >> Absolutely. >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
