From 8271b4a8bf725052a953e0f22e5306e0084f0b7e Mon Sep 17 00:00:00 2001 From: Michael Lotz Date: Mon, 16 Sep 2024 20:53:34 +0200 Subject: [PATCH] x86/arch_vm: Allow arch_vm_set_memory_type to override type. We allow requesting an explicit memory type when calling map_physical_memory but default to the uncached B_MTR_UC when not given. When called without an explicitly requested memory type, allow arch_vm_set_memory_type to modify and return an effective memory type. When an overlapping range already exists, the effective memory type is set to the one of the existing mapping. If there is an explicit memory type request that conflicts with an existing range, or if multiple overlaps with conflicting types would be produced, the mapping is disallow (and a panic is triggered under KDEBUG). This effectively detects and panics when conflicting aliases of physical memory would be created. This is also useful on an MTRR based setup, as such overlaps cannot be properly represented. When using the page attribute table (PAT) to set the memory type on a per page virtual memory mapping basis, this is needed to prevent aliasing of the same physical memory with different types. As per the specs, such aliasing is unsupported and may result in undefined operations that lead to system failure. The mechanism is extended to the general arch_vm_set_memory_type as such aliasing prevention also seems to apply to other architectures (at least on ARM, aliasing is also strongly discouraged). Change-Id: I7aaf6ea8415e92e74cd1643b67793a6857619eea Reviewed-on: https://review.haiku-os.org/c/haiku/+/8339 Tested-by: Commit checker robot Reviewed-by: waddlesplash --- headers/private/kernel/arch/vm.h | 2 +- src/system/kernel/arch/arm/arch_vm.cpp | 3 +- src/system/kernel/arch/arm64/arch_vm.cpp | 3 +- src/system/kernel/arch/m68k/arch_vm.cpp | 3 +- src/system/kernel/arch/ppc/arch_vm.cpp | 3 +- src/system/kernel/arch/riscv64/arch_vm.cpp | 3 +- src/system/kernel/arch/sparc/arch_vm.cpp | 3 +- src/system/kernel/arch/x86/arch_vm.cpp | 99 ++++++++++++++-------- src/system/kernel/vm/vm.cpp | 12 ++- 9 files changed, 85 insertions(+), 46 deletions(-) diff --git a/headers/private/kernel/arch/vm.h b/headers/private/kernel/arch/vm.h index 2fc4a82d93..d5ff39bff4 100644 --- a/headers/private/kernel/arch/vm.h +++ b/headers/private/kernel/arch/vm.h @@ -32,7 +32,7 @@ void arch_vm_aspace_swap(struct VMAddressSpace *from, bool arch_vm_supports_protection(uint32 protection); status_t arch_vm_set_memory_type(struct VMArea *area, phys_addr_t physicalBase, - uint32 type); + uint32 type, uint32 *effectiveType); void arch_vm_unset_memory_type(struct VMArea *area); #ifdef __cplusplus diff --git a/src/system/kernel/arch/arm/arch_vm.cpp b/src/system/kernel/arch/arm/arch_vm.cpp index 34083f34d5..c7ab80b2f3 100644 --- a/src/system/kernel/arch/arm/arch_vm.cpp +++ b/src/system/kernel/arch/arm/arch_vm.cpp @@ -132,7 +132,8 @@ arch_vm_unset_memory_type(VMArea *area) status_t -arch_vm_set_memory_type(VMArea *area, phys_addr_t physicalBase, uint32 type) +arch_vm_set_memory_type(VMArea *area, phys_addr_t physicalBase, uint32 type, + uint32 *effectiveType) { return B_OK; } diff --git a/src/system/kernel/arch/arm64/arch_vm.cpp b/src/system/kernel/arch/arm64/arch_vm.cpp index b1b531c75a..b0be4d846e 100644 --- a/src/system/kernel/arch/arm64/arch_vm.cpp +++ b/src/system/kernel/arch/arm64/arch_vm.cpp @@ -130,7 +130,8 @@ arch_vm_unset_memory_type(VMArea* area) status_t -arch_vm_set_memory_type(VMArea* area, phys_addr_t physicalBase, uint32 type) +arch_vm_set_memory_type(VMArea* area, phys_addr_t physicalBase, uint32 type, + uint32 *effectiveType) { // Memory type is set in page tables during mapping, // no need to do anything more here. diff --git a/src/system/kernel/arch/m68k/arch_vm.cpp b/src/system/kernel/arch/m68k/arch_vm.cpp index df7d102b4c..3b5dc02342 100644 --- a/src/system/kernel/arch/m68k/arch_vm.cpp +++ b/src/system/kernel/arch/m68k/arch_vm.cpp @@ -131,7 +131,8 @@ arch_vm_unset_memory_type(VMArea *area) status_t -arch_vm_set_memory_type(VMArea *area, phys_addr_t physicalBase, uint32 type) +arch_vm_set_memory_type(VMArea *area, phys_addr_t physicalBase, uint32 type, + uint32 *effectiveType) { if (type == 0) return B_OK; diff --git a/src/system/kernel/arch/ppc/arch_vm.cpp b/src/system/kernel/arch/ppc/arch_vm.cpp index 39296034a5..7f63e4d59f 100644 --- a/src/system/kernel/arch/ppc/arch_vm.cpp +++ b/src/system/kernel/arch/ppc/arch_vm.cpp @@ -170,7 +170,8 @@ arch_vm_unset_memory_type(VMArea *area) status_t -arch_vm_set_memory_type(VMArea *area, phys_addr_t physicalBase, uint32 type) +arch_vm_set_memory_type(VMArea *area, phys_addr_t physicalBase, uint32 type, + uint32 *effectiveType) { if (type == 0) return B_OK; diff --git a/src/system/kernel/arch/riscv64/arch_vm.cpp b/src/system/kernel/arch/riscv64/arch_vm.cpp index a93dd0b24d..d1c9b9a803 100644 --- a/src/system/kernel/arch/riscv64/arch_vm.cpp +++ b/src/system/kernel/arch/riscv64/arch_vm.cpp @@ -378,7 +378,8 @@ arch_vm_unset_memory_type(VMArea *area) status_t -arch_vm_set_memory_type(VMArea *area, phys_addr_t physicalBase, uint32 type) +arch_vm_set_memory_type(VMArea *area, phys_addr_t physicalBase, uint32 type, + uint32 *effectiveType) { return B_OK; } diff --git a/src/system/kernel/arch/sparc/arch_vm.cpp b/src/system/kernel/arch/sparc/arch_vm.cpp index 8a72900813..10bea93df0 100644 --- a/src/system/kernel/arch/sparc/arch_vm.cpp +++ b/src/system/kernel/arch/sparc/arch_vm.cpp @@ -113,7 +113,8 @@ arch_vm_unset_memory_type(VMArea *area) status_t -arch_vm_set_memory_type(VMArea *area, phys_addr_t physicalBase, uint32 type) +arch_vm_set_memory_type(VMArea *area, phys_addr_t physicalBase, uint32 type, + uint32 *effectiveType) { if (type == 0) return B_OK; diff --git a/src/system/kernel/arch/x86/arch_vm.cpp b/src/system/kernel/arch/x86/arch_vm.cpp index f238de2936..902fa7bbe4 100644 --- a/src/system/kernel/arch/x86/arch_vm.cpp +++ b/src/system/kernel/arch/x86/arch_vm.cpp @@ -130,6 +130,26 @@ set_mtrrs() static bool add_used_mtrr(uint64 base, uint64 size, uint32 type) { + switch (type) { + case B_MTR_UC: + type = IA32_MTR_UNCACHED; + break; + case B_MTR_WC: + type = IA32_MTR_WRITE_COMBINING; + break; + case B_MTR_WT: + type = IA32_MTR_WRITE_THROUGH; + break; + case B_MTR_WP: + type = IA32_MTR_WRITE_PROTECTED; + break; + case B_MTR_WB: + type = IA32_MTR_WRITE_BACK; + break; + default: + return false; + } + if (sMemoryTypeRegistersUsed == sMemoryTypeRegisterCount) return false; @@ -308,7 +328,7 @@ ensure_temporary_ranges_space(int32 count) } -status_t +static status_t update_mtrrs(update_mtrr_info& updateInfo) { // resize the temporary points/ranges arrays, if necessary @@ -320,7 +340,7 @@ update_mtrrs(update_mtrr_info& updateInfo) int32 pointCount = 0; for (MemoryTypeRangeList::Iterator it = sMemoryTypeRanges.GetIterator(); memory_type_range* range = it.Next();) { - if (range->type == IA32_MTR_UNCACHED) { + if (range->type == B_MTR_UC) { // Ignore uncacheable ranges below a certain size, if requested. // Since we always enforce uncacheability via the PTE attributes, // this is no problem (though not recommended for performance @@ -437,11 +457,11 @@ update_mtrrs(update_mtrr_info& updateInfo) rangeList.Add(&ranges[i]); static const uint32 kMemoryTypes[] = { - IA32_MTR_UNCACHED, - IA32_MTR_WRITE_COMBINING, - IA32_MTR_WRITE_PROTECTED, - IA32_MTR_WRITE_THROUGH, - IA32_MTR_WRITE_BACK + B_MTR_UC, + B_MTR_WC, + B_MTR_WP, + B_MTR_WT, + B_MTR_WB }; static const int32 kMemoryTypeCount = sizeof(kMemoryTypes) / sizeof(*kMemoryTypes); @@ -452,8 +472,7 @@ update_mtrrs(update_mtrr_info& updateInfo) // Remove uncached and write-through ranges after processing them. This // let's us leverage their intersection property with any other // respectively write-back ranges. - bool removeRanges = type == IA32_MTR_UNCACHED - || type == IA32_MTR_WRITE_THROUGH; + bool removeRanges = type == B_MTR_UC || type == B_MTR_WT; optimize_memory_ranges(rangeList, type, removeRanges); } @@ -475,7 +494,7 @@ update_mtrrs(update_mtrr_info& updateInfo) uint32 type = kMemoryTypes[i]; // skip write-back ranges -- that'll be the default type anyway - if (type == IA32_MTR_WRITE_BACK) + if (type == B_MTR_WB) continue; for (int32 i = 0; i < rangeCount; i++) { @@ -493,7 +512,7 @@ update_mtrrs(update_mtrr_info& updateInfo) } -status_t +static status_t update_mtrrs() { // Until we know how many MTRRs we have, pretend everything is OK. @@ -535,37 +554,46 @@ update_mtrrs() static status_t -add_memory_type_range(area_id areaID, uint64 base, uint64 size, uint32 type) +add_memory_type_range(area_id areaID, uint64 base, uint64 size, uint32 type, + uint32 *effectiveType) { // translate the type if (type == 0) return B_OK; - switch (type) { - case B_MTR_UC: - type = IA32_MTR_UNCACHED; - break; - case B_MTR_WC: - type = IA32_MTR_WRITE_COMBINING; - break; - case B_MTR_WT: - type = IA32_MTR_WRITE_THROUGH; - break; - case B_MTR_WP: - type = IA32_MTR_WRITE_PROTECTED; - break; - case B_MTR_WB: - type = IA32_MTR_WRITE_BACK; - break; - default: - return B_BAD_VALUE; - } - TRACE_MTRR2("add_memory_type_range(%" B_PRId32 ", %#" B_PRIx64 ", %#" B_PRIx64 ", %" B_PRIu32 ")\n", areaID, base, size, type); MutexLocker locker(sMemoryTypeLock); + for (MemoryTypeRangeList::Iterator it = sMemoryTypeRanges.GetIterator(); + memory_type_range* range = it.Next();) { + + if (range->area == areaID || range->type == type + || base + size <= range->base + || base >= range->base + range->size) { + continue; + } + + if (effectiveType != NULL) { + type = *effectiveType = range->type; + effectiveType = NULL; + + dprintf("assuming memory type %" B_PRIx32 " for overlapping %#" + B_PRIx64 ", %#" B_PRIx64 " area %" B_PRId32 " from existing %#" + B_PRIx64 ", %#" B_PRIx64 " area %" B_PRId32 "\n", type, + base, size, areaID, range->base, range->size, range->area); + continue; + } + + (KDEBUG ? panic : dprintf)("incompatible overlapping memory %#" B_PRIx64 + ", %#" B_PRIx64 " type %" B_PRIx32 " area %" B_PRId32 + " with existing %#" B_PRIx64 ", %#" B_PRIx64 " type %" B_PRIx32 + " area %" B_PRId32 "\n", base, size, type, areaID, range->base, + range->size, range->type, range->area); + return B_BUSY; + } + memory_type_range* range = areaID >= 0 ? find_range(areaID) : NULL; int32 oldRangeType = -1; if (range != NULL) { @@ -699,7 +727,7 @@ arch_vm_init_post_modules(kernel_args *args) // set the physical memory ranges to write-back mode for (uint32 i = 0; i < args->num_physical_memory_ranges; i++) { add_memory_type_range(-1, args->physical_memory_range[i].start, - args->physical_memory_range[i].size, B_MTR_WB); + args->physical_memory_range[i].size, B_MTR_WB, NULL); } return B_OK; @@ -758,7 +786,8 @@ arch_vm_unset_memory_type(struct VMArea *area) status_t arch_vm_set_memory_type(struct VMArea *area, phys_addr_t physicalBase, - uint32 type) + uint32 type, uint32 *effectiveType) { - return add_memory_type_range(area->id, physicalBase, area->Size(), type); + return add_memory_type_range(area->id, physicalBase, area->Size(), type, + effectiveType); } diff --git a/src/system/kernel/vm/vm.cpp b/src/system/kernel/vm/vm.cpp index b2496b1ed7..9d324bacb1 100644 --- a/src/system/kernel/vm/vm.cpp +++ b/src/system/kernel/vm/vm.cpp @@ -1948,14 +1948,18 @@ vm_map_physical_memory(team_id team, const char* name, void** _address, cache->Unlock(); if (status == B_OK) { - // set requested memory type -- use uncached, if not given + // Set requested memory type -- use uncached if not given but allow it + // to be overridden by ranges that may already exist uint32 memoryType = addressSpec & B_MTR_MASK; - if (memoryType == 0) + bool weak = memoryType == 0; + if (weak) memoryType = B_MTR_UC; + status = arch_vm_set_memory_type(area, physicalAddress, memoryType, + weak ? &memoryType : NULL); + area->SetMemoryType(memoryType); - status = arch_vm_set_memory_type(area, physicalAddress, memoryType); if (status != B_OK) delete_area(locker.AddressSpace(), area, false); } @@ -5345,7 +5349,7 @@ vm_set_area_memory_type(area_id id, phys_addr_t physicalBase, uint32 type) map->Unlock(); // set the physical memory type - status_t error = arch_vm_set_memory_type(area, physicalBase, type); + status_t error = arch_vm_set_memory_type(area, physicalBase, type, NULL); if (error != B_OK) { // reset the memory type of the area and the mapped pages map->Lock();