Re: [edk2-devel] [PATCH v8 14/37] UefiCpuPkg: Add CpuMmuLib to UefiCpuPkg

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

[-- Attachment #1: Type: text/plain, Size: 3509 bytes --]

Hi Laszlo,


Thanks,
Chao
On 2024/2/2 06:46, Laszlo Ersek wrote:
> On 2/1/24 08:57, Chao Li wrote:
>
>> Let's me tell you this library how to work:
>>
>> In PEI stage, in addition to ensuring that the MMU is not used, the user
>> must call the ConfigureMemoryManagementUnit toinitialization the MMU,
>> such as filling the static page tables, set the TLB refill exception
>> entry point, set the page size etc. the ConfigureMemoryManagementUnit is
>> a private API but related to ARCH.
>>
>> During DXE stage, this library will provide services for CpuDxe and
>> other drivers, but almost changes to memory page attributes use the
>> protocols provided by CpuDxe. That is why the CpuMmuLib.h only contains
>> two APIs, it only can get/set the attribute.
>>
>> In short, the PEI stage needs to initialize and set up TLB refill entry
>> point and method(dynamically populate the TLB, keep the PA == VA), and
>> DXE stage is provides services for changing the memory attributes.
> My understanding is the following then:
>
> (1) You should provide a very low level library, implementing and also
> publicly exposing primitives for manipulating page tables. This library
> could / should expose ConfigureMemoryManagementUnit(). This library
> should not use global variables at all, I think -- the user of the
> library would be responsible for tracking state. API names in this
> library should have a good, distinctive prefix.
>
> (2) Introduce a higher level lib class that contains very few APIs --
> APIs that are possible to implement by multiple arches.
>
> (3) Provide a library instance of class (2) by way of consuming (1).
>
> (4) In your platform PEIM, for setting up paging, use (1) directly, or
> (if you can do it) only (2)+(3).
>
> (5) In DXE phase code, use (2)+(3).
>
> Basically I both agree and disagree with Ray's review for your
>
>    [PATCH v3 13/39] UefiCpuPkg: Add CpuMmuLib.h to UefiCpuPkg
>
> I think that Ray is right in that we want good high-level paging APIs
> that can be implemented by multiple arches. On the other hand,
> architectural differences do exist, they are idiosyncratic, they need to
> exist *somewhere*, and that idiosyncratic logic may still be necessary
> to use from *multiple* places -- so that indicates that we need a
> library (low-level) that is not "well designed" but actually a "grab bag
> of utilities" that just does the work.
>
> To satisfy both requirements, insert a new layer of indirection :)
>
> I'm not a big fan of top-down library design. I rather like to look at
> existent usage patterns, and then factoring out common stuff to a
> library. At that point, unification / cleanup possibilities may emerge.
> But getting the API design right immediately -- that requires seeing the
> future, or *extreme* amounts of experience. So an "ugly" interface
> library may be unavoidable. But, we can always wrap it into nicer
> interfaces, for those clients that don't really need the low-level
> primitives.
When I redesign this module, I will try to add a new layer, what you 
call a low-level library.
>
> Laszlo


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



[-- Attachment #2: Type: text/html, Size: 4811 bytes --]