[PATCH] rtdm/drvlib: Prevent pagefaults on arm on io mapping

[PATCH] rtdm/drvlib: Prevent pagefaults on arm on io mapping

[Gunter Grau]

From: Gunter Grau <gunter.grau@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.grau@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)
-- 
2.25.1

Re: [PATCH] rtdm/drvlib: Prevent pagefaults on arm on io mapping

[Jan Kiszka]

On 23.05.22 16:04, Gunter Grau via Xenomai wrote:
> From: Gunter Grau <gunter.grau@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.grau@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

Thanks,
Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux

Re: [PATCH] rtdm/drvlib: Prevent pagefaults on arm on io mapping

[Philippe Gerum]

Jan Kiszka via Xenomai <xenomai@xenomai.org> writes:

> On 23.05.22 16:04, Gunter Grau via Xenomai wrote:
>> From: Gunter Grau <gunter.grau@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.grau@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.

-- 
Philippe.

Re: [PATCH] rtdm/drvlib: Prevent pagefaults on arm on io mapping

[Jan Kiszka]

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.grau@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.grau@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.

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?

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux

RE: [PATCH] rtdm/drvlib: Prevent pagefaults on arm on io mapping

[Grau, Gunter]

Hi,

Your right, for io memory it does not make sense to do COW.
Nevertheless we see a page fault for each page after mapping on first access.
We also saw this at first when we used Xenomai 2.6 on 32bit arm, where it was fixed.

It now happened again when porting to Xenomai3. I was hoping the linux kernel whould now behave in an other way for io memory, but look like it stayed the same at least for 32Bit arm. So I checked how it was handled in mmap and copied the solution from there.

Regards,
Gunter

> -----Original Message-----
> From: Jan Kiszka <jan.kiszka@siemens.com>
> Sent: Mittwoch, 15. Juni 2022 10:12
> To: Philippe Gerum <rpm@xenomai.org>; Grau, Gunter
> <gunter.grau@philips.com>
> Cc: xenomai@xenomai.org
> Subject: Re: [PATCH] rtdm/drvlib: Prevent pagefaults on arm on io mapping
>
> Caution: This e-mail originated from outside of Philips, be careful for
> phishing.
>
>
> 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.grau@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.grau@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.
>
> 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?
>
> Jan
>
> --
> Siemens AG, Technology
> Competence Center Embedded Linux

________________________________
The information contained in this message may be confidential and legally protected under applicable law. The message is intended solely for the addressee(s). If you are not the intended recipient, you are hereby notified that any use, forwarding, dissemination, or reproduction of this message is strictly prohibited and may be unlawful. If you are not the intended recipient, please contact the sender by return e-mail and destroy all copies of the original message.

Re: [PATCH] rtdm/drvlib: Prevent pagefaults on arm on io mapping

[Philippe Gerum]

Jan Kiszka <jan.kiszka@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.grau@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.grau@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).

-- 
Philippe.

Re: [PATCH] rtdm/drvlib: Prevent pagefaults on arm on io mapping

[Jan Kiszka]

On 15.06.22 10:30, Philippe Gerum wrote:
> 
> Jan Kiszka <jan.kiszka@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.grau@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.grau@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

Re: [PATCH] rtdm/drvlib: Prevent pagefaults on arm on io mapping

[Jan Kiszka]

[forgot to update list address...]

On 20.06.22 08:38, Jan Kiszka wrote:
> On 15.06.22 11:55, Jan Kiszka via Xenomai wrote:
>> On 15.06.22 10:30, Philippe Gerum wrote:
>>>
>>> Jan Kiszka <jan.kiszka@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.grau@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.grau@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+)?
>>
> 
> Gentle ping on this because I'd like to decide whether to move the patch
> forward to master - and possibly stable trees - or drop it from next again.
> 
> Thanks,
> Jan
> 

RE: [PATCH] rtdm/drvlib: Prevent pagefaults on arm on io mapping

[Grau, Gunter]

> -----Original Message-----
> From: Jan Kiszka <jan.kiszka@siemens.com>
> Sent: Montag, 20. Juni 2022 08:40
> To: Grau, Gunter <gunter.grau@philips.com>
> Cc: Philippe Gerum <rpm@xenomai.org>; xenomai@lists.linux.dev
> Subject: Re: [PATCH] rtdm/drvlib: Prevent pagefaults on arm on io mapping
>
> Caution: This e-mail originated from outside of Philips, be careful for
> phishing.
>
>
> [forgot to update list address...]
>
> On 20.06.22 08:38, Jan Kiszka wrote:
> > On 15.06.22 11:55, Jan Kiszka via Xenomai wrote:
> >> On 15.06.22 10:30, Philippe Gerum wrote:
> >>>
> >>> Jan Kiszka <jan.kiszka@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.grau@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.grau@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+)?
> >>
> >
> > Gentle ping on this because I'd like to decide whether to move the
> > patch forward to master - and possibly stable trees - or drop it from next
> again.
> >

Hi,

I will try to test it again without the patch and do the function trace within this week.
Current setup is on imx6/7 5.4 kernel and ipipe.

I don't think we will find the time to setup another kernel or dovetail.

Thanks,
Gunter

> > Thanks,
> > Jan
> >

________________________________
The information contained in this message may be confidential and legally protected under applicable law. The message is intended solely for the addressee(s). If you are not the intended recipient, you are hereby notified that any use, forwarding, dissemination, or reproduction of this message is strictly prohibited and may be unlawful. If you are not the intended recipient, please contact the sender by return e-mail and destroy all copies of the original message.

RE: [PATCH] rtdm/drvlib: Prevent pagefaults on arm on io mapping

[Grau, Gunter]

> >
> > [forgot to update list address...]
> >
> > On 20.06.22 08:38, Jan Kiszka wrote:
> > > On 15.06.22 11:55, Jan Kiszka via Xenomai wrote:
> > >> On 15.06.22 10:30, Philippe Gerum wrote:
> > >>>
> > >>> Jan Kiszka <jan.kiszka@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.grau@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.grau@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+)?
> > >>
> > >
> > > Gentle ping on this because I'd like to decide whether to move the
> > > patch forward to master - and possibly stable trees - or drop it
> > > from next
> > again.
> > >
>
> Hi,
>
> I will try to test it again without the patch and do the function trace within
> this week.
> Current setup is on imx6/7 5.4 kernel and ipipe.
>
> I don't think we will find the time to setup another kernel or dovetail.
>
> Thanks,
> Gunter
>

Hi,

I added a dump_stack() in cobalt_assert_nrt() where the XCPU signal is generated.
Maybe you can give me advice if you need better information.

[   13.690407] CPU: 1 PID: 581 Comm: UT1  Tainted: G           O      5.4.191-00703-gbfba1d43d087-dirty #23
[   13.699901] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
[   13.714083] I-pipe domain: Linux
[   13.717347] [<8010f314>] (unwind_backtrace) from [<8010b824>] (show_stack+0x10/0x14)
[   13.725118] [<8010b824>] (show_stack) from [<8084604c>] (dump_stack+0xd8/0xf4)
[   13.732365] [<8084604c>] (dump_stack) from [<801d8318>] (ipipe_trap_hook+0x1b0/0x33c)
[   13.748287] [<801d8318>] (ipipe_trap_hook) from [<801be9d0>] (__ipipe_notify_trap+0x98/0xdc)
[   13.748304] [<801be9d0>] (__ipipe_notify_trap) from [<80114114>] (do_page_fault+0x20/0x444)
[   13.748317] [<80114114>] (do_page_fault) from [<80114800>] (do_DataAbort+0x3c/0xe0)
[   13.782231] [<80114800>] (do_DataAbort) from [<80101f7c>] (__dabt_usr+0x3c/0x40)
[   13.789655] Exception stack(0xc2129fb0 to 0xc2129ff8)
[   13.794735] 9fa0:                                     6c1d7000 00000600 72c61910 0000013c
[   13.802954] 9fc0: 72bff024 72c618e8 004247c8 76ba5000 ffffffff 00000000 004247c8 6a87da3c
[   13.811155] 9fe0: 6c1d9000 6a87da20 00020000 72bb5c68 20010010 ffffffff

Thanks in advance,
Gunter

> > > Thanks,
> > > Jan
> > >
>
> ________________________________
> The information contained in this message may be confidential and legally
> protected under applicable law. The message is intended solely for the
> addressee(s). If you are not the intended recipient, you are hereby notified
> that any use, forwarding, dissemination, or reproduction of this message is
> strictly prohibited and may be unlawful. If you are not the intended recipient,
> please contact the sender by return e-mail and destroy all copies of the
> original message.

________________________________
The information contained in this message may be confidential and legally protected under applicable law. The message is intended solely for the addressee(s). If you are not the intended recipient, you are hereby notified that any use, forwarding, dissemination, or reproduction of this message is strictly prohibited and may be unlawful. If you are not the intended recipient, please contact the sender by return e-mail and destroy all copies of the original message.

Re: [PATCH] rtdm/drvlib: Prevent pagefaults on arm on io mapping

[Jan Kiszka]

On 20.06.22 12:44, Grau, Gunter wrote:
> Hi,
> 
> I added a dump_stack() in cobalt_assert_nrt() where the XCPU signal is generated.
> Maybe you can give me advice if you need better information.
> 
> [   13.690407] CPU: 1 PID: 581 Comm: UT1  Tainted: G           O      5.4.191-00703-gbfba1d43d087-dirty #23
> [   13.699901] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> [   13.714083] I-pipe domain: Linux
> [   13.717347] [<8010f314>] (unwind_backtrace) from [<8010b824>] (show_stack+0x10/0x14)
> [   13.725118] [<8010b824>] (show_stack) from [<8084604c>] (dump_stack+0xd8/0xf4)
> [   13.732365] [<8084604c>] (dump_stack) from [<801d8318>] (ipipe_trap_hook+0x1b0/0x33c)
> [   13.748287] [<801d8318>] (ipipe_trap_hook) from [<801be9d0>] (__ipipe_notify_trap+0x98/0xdc)
> [   13.748304] [<801be9d0>] (__ipipe_notify_trap) from [<80114114>] (do_page_fault+0x20/0x444)
> [   13.748317] [<80114114>] (do_page_fault) from [<80114800>] (do_DataAbort+0x3c/0xe0)
> [   13.782231] [<80114800>] (do_DataAbort) from [<80101f7c>] (__dabt_usr+0x3c/0x40)
> [   13.789655] Exception stack(0xc2129fb0 to 0xc2129ff8)
> [   13.794735] 9fa0:                                     6c1d7000 00000600 72c61910 0000013c
> [   13.802954] 9fc0: 72bff024 72c618e8 004247c8 76ba5000 ffffffff 00000000 004247c8 6a87da3c
> [   13.811155] 9fe0: 6c1d9000 6a87da20 00020000 72bb5c68 20010010 ffffffff

OK, a regular page fault that the out-of-band code in I-pipe was not
able to fix up. The question is whether we lack some logic there to do
that or if that fixup really requires Linux.

Can you trace which resolution Linux applies to this after switching to
non-RT? Or do you already have a suspicion, Philippe?

Can this be reproduced in qemu-arm as well? With some dummy device and
driver? Would allow to examine it also with Dovetail.

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux

Re: [PATCH] rtdm/drvlib: Prevent pagefaults on arm on io mapping

[Philippe Gerum]

Jan Kiszka <jan.kiszka@siemens.com> writes:

> On 20.06.22 12:44, Grau, Gunter wrote:
>> Hi,
>> 
>> I added a dump_stack() in cobalt_assert_nrt() where the XCPU signal is generated.
>> Maybe you can give me advice if you need better information.
>> 
>> [   13.690407] CPU: 1 PID: 581 Comm: UT1  Tainted: G           O      5.4.191-00703-gbfba1d43d087-dirty #23
>> [   13.699901] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
>> [   13.714083] I-pipe domain: Linux
>> [   13.717347] [<8010f314>] (unwind_backtrace) from [<8010b824>] (show_stack+0x10/0x14)
>> [   13.725118] [<8010b824>] (show_stack) from [<8084604c>] (dump_stack+0xd8/0xf4)
>> [   13.732365] [<8084604c>] (dump_stack) from [<801d8318>] (ipipe_trap_hook+0x1b0/0x33c)
>> [   13.748287] [<801d8318>] (ipipe_trap_hook) from [<801be9d0>] (__ipipe_notify_trap+0x98/0xdc)
>> [   13.748304] [<801be9d0>] (__ipipe_notify_trap) from [<80114114>] (do_page_fault+0x20/0x444)
>> [   13.748317] [<80114114>] (do_page_fault) from [<80114800>] (do_DataAbort+0x3c/0xe0)
>> [   13.782231] [<80114800>] (do_DataAbort) from [<80101f7c>] (__dabt_usr+0x3c/0x40)
>> [   13.789655] Exception stack(0xc2129fb0 to 0xc2129ff8)
>> [   13.794735] 9fa0:                                     6c1d7000 00000600 72c61910 0000013c
>> [   13.802954] 9fc0: 72bff024 72c618e8 004247c8 76ba5000 ffffffff 00000000 004247c8 6a87da3c
>> [   13.811155] 9fe0: 6c1d9000 6a87da20 00020000 72bb5c68 20010010 ffffffff
>
> OK, a regular page fault that the out-of-band code in I-pipe was not
> able to fix up. The question is whether we lack some logic there to do
> that or if that fixup really requires Linux.
>
> Can you trace which resolution Linux applies to this after switching to
> non-RT? Or do you already have a suspicion, Philippe?
>
> Can this be reproduced in qemu-arm as well? With some dummy device and
> driver? Would allow to examine it also with Dovetail.
>
> Jan

The I-pipe is wrong at:
https://source.denx.de/Xenomai/ipipe-arm/-/blob/ipipe/master/arch/arm/mm/fault.c#L564

Reporting a trap entry to the real-time core should be postponed until
__do_kernel_fault() can attempt to fixup the exception, in which case
we would remain on the oob stage as expected:
https://source.denx.de/Xenomai/ipipe-arm/-/blob/ipipe/master/arch/arm/mm/fault.c#L265
https://source.denx.de/Xenomai/ipipe-arm/-/blob/ipipe/master/arch/arm/mm/fault.c#L200

Dovetail has it right:
https://source.denx.de/Xenomai/linux-dovetail/-/blob/v5.19-dovetail-rebase/arch/arm/mm/fault.c#L557
https://source.denx.de/Xenomai/linux-dovetail/-/blob/v5.19-dovetail-rebase/arch/arm/mm/fault.c#L279
https://source.denx.de/Xenomai/linux-dovetail/-/blob/v5.19-dovetail-rebase/arch/arm/mm/fault.c#L205

-- 
Philippe.

RE: [PATCH] rtdm/drvlib: Prevent pagefaults on arm on io mapping

[Grau, Gunter]

> -----Original Message-----
> From: Philippe Gerum <rpm@xenomai.org>
> Sent: Montag, 20. Juni 2022 15:58
> To: Jan Kiszka <jan.kiszka@siemens.com>
> Cc: Grau, Gunter <gunter.grau@philips.com>; xenomai@lists.linux.dev
> Subject: Re: [PATCH] rtdm/drvlib: Prevent pagefaults on arm on io mapping
>
> Caution: This e-mail originated from outside of Philips, be careful for
> phishing.
>
>
> Jan Kiszka <mailto:jan.kiszka@siemens.com> writes:
>
> > On 20.06.22 12:44, Grau, Gunter wrote:
> >> Hi,
> >>
> >> I added a dump_stack() in cobalt_assert_nrt() where the XCPU signal is
> generated.
> >> Maybe you can give me advice if you need better information.
> >>
> >> [   13.690407] CPU: 1 PID: 581 Comm: UT1  Tainted: G           O      5.4.191-
> 00703-gbfba1d43d087-dirty #23
> >> [   13.699901] Hardware name: Freescale i.MX6 Quad/DualLite (Device
> Tree)
> >> [   13.714083] I-pipe domain: Linux
> >> [   13.717347] [<8010f314>] (unwind_backtrace) from [<8010b824>]
> (show_stack+0x10/0x14)
> >> [   13.725118] [<8010b824>] (show_stack) from [<8084604c>]
> (dump_stack+0xd8/0xf4)
> >> [   13.732365] [<8084604c>] (dump_stack) from [<801d8318>]
> (ipipe_trap_hook+0x1b0/0x33c)
> >> [   13.748287] [<801d8318>] (ipipe_trap_hook) from [<801be9d0>]
> (__ipipe_notify_trap+0x98/0xdc)
> >> [   13.748304] [<801be9d0>] (__ipipe_notify_trap) from [<80114114>]
> (do_page_fault+0x20/0x444)
> >> [   13.748317] [<80114114>] (do_page_fault) from [<80114800>]
> (do_DataAbort+0x3c/0xe0)
> >> [   13.782231] [<80114800>] (do_DataAbort) from [<80101f7c>]
> (__dabt_usr+0x3c/0x40)
> >> [   13.789655] Exception stack(0xc2129fb0 to 0xc2129ff8)
> >> [   13.794735] 9fa0:                                     6c1d7000 00000600 72c61910
> 0000013c
> >> [   13.802954] 9fc0: 72bff024 72c618e8 004247c8 76ba5000 ffffffff 00000000
> 004247c8 6a87da3c
> >> [   13.811155] 9fe0: 6c1d9000 6a87da20 00020000 72bb5c68 20010010
> ffffffff
> >
> > OK, a regular page fault that the out-of-band code in I-pipe was not
> > able to fix up. The question is whether we lack some logic there to do
> > that or if that fixup really requires Linux.
> >
> > Can you trace which resolution Linux applies to this after switching
> > to non-RT? Or do you already have a suspicion, Philippe?
> >
> > Can this be reproduced in qemu-arm as well? With some dummy device
> and
> > driver? Would allow to examine it also with Dovetail.
> >
> > Jan
>
> The I-pipe is wrong at:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsource.denx.de%2FXenomai%2Fipipe-arm%2F-%2Fblob%2Fipipe%2Fmaster%2Farch%2Farm%2Fmm%2Ffault.c%23L564&amp;data=05%7C01%7C%7Ca81ce9af590246e89c5708da52c6a6c7%7C1a407a2d76754d178692b3ac285306e4%7C0%7C0%7C637913310397946190%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=XCYaeQPDGA0EPfI9gYsRxd%2BC%2BUQz44qju8%2FC4Qyh%2BDs%3D&amp;reserved=0
>
> Reporting a trap entry to the real-time core should be postponed until
> __do_kernel_fault() can attempt to fixup the exception, in which case we
> would remain on the oob stage as expected:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsource.denx.de%2FXenomai%2Fipipe-arm%2F-%2Fblob%2Fipipe%2Fmaster%2Farch%2Farm%2Fmm%2Ffault.c%23L265&amp;data=05%7C01%7C%7Ca81ce9af590246e89c5708da52c6a6c7%7C1a407a2d76754d178692b3ac285306e4%7C0%7C0%7C637913310397946190%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=rMez063%2FtfoTQKltrz3C6v6KCXTKOHI0sHUNAQYExys%3D&amp;reserved=0
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsource.denx.de%2FXenomai%2Fipipe-arm%2F-%2Fblob%2Fipipe%2Fmaster%2Farch%2Farm%2Fmm%2Ffault.c%23L200&amp;data=05%7C01%7C%7Ca81ce9af590246e89c5708da52c6a6c7%7C1a407a2d76754d178692b3ac285306e4%7C0%7C0%7C637913310397946190%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=bwsKfI%2Bb5MlhfWd%2BT56oGm9k8nIDxdk%2FXA3WXBMnXHs%3D&amp;reserved=0
>
> Dovetail has it right:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsource.denx.de%2FXenomai%2Flinux-dovetail%2F-%2Fblob%2Fv5.19-dovetail-rebase%2Farch%2Farm%2Fmm%2Ffault.c%23L557&amp;data=05%7C01%7C%7Ca81ce9af590246e89c5708da52c6a6c7%7C1a407a2d76754d178692b3ac285306e4%7C0%7C0%7C637913310397946190%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=d1IUSRRXjRwAvWHubBohFacwYnu5Fpd8hWqU%2Fq73Mi8%3D&amp;reserved=0
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsource.denx.de%2FXenomai%2Flinux-dovetail%2F-%2Fblob%2Fv5.19-dovetail-rebase%2Farch%2Farm%2Fmm%2Ffault.c%23L279&amp;data=05%7C01%7C%7Ca81ce9af590246e89c5708da52c6a6c7%7C1a407a2d76754d178692b3ac285306e4%7C0%7C0%7C637913310397946190%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=uF3eUviJ2mYLJCS6KkbV8ceyTrJWMa4M%2FK0cYVM7XFc%3D&amp;reserved=0
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsource.denx.de%2FXenomai%2Flinux-dovetail%2F-%2Fblob%2Fv5.19-dovetail-rebase%2Farch%2Farm%2Fmm%2Ffault.c%23L205&amp;data=05%7C01%7C%7Ca81ce9af590246e89c5708da52c6a6c7%7C1a407a2d76754d178692b3ac285306e4%7C0%7C0%7C637913310397946190%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=bwb6tbky5xH7qgIJ1mUuM0JSfoVHGbd4jnB%2FMYPzYuo%3D&amp;reserved=0
>
> --
> Philippe.

Hi,

In complete absence of knowledge what I am doing I removed the extra code in fault_mm.c  😉 :

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index f712fa20aea2..2b46c67985cf 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -558,9 +558,6 @@ do_translation_fault(unsigned long addr, unsigned int fsr,
        return 0;

 bad_area:
-       if (__ipipe_report_trap(IPIPE_TRAP_ACCESS, regs))
-               return 0;
-
        irqflags = fault_entry(regs);

        do_bad_area(addr, fsr, regs);

I can confirm, that this works for me then without my patch.

Thanks,
Gunter

________________________________
The information contained in this message may be confidential and legally protected under applicable law. The message is intended solely for the addressee(s). If you are not the intended recipient, you are hereby notified that any use, forwarding, dissemination, or reproduction of this message is strictly prohibited and may be unlawful. If you are not the intended recipient, please contact the sender by return e-mail and destroy all copies of the original message.

Re: [PATCH] rtdm/drvlib: Prevent pagefaults on arm on io mapping

[Jan Kiszka]

On 20.06.22 16:54, Grau, Gunter wrote:
> 
> 
>> -----Original Message-----
>> From: Philippe Gerum <rpm@xenomai.org>
>> Sent: Montag, 20. Juni 2022 15:58
>> To: Jan Kiszka <jan.kiszka@siemens.com>
>> Cc: Grau, Gunter <gunter.grau@philips.com>; xenomai@lists.linux.dev
>> Subject: Re: [PATCH] rtdm/drvlib: Prevent pagefaults on arm on io mapping
>>
>> Caution: This e-mail originated from outside of Philips, be careful for
>> phishing.
>>
>>
>> Jan Kiszka <mailto:jan.kiszka@siemens.com> writes:
>>
>>> On 20.06.22 12:44, Grau, Gunter wrote:
>>>> Hi,
>>>>
>>>> I added a dump_stack() in cobalt_assert_nrt() where the XCPU signal is
>> generated.
>>>> Maybe you can give me advice if you need better information.
>>>>
>>>> [   13.690407] CPU: 1 PID: 581 Comm: UT1  Tainted: G           O      5.4.191-
>> 00703-gbfba1d43d087-dirty #23
>>>> [   13.699901] Hardware name: Freescale i.MX6 Quad/DualLite (Device
>> Tree)
>>>> [   13.714083] I-pipe domain: Linux
>>>> [   13.717347] [<8010f314>] (unwind_backtrace) from [<8010b824>]
>> (show_stack+0x10/0x14)
>>>> [   13.725118] [<8010b824>] (show_stack) from [<8084604c>]
>> (dump_stack+0xd8/0xf4)
>>>> [   13.732365] [<8084604c>] (dump_stack) from [<801d8318>]
>> (ipipe_trap_hook+0x1b0/0x33c)
>>>> [   13.748287] [<801d8318>] (ipipe_trap_hook) from [<801be9d0>]
>> (__ipipe_notify_trap+0x98/0xdc)
>>>> [   13.748304] [<801be9d0>] (__ipipe_notify_trap) from [<80114114>]
>> (do_page_fault+0x20/0x444)
>>>> [   13.748317] [<80114114>] (do_page_fault) from [<80114800>]
>> (do_DataAbort+0x3c/0xe0)
>>>> [   13.782231] [<80114800>] (do_DataAbort) from [<80101f7c>]
>> (__dabt_usr+0x3c/0x40)
>>>> [   13.789655] Exception stack(0xc2129fb0 to 0xc2129ff8)
>>>> [   13.794735] 9fa0:                                     6c1d7000 00000600 72c61910
>> 0000013c
>>>> [   13.802954] 9fc0: 72bff024 72c618e8 004247c8 76ba5000 ffffffff 00000000
>> 004247c8 6a87da3c
>>>> [   13.811155] 9fe0: 6c1d9000 6a87da20 00020000 72bb5c68 20010010
>> ffffffff
>>>
>>> OK, a regular page fault that the out-of-band code in I-pipe was not
>>> able to fix up. The question is whether we lack some logic there to do
>>> that or if that fixup really requires Linux.
>>>
>>> Can you trace which resolution Linux applies to this after switching
>>> to non-RT? Or do you already have a suspicion, Philippe?
>>>
>>> Can this be reproduced in qemu-arm as well? With some dummy device
>> and
>>> driver? Would allow to examine it also with Dovetail.
>>>
>>> Jan
>>
>> The I-pipe is wrong at:
>> https://source.denx.de/Xenomai/ipipe-arm/-/blob/ipipe/master/arch/arm/mm/fault.c#L564
>>
>> Reporting a trap entry to the real-time core should be postponed until
>> __do_kernel_fault() can attempt to fixup the exception, in which case we
>> would remain on the oob stage as expected:
>> https://source.denx.de/Xenomai/ipipe-arm/-/blob/ipipe/master/arch/arm/mm/fault.c#L265
>> https://source.denx.de/Xenomai/ipipe-arm/-/blob/ipipe/master/arch/arm/mm/fault.c#L200
>>
>> Dovetail has it right:
>> https://source.denx.de/Xenomai/linux-dovetail/-/blob/v5.19-dovetail-rebase/arch/arm/mm/fault.c#L557
>> https://source.denx.de/Xenomai/linux-dovetail/-/blob/v5.19-dovetail-rebase/arch/arm/mm/fault.c#L279
>> https://source.denx.de/Xenomai/linux-dovetail/-/blob/v5.19-dovetail-rebase/arch/arm/mm/fault.c#L205
>>
>> --
>> Philippe.
> 
> Hi,
> 
> In complete absence of knowledge what I am doing I removed the extra code in fault_mm.c  😉 :
> 
> diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
> index f712fa20aea2..2b46c67985cf 100644
> --- a/arch/arm/mm/fault.c
> +++ b/arch/arm/mm/fault.c
> @@ -558,9 +558,6 @@ do_translation_fault(unsigned long addr, unsigned int fsr,
>         return 0;
> 
>  bad_area:
> -       if (__ipipe_report_trap(IPIPE_TRAP_ACCESS, regs))
> -               return 0;
> -
>         irqflags = fault_entry(regs);
> 
>         do_bad_area(addr, fsr, regs);
> 
> I can confirm, that this works for me then without my patch.
> 

Yeah, but this is not getting us to the solution. I-pipe in general, not
just on arm, worked differently /wrt when a page fault was reported to
the RT core. That could be changed, but I don't think it's worth the
hassle anymore.

In that light, we should take your patch with an updated commit log to
stable/v3.2.x and older. However, I will drop it now from next which is
dovetail-only, thus not affected by this.

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux