Hi Andrei,

I agree with you, I will move LoongArch-specific stuff from UefiCpuPkg 
to MdePkg/Include/ in V3.

And you are also saied record the idea of moving ArmMmuLib to CpuMmuLib, 
I would to say, this header is referenced from ArmPkg, and I plan to 
move the ArmMmuLib from ArmPkg to UefiCpuPkg after merging this patch 
set, and other BZ number will be used. The premise is that the 
maintainers of ArmPkg agrees move operation.


Thanks,
Chao
On 2023/11/9 09:17, Andrei Warkentin wrote:
> Hi Chao,
>
> I see that you're adding a header, CpuMmuLib.h, but this header is not generic - it includes a bunch of Loongson arch-specific "stuff".
>
> I agree in principle that there's room for a generic CpuMmuLib, and I don't think you ought to be on the hook to refactor ArmMmuLib, but I suggest moving
> the architecture-specific PTE #defines into MdePkg/Include similar to ./MdePkg/Include/Register/RiscV64. (it would, though, be nice to at least record the idea of moving ArmMmuLib to CpuMmuLib.... raise a BZ for that or something!)
>
> This will avoid the #ifdef MDE_CPU_LOONGARCH64 inside a generic Library header.
>
> A
>
>> -----Original Message-----
>> From: Chao Li<lichao@loongson.cn>
>> Sent: Sunday, November 5, 2023 9:28 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 v2 11/30] 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 | 194
>> +++++++++++++++++++++++++
>>   UefiCpuPkg/UefiCpuPkg.dec              |   4 +
>>   2 files changed, 198 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..8f524d31d4
>> --- /dev/null
>> +++ b/UefiCpuPkg/Include/Library/CpuMmuLib.h
>> @@ -0,0 +1,194 @@
>> +/** @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>
>> +
>> +#ifdef MDE_CPU_LOONGARCH64
>> +
>> +/* Page table property definitions  */
>> +#define PAGE_VALID_SHIFT   0
>> +#define PAGE_DIRTY_SHIFT   1
>> +#define PAGE_PLV_SHIFT     2  // 2~3, two bits
>> +#define CACHE_SHIFT        4  // 4~5, two bits
>> +#define PAGE_GLOBAL_SHIFT  6
>> +#define PAGE_HUGE_SHIFT    6  // HUGE is a PMD bit
>> +
>> +#define PAGE_HGLOBAL_SHIFT  12 // HGlobal is a PMD bit
>> +#define PAGE_PFN_SHIFT      12
>> +#define PAGE_PFN_END_SHIFT  48
>> +#define PAGE_NO_READ_SHIFT  61
>> +#define PAGE_NO_EXEC_SHIFT  62
>> +#define PAGE_RPLV_SHIFT     63
>> +
>> +/* Used by TLB hardware (placed in EntryLo*) */
>> +#define PAGE_VALID    ((UINTN)(1) << PAGE_VALID_SHIFT)
>> +#define PAGE_DIRTY    ((UINTN)(1) << PAGE_DIRTY_SHIFT)
>> +#define PAGE_PLV      ((UINTN)(3) << PAGE_PLV_SHIFT)
>> +#define PAGE_GLOBAL   ((UINTN)(1) << PAGE_GLOBAL_SHIFT)
>> +#define PAGE_HUGE     ((UINTN)(1) << PAGE_HUGE_SHIFT)
>> +#define PAGE_HGLOBAL  ((UINTN)(1) << PAGE_HGLOBAL_SHIFT) #define
>> +PAGE_NO_READ  ((UINTN)(1) << PAGE_NO_READ_SHIFT) #define
>> PAGE_NO_EXEC
>> +((UINTN)(1) << PAGE_NO_EXEC_SHIFT)
>> +#define PAGE_RPLV     ((UINTN)(1) << PAGE_RPLV_SHIFT)
>> +#define CACHE_MASK    ((UINTN)(3) << CACHE_SHIFT)
>> +#define PFN_SHIFT     (EFI_PAGE_SHIFT - 12 + PAGE_PFN_SHIFT)
>> +
>> +#define PLV_KERNEL  0
>> +#define PLV_USER    3
>> +
>> +#define PAGE_USER    (PLV_USER << PAGE_PLV_SHIFT)
>> +#define PAGE_KERNEL  (PLV_KERN << PAGE_PLV_SHIFT)
>> +
>> +#define CACHE_SUC  (0 << CACHE_SHIFT) // Strong-ordered UnCached
>> +#define CACHE_CC   (1 << CACHE_SHIFT) // Coherent Cached
>> +#define CACHE_WUC  (2 << CACHE_SHIFT) // Weak-ordered UnCached
>> +
>> +#define EFI_MEMORY_CACHETYPE_MASK  (EFI_MEMORY_UC  | \
>> +                                    EFI_MEMORY_WC  | \
>> +                                    EFI_MEMORY_WT  | \
>> +                                    EFI_MEMORY_WB  | \
>> +                                    EFI_MEMORY_UCE   \
>> +                                    )
>> +#endif
>> +
>> +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.
>> +
>> +  @retval  EFI_SUCCESS    The Attributes was set successfully
>> +**/
>> +EFI_STATUS
>> +SetMemoryAttributes (
>> +  IN EFI_PHYSICAL_ADDRESS  BaseAddress,
>> +  IN UINTN                 Length,
>> +  IN UINTN                 Attributes
>> +  );
>> +
>> +/**
>> +  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[in 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,
>> +  IN 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
>
>
> 
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111187): https://edk2.groups.io/g/devel/message/111187
Mute This Topic: https://groups.io/mt/102413866/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-