Jan Kiszka <[email protected]> writes:
> On 15.06.22 09:54, Philippe Gerum wrote: >> >> Jan Kiszka via Xenomai <[email protected]> writes: >> >>> On 23.05.22 16:04, Gunter Grau via Xenomai wrote: >>>> From: Gunter Grau <[email protected]> >>>> >>>> When mapping io memory into userspace an extra simulated pagefault for all >>>> pages is added to prevent later pagefaults because of copy on write >>>> mechanisms. This happens only on architectures that have defined the >>>> needed cobalt_machine.prefault function. >>>> >>>> Signed-off-by: Gunter Grau <[email protected]> >>>> --- >>>> kernel/cobalt/rtdm/drvlib.c | 10 +++++++++- >>>> 1 file changed, 9 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/kernel/cobalt/rtdm/drvlib.c b/kernel/cobalt/rtdm/drvlib.c >>>> index 4eaf3a57c..db8431ee1 100644 >>>> --- a/kernel/cobalt/rtdm/drvlib.c >>>> +++ b/kernel/cobalt/rtdm/drvlib.c >>>> @@ -1761,6 +1761,7 @@ static int mmap_iomem_helper(struct vm_area_struct >>>> *vma, phys_addr_t pa) >>>> { >>>> pgprot_t prot = PAGE_SHARED; >>>> unsigned long len; >>>> + int ret; >>>> >>>> len = vma->vm_end - vma->vm_start; >>>> #ifndef CONFIG_MMU >>>> @@ -1774,8 +1775,15 @@ static int mmap_iomem_helper(struct vm_area_struct >>>> *vma, phys_addr_t pa) >>>> #endif >>>> vma->vm_page_prot = pgprot_noncached(prot); >>>> >>>> - return remap_pfn_range(vma, vma->vm_start, pa >> PAGE_SHIFT, >>>> + ret = remap_pfn_range(vma, vma->vm_start, pa >> PAGE_SHIFT, >>>> len, vma->vm_page_prot); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + if (cobalt_machine.prefault) >>>> + cobalt_machine.prefault(vma); >>>> + >>>> + return ret; >>>> } >>>> >>>> static int mmap_buffer_helper(struct rtdm_fd *fd, struct vm_area_struct >>>> *vma) >>> >>> Wow, that was likely broken by the refactoring in c8e9e166, long ago. >>> >>> Applied to next >>> >> >> The prefault hook has always been specifically about COW-breaking, I/O >> memory has no business with this, so there is no point in having the >> iomem helper calling the prefaulting hook. >> >> I suspect that rtdm_mmap_to_user() should be called instead of >> rtdm_iomap_to_user() in the case at hand. >> > > If Gunter is mapping IO memory, rtdm_mmap_to_user is surely not the > right thing. The xenomai2 implementation had a single helper dealing with I/O and kernel memory mappings (from virtual and linear memory) altogether. So I would not find impossible that wrong assumptions could be made from this implementation. > > Could it be that the prefault callback also has the side effect of > setting all page table entries that would otherwise only be filled lazily? Yes, this could prevent PTE misses, however I would expect minor faults to take place, those should be handled directly from the out-of-band stage. Otherwise, this _might_ be an issue with the interrupt pipeline used. The prefault for ARM was kind of a hack to work around shortcomings from the generic COW-breaking mechanism implemented by the I-pipe on ARM (IIRC, this had to do with LPAE support). -- Philippe.
