Question about ION carveout heap support partial cache flush

Question about ION carveout heap support partial cache flush

[zhangfei gao]

On Mon, Aug 27, 2012 at 1:46 PM, Haojian Zhuang
<haojian.zhuang@gmail.com> wrote:
> On Mon, Aug 27, 2012 at 9:51 AM, zhangfei gao <zhangfei.gao@gmail.com> wrote:
>> Hi, All
>>
>> We met question about dmac_map_area & dmac_flush_range from user addr.
>> mcr would not return on armv7 processor.
>>
>> Existing ion carveout heap does not support partial cache flush.
>> Total cache will be flushed at all.
>>
>> There is only one dirty bit for carveout heap, as well as sg_table->nents.
>> drivers/gpu/ion/ion_carveout_heap.c
>> ion_carveout_heap_map_dma -> sg_alloc_table(table, 1, GFP_KERNEL);
>> ion_buffer_alloc_dirty -> pages = buffer->sg_table->nents;
>>
>> We want to support partial cache flush.
>> Align to cache line, instead of PAGE_SIZE, for efficiency consideration.
>> We have considered extended dirty bit, but looks like only align to PAGE_SIZE.
>>
>> For experiment we modify ioctl ION_IOC_SYNC on armv7.
>> And directly use dmac_map_area & dmac_flush_range with add from user space.
>> However, we find dmac_map_area can not work with this addr from user space.
>> In fact, it is  mcr can not work with addr from user space, it would hung.
>
> Let me summerize it. First, user space address is mapped. Then,
> flushing user space
> address is triggered. It's a workaround of fixing non-existed virtual
> address without fixing
> vmap() or any other solution. It's just a quick fix.
>
> Zhangfei,
> I doubt that the issue may be caused by missing memory barrier.
> Flushing is using
> coprocessor instructions. It's a little different.

Is there any limitation that dmac_map_area & dmac_flush_range
supporting addr mapped from user space?
And mcr can not return with user space addr.
While __davt_svc -> page fault happen, even the page table has already
been set up.

Thanks

Question about ION carveout heap support partial cache flush

[Russell King - ARM Linux]

On Mon, Aug 27, 2012 at 04:23:31PM +0800, zhangfei gao wrote:
> On Mon, Aug 27, 2012 at 1:46 PM, Haojian Zhuang
> <haojian.zhuang@gmail.com> wrote:
> > Let me summerize it. First, user space address is mapped. Then,
> > flushing user space
> > address is triggered. It's a workaround of fixing non-existed virtual
> > address without fixing
> > vmap() or any other solution. It's just a quick fix.
> >
> > Zhangfei,
> > I doubt that the issue may be caused by missing memory barrier.
> > Flushing is using
> > coprocessor instructions. It's a little different.
> 
> Is there any limitation that dmac_map_area & dmac_flush_range
> supporting addr mapped from user space?

They DEFINITELY DO NOT SUPPORT FLUSHING USER SPACE.

Question about ION carveout heap support partial cache flush

[zhangfei gao]

On Mon, Aug 27, 2012 at 4:29 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Aug 27, 2012 at 04:23:31PM +0800, zhangfei gao wrote:
>> On Mon, Aug 27, 2012 at 1:46 PM, Haojian Zhuang
>> <haojian.zhuang@gmail.com> wrote:
>> > Let me summerize it. First, user space address is mapped. Then,
>> > flushing user space
>> > address is triggered. It's a workaround of fixing non-existed virtual
>> > address without fixing
>> > vmap() or any other solution. It's just a quick fix.
>> >
>> > Zhangfei,
>> > I doubt that the issue may be caused by missing memory barrier.
>> > Flushing is using
>> > coprocessor instructions. It's a little different.
>>
>> Is there any limitation that dmac_map_area & dmac_flush_range
>> supporting addr mapped from user space?
>
> They DEFINITELY DO NOT SUPPORT FLUSHING USER SPACE.

Thanks Russell

Any suggestion of flushing cache according to cache line, instead of PAGE_SIZE.
In order to get specific area, we use addr from user directly, which
may not be the PAGE start.

We have some usage case to flush cache according to cache line.
CPU - cache - ddr - gpu
1. For correctness, driver only flush used size, if align to PAGE_SIZE,
other area may be flushed by mistake.
2. for efficiency, cache line align will be perfered.

Thanks

Question about ION carveout heap support partial cache flush

[Haojian Zhuang]

On Mon, Aug 27, 2012 at 4:56 PM, zhangfei gao <zhangfei.gao@gmail.com> wrote:
>
> Any suggestion of flushing cache according to cache line, instead of PAGE_SIZE.
> In order to get specific area, we use addr from user directly, which
> may not be the PAGE start.
>
> We have some usage case to flush cache according to cache line.
> CPU - cache - ddr - gpu
> 1. For correctness, driver only flush used size, if align to PAGE_SIZE,
> other area may be flushed by mistake.
> 2. for efficiency, cache line align will be perfered.

I think that maybe flushing PAGE_SIZE is acceptable. ION/PMEM is always designed
for large memory sharing. Maybe you can monitor the partial flushing
user case, I doubt
they're always flushing several pages, not several cachelines.

Question about ION carveout heap support partial cache flush

[Haojian Zhuang]

On Mon, Aug 27, 2012 at 4:29 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Aug 27, 2012 at 04:23:31PM +0800, zhangfei gao wrote:
>> On Mon, Aug 27, 2012 at 1:46 PM, Haojian Zhuang
>> <haojian.zhuang@gmail.com> wrote:
>> > Let me summerize it. First, user space address is mapped. Then,
>> > flushing user space
>> > address is triggered. It's a workaround of fixing non-existed virtual
>> > address without fixing
>> > vmap() or any other solution. It's just a quick fix.
>> >
>> > Zhangfei,
>> > I doubt that the issue may be caused by missing memory barrier.
>> > Flushing is using
>> > coprocessor instructions. It's a little different.
>>
>> Is there any limitation that dmac_map_area & dmac_flush_range
>> supporting addr mapped from user space?
>
> They DEFINITELY DO NOT SUPPORT FLUSHING USER SPACE.

I agree that those API are defined to support kernel space address only.
But the implementation is only flushing address by assembly code.

If we don't care the original goal of API, the problem should be caused
by address mapping isn't set up yet. I doubt that the first step isn't fully
finished. Since the second step is using coprocessor instruction, that's
a shortcut. If we add delay such as memory barrier, the first step can
be really finished before the second step running.

Question about ION carveout heap support partial cache flush

[Russell King - ARM Linux]

On Mon, Aug 27, 2012 at 09:43:10PM +0800, Haojian Zhuang wrote:
> On Mon, Aug 27, 2012 at 4:29 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Mon, Aug 27, 2012 at 04:23:31PM +0800, zhangfei gao wrote:
> >> On Mon, Aug 27, 2012 at 1:46 PM, Haojian Zhuang
> >> <haojian.zhuang@gmail.com> wrote:
> >> > Let me summerize it. First, user space address is mapped. Then,
> >> > flushing user space
> >> > address is triggered. It's a workaround of fixing non-existed virtual
> >> > address without fixing
> >> > vmap() or any other solution. It's just a quick fix.
> >> >
> >> > Zhangfei,
> >> > I doubt that the issue may be caused by missing memory barrier.
> >> > Flushing is using
> >> > coprocessor instructions. It's a little different.
> >>
> >> Is there any limitation that dmac_map_area & dmac_flush_range
> >> supporting addr mapped from user space?
> >
> > They DEFINITELY DO NOT SUPPORT FLUSHING USER SPACE.
> 
> I agree that those API are defined to support kernel space address only.
> But the implementation is only flushing address by assembly code.

No.  Have you looked at what the dma_map_page() and scatterlist stuff
does?  It all wants to deal with a 'struct page'.  Have you looked at
how L2 caches are handled?  That wants to be able to use virt_to_phys
and similar to convert addresses, which only work on the linear mapped
region.

Have you thought about what happens when the user's page gets unmapped
or swapped out (or even just aged) ?  I don't think you've given any of
this any consideration what so ever.

There's more here than just "oh we can just pass random address X into
this function and hope that nothing goes wrong."  It's really not that
simple.