On 15.06.22 10:30, Philippe Gerum wrote:
>
> Jan Kiszka <jan.kis...@siemens.com> writes:
>
>> On 15.06.22 09:54, Philippe Gerum wrote:
>>>
>>> Jan Kiszka via Xenomai <xenomai@xenomai.org> writes:
>>>
>>>> On 23.05.22 16:04, Gunter Grau via Xenomai wrote:
>>>>> From: Gunter Grau <gunter.g...@philips.com>
>>>>>
>>>>> 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 <gunter.g...@philips.com>
>>>>> ---
>>>>> 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).
>
Gunter, could you take a function-trace of that very first fault without
your patch applied? Then we may see better what actually happens here,
specifically if your patch just happens to paper over a real issue.
BTW, you only tested it with I-pipe kernels so far, or did you also see
the issue with dovetail (5.10+)?
Jan
--
Siemens AG, Technology
Competence Center Embedded Linux
