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.

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?

If the backend silicon IP is the same one that supports the "one" lib design, 
can we refine the lib API a bit?

We have (Set|Get)MemoryRegionAttribute() and 
(Set|Clear)MemoryRegion(NoExec|ReadOnly). Can we merge them together?

And the API ConfigureMemoryManagementUint() accepts MEMORY_REGION_DESCRIPTOR 
but none of other APIs helps to construct the descriptor.

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 <lic...@loongson.cn>
> Sent: Friday, November 17, 2023 6:00 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.d...@intel.com>; Ni, Ray <ray...@intel.com>; Kumar,
> Rahul R <rahul.r.ku...@intel.com>; Gerd Hoffmann <kra...@redhat.com>;
> Leif Lindholm <quic_llind...@quicinc.com>; Ard Biesheuvel
> <ardb+tianoc...@kernel.org>; Sami Mujawar <sami.muja...@arm.com>;
> Sunil V L <suni...@ventanamicro.com>; Warkentin, Andrei
> <andrei.warken...@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.d...@intel.com>
> Cc: Ray Ni <ray...@intel.com>
> Cc: Rahul Kumar <rahul1.ku...@intel.com>
> Cc: Gerd Hoffmann <kra...@redhat.com>
> Cc: Leif Lindholm <quic_llind...@quicinc.com>
> Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org>
> Cc: Sami Mujawar <sami.muja...@arm.com>
> Cc: Sunil V L <suni...@ventanamicro.com>
> Cc: Andrei Warkentin <andrei.warken...@intel.com>
> Signed-off-by: Chao Li <lic...@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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111871): https://edk2.groups.io/g/devel/message/111871
Mute This Topic: https://groups.io/mt/102644768/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to