Memory mapping issues in ARM64 #351
Conversation
vm map takes arg 3 as virtual address. And same needs
to be passed to retrieve aspace. Fix this by passing
right arg.
Test: Ran 'vm map' test
] vm map 0xc0000000 0xffff000ff0000000 1 0x0
arch_mmu_map returns 0
Signed-off-by: Chintan Pandya <[email protected]>
Alias mapping with different mapping attributes creates
unknown behavior with either of the mappings. This was
specially observed when previously mapped cached region
is re-mapped at other virtual address as uncached region
creating stability issues as mentioned in b/240659444.
Do proper error checking for new mappings against
mmu_initial_mapping.
Test: Created alias mapping with mismatch attribute and
tested on gem5
] vmm alloc_physical 0xc0000000 4096 10
Mismatch attributes, device mapping failed
vmm_alloc_physical: arch_mmu_map returns -17
vmm_alloc_physical returns -5, ptr 0xffff000000000000
] vmm alloc_physical 0x2A000000 4096 10
vmm_alloc_physical returns 0, ptr 0xffff000000000000
Signed-off-by: Chintan Pandya <[email protected]>
| @@ -177,7 +177,7 @@ static int mem_test(int argc, const console_cmd_args *argv) { | |||
| } | |||
|
|
|||
| /* allocate a region to test in */ | |||
| status_t err = vmm_alloc_contiguous(vmm_get_kernel_aspace(), "memtest", len, &ptr, 0, 0, ARCH_MMU_FLAG_UNCACHED); | |||
There was a problem hiding this comment.
Unfortunately this and the above code, in lkboot, rely on uncached mappings in order to properly work.
| @@ -521,6 +523,35 @@ int arch_mmu_map(arch_aspace_t *aspace, vaddr_t vaddr, paddr_t paddr, uint count | |||
| return NO_ERROR; | |||
|
|
|||
| int ret; | |||
|
|
|||
There was a problem hiding this comment.
This routine is a bit problematic due to generally removing the ability to map things as uncached anywhere in practice, except as a hardware register.
Is the problem that since the regular mapping still exists there can exist writeback cache lines that trash the uncached mapping (which seems possible) or that via the large mapping it can prefetch into it? In the latter case that generally seems harmless unless it's secure memory or whatnot, provided the memory is cleaned+invalidated (or just invalidated) prior to mapping it here. In that case the extra cached mapping can only at worse end up with a prefetch that ends up with a RO cache line.
| @@ -370,10 +370,17 @@ status_t vmm_alloc_physical(vmm_aspace_t *aspace, const char *name, size_t size, | |||
|
|
|||
| /* map all of the pages */ | |||
| int err = arch_mmu_map(&aspace->arch_aspace, r->base, paddr, size / PAGE_SIZE, arch_mmu_flags); | |||
| LTRACEF("arch_mmu_map returns %d\n", err); | |||
| if (err) { | |||
There was a problem hiding this comment.
These error catching routines here and below look good, can you split them into a separate commit so they can be taken independent of the meat of the change?
No description provided.