Re: [edk2-devel] [PATCH v4 12/37] UefiCpuPkg: Add CpuMmuLib.h to UefiCpuPkg

["Chao Li" <lichao@loongson.cn>]

Hi Ray,

For you two comments:

BTY, can you review the other UefiCpuPkg patches in this series?

On 2023/12/13 下午1:17, Ni, Ray wrote:
> Chao,
> Thanks for providing such a cleaner lib API.
> Only 2 comments in below:
>
>> +
>> +#define EFI_MEMORY_CACHETYPE_MASK  (EFI_MEMORY_UC  | \
>> +                                    EFI_MEMORY_WC  | \
>> +                                    EFI_MEMORY_WT  | \
>> +                                    EFI_MEMORY_WB  | \
>> +                                    EFI_MEMORY_UCE   \
>> +                                    )
> 1. Why do you need to define the EFI_MEMORY_CACHETYPE_MASK here?
> Can you just mention that the following APIs only return 5 bits: UC/WC/WT/WB/UCE
> without defining the EFI_MEMORY_CACHETYPE_MASK?
Agree, I will remove this definition in V5 and if someone uses this 
macro, it will be private.
>
>> +
>> +typedef struct {
>> +  EFI_PHYSICAL_ADDRESS    PhysicalBase;
>> +  EFI_VIRTUAL_ADDRESS     VirtualBase;
>> +  UINTN                   Length;
>> +  UINTN                   Attributes;
>> +} MEMORY_REGION_DESCRIPTOR;
>> +
>> +/**
>> +  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
>> +EFIAPI
>> +GetMemoryRegionAttributes (
>> +  IN     UINTN  BaseAddress,
>> +  IN     UINTN  EndAddress,
>> +  OUT    UINTN  *RegionLength,
>> +  OUT    UINTN  *RegionAttributes
>> +  );
> 2. If the actual memory ranges are as follows:
> [0 - 1M, UC]
> [1M - 1G, WB]
>
> What's the result of following call:
> a. GetMemoryRegionAttributes (512KB, 1MB)
> b. GetMemoryRegionAttributes (512KB, 2MB)
Yes, if the given memory region has two or more types of attributes, 
this API may return the first attribute, I should design this API again, 
thanks.
>
>
>
> 
>



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