Skip to content

[Feature]: Consider setting cache attributes on freed pages #1418

@thomashinds

Description

@thomashinds

Feature Overview

When memory is freed via free_pages, Patina sets certain attributes before freeing - preserving cache attributes and setting RP & XP.

ref:

if let Err(e) = self.set_memory_space_attributes_worker(
current_range.start as usize,
(current_range.end - current_range.start) as usize,
MemoryProtectionPolicy::apply_free_memory_policy(desc.attributes),
desc.attributes,

/// Rule: All free memory should be marked as EFI_MEMORY_RP, EFI_MEMORY_XP, and the preserved cache attributes.
/// EFI_MEMORY_RP will cause the memory to be unmapped in the page table, but we still set EFI_MEMORY_XP to align
/// with the originally added memory so that free memory can be coalesced into fewer blocks.
///
/// Arguments
/// - * `attributes` - The existing attributes for the memory region
///
/// Use Case: This is called whenever memory is freed in the GCD
pub(crate) const fn apply_free_memory_policy(attributes: u64) -> u64 {
(attributes & efi::CACHE_ATTRIBUTE_MASK) | efi::MEMORY_RP | efi::MEMORY_XP
}

Solution Overview

Consider enforcing opinionated cache attributes on freed pages, in order to have a stronger guarantee on the state of the pages' attributes if allocated in the future, and to allow freed regions to coalesce together if cache attributes were changed while in use. This should be reasonable to do because platforms should not impose requirements on the cache attributes of pooled free memory.

Alternatives Considered

  • Make no changes - as I understand it this is closer to how EDK2's core behaves today but has the drawbacks identified above.
  • Enforce that the caller should restore attributes before freeing.

Urgency

Low

Are you going to implement the feature request?

Someone else needs to implement the feature

Do you need maintainer feedback?

Maintainer feedback requested

Anything else?

No response

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions