Hi Ray,
Thanks for review, here are some of my thoughts:
Let me understand, 'cache setting' means does it access a certain address(probably a memory address) via cache? If so, I'd say the 'cache setting' should be a part of attributes.Chao, Since the lib class is so general, I'd like to understand more details to make sure it can properly fit into any CPU arch. In X86, cache setting is through MSRs and Page tables, and memory access control (read-only, not-present, non-executable) is through page tables.
In LoongArch64, cache settings and memory access control are performed via page tables. Please check the patch 14 of this series.This CpuMmuLib is to provide both services. How does LoongArch64 manage the cache settings and memory access control? Is it proper to combine both services into one lib?
Yes, I think Attribute's instance family can be bear the memory access and cache setting. So what are you suggestions if we improve the lib API?If the backend silicon IP is the same one that supports the "one" lib design, can we refine the lib API a bit?
Do you means the (Set|Get) merge together(differentiate Get or Set operations by parameters)? If so, I think it's OK, but maybe some existing instances will be modified together.We have (Set|Get)MemoryRegionAttribute() and (Set|Clear)MemoryRegion(NoExec|ReadOnly). Can we merge them together?
Yes, currently, no one helps construct MEMORY_REGION_DESCRIPTOR. I think the construction of descriptors is not part of the API, it should be the localized or private when I design them. Do I need to add an API to construct descripters?And the API ConfigureMemoryManagementUint() accepts MEMORY_REGION_DESCRIPTOR but none of other APIs helps to construct the descriptor.
Yes, the APIs looks like so fragmented and we should improve them. So we should talk more about this API, thanks.It seems to me the MmuLib is simply a combination of different random APIs. It's not a well-designed library class. We need more discussion to make it be able to be accommodated by other archs in future, at least by figuring out the path of X86, ARM.
Thanks, Ray-----Original Message----- From: Chao Li <lichao@loongson.cn> Sent: Friday, November 17, 2023 6:00 PM To: devel@edk2.groups.io Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Leif Lindholm <quic_llindhol@quicinc.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Sami Mujawar <sami.mujawar@arm.com>; Sunil V L <sunilvl@ventanamicro.com>; Warkentin, Andrei <andrei.warkentin@intel.com> Subject: [PATCH v3 13/39] UefiCpuPkg: Add CpuMmuLib.h to UefiCpuPkg Add a new header file CpuMmuLib.h, whitch is referenced from ArmPkg/Include/Library/ArmMmuLib.h. Currently, only support for LoongArch64 is added, and more architectures can be accommodated in the future. BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4584 Cc: Eric Dong <eric.dong@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Leif Lindholm <quic_llindhol@quicinc.com> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org> Cc: Sami Mujawar <sami.mujawar@arm.com> Cc: Sunil V L <sunilvl@ventanamicro.com> Cc: Andrei Warkentin <andrei.warkentin@intel.com> Signed-off-by: Chao Li <lichao@loongson.cn> --- UefiCpuPkg/Include/Library/CpuMmuLib.h | 155 +++++++++++++++++++++++++ UefiCpuPkg/UefiCpuPkg.dec | 4 + 2 files changed, 159 insertions(+) create mode 100644 UefiCpuPkg/Include/Library/CpuMmuLib.h diff --git a/UefiCpuPkg/Include/Library/CpuMmuLib.h b/UefiCpuPkg/Include/Library/CpuMmuLib.h new file mode 100644 index 0000000000..23b2fe34ac --- /dev/null +++ b/UefiCpuPkg/Include/Library/CpuMmuLib.h @@ -0,0 +1,155 @@ +/** @file + + Copyright (c) 2023 Loongson Technology Corporation Limited. All rights reserved.<BR> + + SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + +#ifndef CPU_MMU_LIB_H_ +#define CPU_MMU_LIB_H_ + +#include <Uefi/UefiBaseType.h> + +#define EFI_MEMORY_CACHETYPE_MASK (EFI_MEMORY_UC | \ + EFI_MEMORY_WC | \ + EFI_MEMORY_WT | \ + EFI_MEMORY_WB | \ + EFI_MEMORY_UCE \ + ) + +typedef struct { + EFI_PHYSICAL_ADDRESS PhysicalBase; + EFI_VIRTUAL_ADDRESS VirtualBase; + UINTN Length; + UINTN Attributes; +} MEMORY_REGION_DESCRIPTOR; + +/** + Converts EFI Attributes to corresponding architecture Attributes. + + @param[in] EfiAttributes Efi Attributes. + + @retval Corresponding architecture attributes. +**/ +UINTN +EfiAttributeConverse ( + IN UINTN EfiAttributes + ); + +/** + Finds the length and memory properties of the memory region corresponding to the specified base address. + + @param[in] BaseAddress To find the base address of the memory region. + @param[in] EndAddress To find the end address of the memory region. + @param[out] RegionLength The length of the memory region found. + @param[out] RegionAttributes Properties of the memory region found. + + @retval EFI_SUCCESS The corresponding memory area was successfully found + EFI_NOT_FOUND No memory area found +**/ +EFI_STATUS +GetMemoryRegionAttribute ( + IN UINTN BaseAddress, + IN UINTN EndAddress, + OUT UINTN *RegionLength, + OUT UINTN *RegionAttributes + ); + +/** + Sets the Attributes of the specified memory region + + @param[in] BaseAddress The base address of the memory region to set the Attributes. + @param[in] Length The length of the memory region to set the Attributes. + @param[in] Attributes The Attributes to be set. + @param[in] AttributeMask Mask of memory attributes to take into account. + + @retval EFI_SUCCESS The Attributes was set successfully +**/ +EFI_STATUS +SetMemoryRegionAttributes ( + IN EFI_PHYSICAL_ADDRESS BaseAddress, + IN UINTN Length, + IN UINTN Attributes, + IN UINT64 AttributeMask + ); + +/** + Sets the non-executable Attributes for the specified memory region + + @param[in] BaseAddress The base address of the memory region to set the Attributes. + @param[in] Length The length of the memory region to set the Attributes. + + @retval EFI_SUCCESS The Attributes was set successfully +**/ +EFI_STATUS +SetMemoryRegionNoExec ( + IN EFI_PHYSICAL_ADDRESS BaseAddress, + IN UINTN Length + ); + +/** + Clears the non-executable Attributes for the specified memory region + + @param[in] BaseAddress The base address of the memory region to clear the Attributes. + @param[in] Length The length of the memory region to clear the Attributes. + + @retval EFI_SUCCESS The Attributes was clear successfully +**/ +EFI_STATUS +EFIAPI +ClearMemoryRegionNoExec ( + IN EFI_PHYSICAL_ADDRESS BaseAddress, + IN UINT64 Length + ); + +/** + Sets the read-only Attributes for the specified memory region + + @param[in] BaseAddress The base address of the memory region to set the Attributes. + @param[in] Length The length of the memory region to set the Attributes. + + @retval EFI_SUCCESS The Attributes was set successfully +**/ +EFI_STATUS +EFIAPI +SetMemoryRegionReadOnly ( + IN EFI_PHYSICAL_ADDRESS BaseAddress, + IN UINT64 Length + ); + +/** + Clears the read-only Attributes for the specified memory region + + @param[in] BaseAddress The base address of the memory region to clear the Attributes. + @param[in] Length The length of the memory region to clear the Attributes. + + @retval EFI_SUCCESS The Attributes was clear successfully +**/ +EFI_STATUS +EFIAPI +ClearMemoryRegionReadOnly ( + IN EFI_PHYSICAL_ADDRESS BaseAddress, + IN UINT64 Length + ); + +/** + Create a page table and initialize the memory management unit(MMU). + + @param[in] MemoryTable A pointer to a memory ragion table. + @param[out] TranslationTableBase A pointer to a translation table base address. + @param[out] TranslationTableSize A pointer to a translation table base size. + + @retval EFI_SUCCESS Configure MMU successfully. + EFI_INVALID_PARAMETER MemoryTable is NULL. + EFI_UNSUPPORTED Out of memory space or size not aligned. +**/ +EFI_STATUS +EFIAPI +ConfigureMemoryManagementUint ( + IN MEMORY_REGION_DESCRIPTOR *MemoryTable, + OUT VOID **TranslationTableBase OPTIONAL, + OUT UINTN *TranslationTableSize OPTIONAL + ); + +#endif // CPU_MMU_LIB_H_ diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec index 154b1d06fe..150beae981 100644 --- a/UefiCpuPkg/UefiCpuPkg.dec +++ b/UefiCpuPkg/UefiCpuPkg.dec @@ -62,6 +62,10 @@ ## @libraryclass Provides function for manipulating x86 paging structures. CpuPageTableLib|Include/Library/CpuPageTableLib.h +[LibraryClasses.LoongArch64] + ## @libraryclass Provides macros and functions for the memory management unit. + CpuMmuLib|Include/Library/CpuMmuLib.h + ## @libraryclass Provides functions for manipulating smram savestate registers. MmSaveStateLib|Include/Library/MmSaveStateLib.h -- 2.27.0