[Feature]: Create common interface for setting attributes across MemorySpaceDescriptors

[cfernald]

Feature Overview

Currently, there are several implementations of using core_get_memory_space_descriptor to set attributes accross multiple descriptors. Examples at the type of this feature request:

• set_page_attributes
• clear_memory_attributes
• set_memory_attributes

Solution Overview

Create a common method for doing this to avoid redundant code

Alternatives Considered

No response

Urgency

Low

Are you going to implement the feature request?

Someone else needs to implement the feature

Do you need maintainer feedback?

No maintainer feedback needed

Anything else?

No response

[cfernald]

Assigning to @HappyCoding-2025 who asked to take this on.

[HappyCoding-2025]

Hi Team,
I looked at the current paging and system memory handling ; Yes, there is redundancy and duplicated logic in separate traits and functions for paging and system memory in the current implementation.

Here is the proposed design, let us discuss this, once you are all okay, will start the implementation for this.

1. Unify memory and paging attribute management under a single trait which would provide 2 functionality

for set, clear and update operations]
for get -retrieving current access/caching attributes for a memory range.

2. Both system memory module and paging module will then implement this unified trait. System memory trait implementation will still delegate low-level paging changes to paging module.
   Are you all okay with this?

@cfernald @makubacki @Javagedes @joschock @singweeMsftGitHub

[os-d]

@HappyCoding-2025 this issue is intended to address the duplication of this pattern:

patina/patina_dxe_core/src/memory_attributes_protocol.rs

Lines 90 to 129 in 403d104

	let mut current_base = base_address;
	let range_end = (base_address + length);
	while current_base < range_end {
	let descriptor = match dxe_services::core_get_memory_space_descriptor(current_base as efi::PhysicalAddress) {
	Ok(descriptor) => descriptor,
	Err(e) => {
	log::error!(
	"Memory descriptor fetching failed with error {:#x?} for {:#x} in {}",
	e,
	current_base,
	function!()
	);
	// Only a few error codes are allowed per UEFI spec, so return unsupported
	return efi::Status::UNSUPPORTED;
	}
	};
	let descriptor_end = descriptor.base_address + descriptor.length;
	// it is still legal to split a descriptor and only set the attributes on part of it
	let next_base = u64::min(descriptor_end, range_end);
	let current_len = next_base - current_base;
	// this API only adds new attributes that are set, it ignores all 0 attributes. So, we need to get the memory
	// descriptor first and then set the new attributes as the GCD API takes into account all attributes set or unset.
	let new_attributes = descriptor.attributes | attributes;
	match dxe_services::core_set_memory_space_attributes(current_base, current_len, new_attributes) {
	Ok(_) => {}
	// only a few status codes are allowed per UEFI spec, so return unsupported
	// we don't have a reliable mechanism to reset any previously set attributes if an earlier block succeeded
	// because any tracking mechanism would be require memory allocations which could change the descriptors
	// and cause some attributes to be set on a potentially incorrect memory region. At this point if we have
	// failed, the system is dead, barring a bootloader allocating new memory and attempting to set attributes
	// there, because this API is only used by a bootloader setting memory attributes for the next image it is
	// loading. The expectation is that on a future boot the platform would disable this protocol.
	Err(status) => return efi::Status::UNSUPPORTED,
	};
	current_base = next_base;
	}
	efi::Status::SUCCESS

Please take a look at that and see if you want to help address this. The current paging/memory management traits are not in scope for this issue, but we can discuss outside of this one.