Philippe Gerum wrote:
> Jan Kiszka wrote:
>> Philippe Gerum wrote:
>>> Jan Kiszka wrote:
>>>> Philippe Gerum wrote:
>>>>> Jan Kiszka wrote:
>>>>>> Philippe Gerum wrote:
>>>>>>> Jan Kiszka wrote:
>>>>>>>> Philippe Gerum wrote:
>>>>>>>>> Jan Kiszka wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> doesn't this patch [1] have some relevance for us as well? As we use
>>>>>>>>>> xnarch_remap_io_page_range also for non-IO memory, I'm hesitating to
>>>>>>>>>> suggest that we apply this unconditionally at xnarch level. Ideas 
>>>>>>>>>> welcome.
>>>>>>>>>>
>>>>>>>>> Yes, I think it makes a lot of sense on powerpc at least, since doing 
>>>>>>>>> so will
>>>>>>>>> set the PAGE_GUARDED bit as well, and we obviously want to avoid any
>>>>>>>>> out-of-order access of I/O memory.
>>>>>>>>>
>>>>>>>>> (I don't see the reason to force the VM_RESERVED and VM_IO on the vma 
>>>>>>>>> though,
>>>>>>>>> since remap_pfn_range will do it anyway.)
>>>>>>>> No, I was talking about cases where we may pass kmalloc'ed memory to
>>>>>>>> xnarch_remap_io_page_range. In that case, caching and out-of-order
>>>>>>>> access may be desired for performance reasons.
>>>>>>>>
>>>>>>> xnarch_remap_io_page_range is intended for I/O memory only, some 
>>>>>>> assumptions are
>>>>>>> made on this. rtdm_mmap_buffer() should be fixed; it would be much 
>>>>>>> better to
>>>>>>> define another internal interface at xnarch level to specifically 
>>>>>>> perform
>>>>>>> kmalloc mapping.
>>>>>> Yeah, probably. But I think the issue is not just limited to RTDM. The
>>>>>> xnheap can be kmalloc-hosted as well.
>>>>>>
>>>>> This one is used with DMA memory. What I would suggest, is something like 
>>>>> this:
>>>>>
>>>>> --- ksrc/skins/rtdm/drvlib.c      (revision 3590)
>>>>> +++ ksrc/skins/rtdm/drvlib.c      (working copy)
>>>>> @@ -1738,9 +1738,12 @@
>>>>>           }
>>>>>           return 0;
>>>>>   } else
>>>>> -#endif /* CONFIG_MMU */
>>>>>           return xnarch_remap_io_page_range(vma, maddr, paddr,
>>>>>                                             size, PAGE_SHARED);
>>>>> +#else
>>>>> + return xnarch_remap_kmem_page_range(vma, maddr, paddr,
>>>>> +                                     size, PAGE_SHARED);
>>>>> +#endif /* CONFIG_MMU */
>>>>>  }
>>>>>
>>>>>  static struct file_operations rtdm_mmap_fops = {
>>>>>
>>>>>
>>>>> I.e. split the cases where MMU is absent from the one where MMU is there 
>>>>> but we
>>>>> come from rtdm_iomap_to_user.
>>>> Makes no sense to me yet. With CONFIG_MMU we have 3 cases, not just two,
>>>> no? We have to use mmap_data->src_paddr to tell kmem_page apart from
>>>> io_page.
>>> That's what the patch I sent right after this one does.
>> Nope, vaddr can be NULL in both cases, only paddr is differentiating.
>>
>> This one should do what we need:
>>
>> Index: ksrc/skins/rtdm/drvlib.c
>> ===================================================================
>> --- ksrc/skins/rtdm/drvlib.c (Revision 3594)
>> +++ ksrc/skins/rtdm/drvlib.c (Arbeitskopie)
>> @@ -1739,8 +1739,12 @@ static int rtdm_mmap_buffer(struct file 
>>              return 0;
>>      } else
>>  #endif /* CONFIG_MMU */
>> +    if (mmap_data->src_paddr)
>>              return xnarch_remap_io_page_range(vma, maddr, paddr,
>>                                                size, PAGE_SHARED);
>> +    else
>> +            return xnarch_remap_kmem_page_range(vma, maddr, paddr,
>> +                                                size, PAGE_SHARED);
>>  }
>>  
>>  static struct file_operations rtdm_mmap_fops = {
>>
>>
> 
> Ok. So if that's fine with you, I think we should merge that because the 
> current
> situation is error-prone.

Will do, patches for 2.3..trunk in stand-by for now. Do you have a
xnarch_remap_kmem_page_range definition at hand?

> 
> For the time being, I've defined a remap_kmem wrapper which mirrors the 
> previous
> actions of the remap_io one, before I fixed the latter with the noncache
> protection bits. At least we should get the same behaviour than previously wrt
> to rtdm_mmap_buffer. This leaves some time to think about the kmem mapping 
> modes
> without breaking the current situation, but they should be correct now.
> 

OK.

Jan

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core

Reply via email to