Hi Ray,

For the GetMemoryRegionAttributes, here are some of my plans:

Thanks,
Chao

On 2023/12/14 10:53, Chao Li wrote:

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.

For this API, I think there may be two options:

Plan A:

It will return all of the attributes and lengths located in the region. We need to return a pointer to a structure that records the attributes and length of each part.

Plan B:

It only returns the attribute and length of the first part, leaving it up to the caller to decide to look for the next part.

I'm leaning toward plan B, what do you think?

_._,_._,_

Groups.io Links:

You receive all messages sent to this group.

_._,_._,_