Changeset View
Standalone View
sys/arm64/arm64/pmap.c
- This file is larger than 256 KB, so syntax highlighting is disabled by default.
| Show First 20 Lines • Show All 9,292 Lines • ▼ Show 20 Lines | pmap_map_io_transient(vm_page_t page[], vm_offset_t vaddr[], int count, | ||||
| if (!needs_mapping) | if (!needs_mapping) | ||||
| return (false); | return (false); | ||||
| if (!can_fault) | if (!can_fault) | ||||
| sched_pin(); | sched_pin(); | ||||
| for (i = 0; i < count; i++) { | for (i = 0; i < count; i++) { | ||||
| paddr = VM_PAGE_TO_PHYS(page[i]); | paddr = VM_PAGE_TO_PHYS(page[i]); | ||||
| if (!PHYS_IN_DMAP(paddr)) { | if (!PHYS_IN_DMAP(paddr)) { | ||||
| panic( | pmap_qenter(vaddr[i], &page[i], 1); | ||||
markj: Suppose pmap_qenter() adds a PTE, and then an unrelated pmap_enter() into the kernel map… | |||||
kibUnsubmitted Not Done Inline ActionsFor the same situation on amd64, I tried to come up with an argument why is it impossible, but failed. Of course, it is highly unlikely, but still. It seems not possible to do a trick with some unused bit from PTE, since there is no unused bits. E.g. combinations like wired but not managed etc. It seems that the best way (just for transient io mappings, but not for general qenter/qremove) is to allocate three pages for single page mapping, making the frame before and after the unmapped guards. kib: For the same situation on amd64, I tried to come up with an argument why is it impossible, but… | |||||
markjUnsubmitted Not Done Inline ActionsGuard pages should work... or should we simply use pmap_enter() here? I'm not sure if it is safe to lock the kernel pmap in this context. markj: Guard pages should work... or should we simply use pmap_enter() here? I'm not sure if it is… | |||||
kibUnsubmitted Not Done Inline Actionspmap_enter() would change semantic. For instance, we get spurious PGA_WRITEABLE tracking. kib: pmap_enter() would change semantic. For instance, we get spurious PGA_WRITEABLE tracking. | |||||
alcUnsubmitted Not Done Inline ActionsYes. It stems from the way that we currently manage the kernel virtual address space. Ignoring the faultable submaps within the kernel virtual address space for the moment, the rest of the pmap_enter() calls within the kernel virtual address space happen on virtual addresses from subarenas that import superpage-sized and -aligned address ranges. So, the virtual address that we allocate here can't be close enough to one of those pmap_enter() calls for it to be caught up in a promotion. That said, rather than relying here on the good behavior of the rest of the kernel, allocating here from a subarena, not the kernel_arena, that makes this guarantee explicit would be better engineering. alc: Yes. It stems from the way that we currently manage the kernel virtual address space. | |||||
kibUnsubmitted Not Done Inline ActionsBut then there must be a gap between subarenas that use pmap_enter(), and our vmem allocations that are served by pmap_qenter(). We do not provide such guards around, I believe. kib: But then there must be a gap between subarenas that use pmap_enter(), and our vmem allocations… | |||||
alcUnsubmitted Not Done Inline ActionsNo, if the import into the subarena is superpage aligned and a multiple of the superpage size, which they are. The pmap_qenter() here can't fall within a superpage-sized and -aligned address range on which we do a pmap_enter(). alc: No, if the import into the subarena is superpage aligned and a multiple of the superpage size… | |||||
andrewAuthorUnsubmitted Done Inline ActionsIf we don't want the entries created in pmap_qenter to be promoted we could add ATTR_SW_NO_PROMOTE. andrew: If we don't want the entries created in `pmap_qenter` to be promoted we could add… | |||||
alcUnsubmitted Not Done Inline ActionsWhile there would be no harm in having pmap_qenter set ATTR_SW_NO_PROMOTE, this issue is a concern across most if not all architectures that implement superpage promotion, so ultimately I'd rather see it handled uniformly at the machine-independent layer. The pmap_pte_exists that is eventually called by pmap_qremove already has a similar assertion to what @kib added to amd64 reporting an unexpected superpage. alc: While there would be no harm in having pmap_qenter set ATTR_SW_NO_PROMOTE, this issue is a… | |||||
markjUnsubmitted Not Done Inline ActionsAh, so it would be a bug to "optimize" KVA allocation by collapsing vm_dom[0].vmd_kernel_arena into kernel_arena on systems with only a single NUMA domain, which is a thought I've had once or twice. markj: Ah, so it would be a bug to "optimize" KVA allocation by collapsing `vm_dom[0]. | |||||
alcUnsubmitted Not Done Inline Actions@markj , yes. That is why I think we would be wise to make this function allocate from a different arena, which is itself setup to do superpage-sized and -aligned imports. Could we do this with the existing transient I/O arena? alc: @markj , yes. That is why I think we would be wise to make this function allocate from a… | |||||
markjUnsubmitted Not Done Inline ActionsA bit of modification would be needed in order to use transient_arena here:
I think that it's probably still the right direction to go. I'll work on a patch to implement that. markj: A bit of modification would be needed in order to use `transient_arena` here:
- we need to make… | |||||
alcUnsubmitted Not Done Inline ActionsAs a reminder, please add an XXX comment here saying that the implementation is still incomplete: When !can_fault, we don't need to perform a broadcast TLB invalidation, only a local one. alc: As a reminder, please add an XXX comment here saying that the implementation is still… | |||||
alcUnsubmitted Not Done Inline Actions... otherwise, the above pinning can be dropped. It's only needed for mappings that perform a local-only invalidation. alc: ... otherwise, the above pinning can be dropped. It's only needed for mappings that perform a… | |||||
kibUnsubmitted Not Done Inline ActionsBut the implementation if formally correct after this patch, do you agree? kib: But the implementation if formally correct after this patch, do you agree? | |||||
alcUnsubmitted Not Done Inline ActionsYes, but the pinning is currently pointless. alc: Yes, but the pinning is currently pointless. | |||||
| "pmap_map_io_transient: TODO: Map out of DMAP data"); | |||||
| } | } | ||||
| } | } | ||||
| return (needs_mapping); | return (needs_mapping); | ||||
| } | } | ||||
| void | void | ||||
| pmap_unmap_io_transient(vm_page_t page[], vm_offset_t vaddr[], int count, | pmap_unmap_io_transient(vm_page_t page[], vm_offset_t vaddr[], int count, | ||||
| bool can_fault) | bool can_fault) | ||||
| { | { | ||||
| vm_paddr_t paddr; | vm_paddr_t paddr; | ||||
| int i; | int i; | ||||
| if (!can_fault) | if (!can_fault) | ||||
| sched_unpin(); | sched_unpin(); | ||||
| for (i = 0; i < count; i++) { | for (i = 0; i < count; i++) { | ||||
| paddr = VM_PAGE_TO_PHYS(page[i]); | paddr = VM_PAGE_TO_PHYS(page[i]); | ||||
| if (!PHYS_IN_DMAP(paddr)) { | if (!PHYS_IN_DMAP(paddr)) { | ||||
| panic("ARM64TODO: pmap_unmap_io_transient: Unmap data"); | pmap_qremove(vaddr[i], 1); | ||||
| vmem_free(kernel_arena, vaddr[i], PAGE_SIZE); | |||||
| } | } | ||||
| } | } | ||||
| } | } | ||||
| bool | bool | ||||
| pmap_is_valid_memattr(pmap_t pmap __unused, vm_memattr_t mode) | pmap_is_valid_memattr(pmap_t pmap __unused, vm_memattr_t mode) | ||||
| { | { | ||||
| ▲ Show 20 Lines • Show All 567 Lines • Show Last 20 Lines | |||||
Suppose pmap_qenter() adds a PTE, and then an unrelated pmap_enter() into the kernel map results in promotion of the PTE's L3 table. pmap_qremove() will just clear it, which seems wrong. Is there anything preventing this?