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) | | Mute This Topic | New Topic
Your Subscription | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_