Hi Yanbo,

I didn't do it in the way you suggest for the same reason that the 
SplitTable() logic doesn't set attributes
on descriptors of type EfiRuntimeServicesData or other memory types. The 
purpose of the SplitTable() function
is to use the input image records to split descriptors so each image 
section has it's own descriptor. I think
it's reasonable to set the attributes on descriptors associated with 
images because those new descriptors are the
intended side effect of the function, but I don't think setting 
attributes on other descriptors is a good design pattern.
This pattern matches what's done in PiSmmCore/MemoryAttributesTable.c. 
Also, even if we did
it the way you suggest, we would still need call 
EnforceMemoryMapAttribute() later to set XP on the
EfiRuntimeServicesData descriptors.

Can you or Dandan explain the origin of the extra EfiRuntimeServicesCode 
regions which aren't
part of loaded runtime images? It would be a good datapoint for our 
discussion on the proposed fix.

-Taylor
On 4/17/2024 7:04 AM, Huang, Yanbo wrote:
>
> Hi Taylor,
>
> Thanks for your update.
>
> After test, issue can be fixed by your patch.
>
> But why we not set the EFI_MEMORY_XP or EFI_MEMORY_RO attribute in 
> SplitRecord API?
>
> If we set the attribute in the beginning of the NewRecord created, it 
> seems we don’t need to EnforceMemoryMapAttribute later?
>
> Best Regards,
>
> Yanbo Huang
>


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