RE: [rpmsg PATCH v2 1/1] rpmsg: virtio_rpmsg_bus: fix unexpected huge vmap mappings

[Andy Duan <fugang.duan@nxp.com>]

From: Andy Duan Sent: 2018年12月28日 9:48
> From: Christoph Hellwig <hch@infradead.org> Sent: 2018年12月27日
> 20:19
> > On Thu, Dec 27, 2018 at 02:36:53AM +0000, Andy Duan wrote:
> > > Rpmsg is used to communicate with remote cpu like M4, the allocated
> > > memory is shared by Linux and M4 side. In general, Linux side
> > > reserved the static memory region like per-device DMA pool as
> > > coherent memory for the RPMSG receive/transmit buffers. For the
> > > static memory region, normal page allocator cannot match the
> > > requirement unless there have protocol to tell M4 the dynamic RPMSG
> receive/transmit buffers.
> >
> > In that case you need a OF reserved memory node, like we use for the
> > "shared-dma-pool" coherent or contiguous allocations.  Currently we
> > have those two variants wired up the the DMA allocator, but they can
> > also used directly by drivers.  To be honest I don't really like like
> > drivers getting too intimate with the memory allocator, but I also
> > don't think that providing a little glue code to instanciat a CMA pool
> > for a memory that can be used directly by the driver is much of an
> > issue.  Most of it could be reused from the existing code, just with a slightly
> lower level interfaces.
> >
> > > To stop to extract pages from dma_alloc_coherent, the rpmsg bus
> > > implementation base on virtio that already use the scatterlist
> > > mechanism for vring memory. So for virtio driver like RPMSG bus, we
> > > have to extract pages from dma_alloc_coherent.
> >
> > This sentence doesn't parse for me.
> 
> Virtio supply the APIs that require the scatterlist pages for virtio in/out buf:
> int virtqueue_add_inbuf(struct virtqueue *vq,
>                         struct scatterlist *sg, unsigned int num,
>                         void *data,
>                         gfp_t gfp)
> int virtqueue_add_outbuf(struct virtqueue *vq,
>                          struct scatterlist *sg, unsigned int num,
>                          void *data,
>                          gfp_t gfp)
> 
> 
> >
> > > I don't think the patch is one hack,  as we already know the
> > > physical address for the coherent memory,  just want to get pages,
> > > the interface "pfn_to_page(PHYS_PFN(x))" is very reasonable to the
> > > related pages.
> >
> > struct scatterlist doesn't (directly) refer to physical address, it
> > refers to page structures, which encode a kernel virtual address in
> > the kernel direct mapping, and we intentionally do not guarantee a
> > return in the kernel direct mapping for the DMA coherent allocator, as
> > in many cases we have to either allocate from a special pool, from a
> > special address window or remap memory to mark it as uncached.  How
> > that is done is an implenentation detail that is not exposed to drivers and
> may change at any time.
> >
> > > If you stick to use normal page allocator and streaming DMA API in
> > > RPMSG,  then we have to:
> > > - add new quirk feature for virtio like the same function as
> > > "VIRTIO_F_IOMMU_PLATFORM",
> >
> > You have to do that anyway.
> 
> I discuss with our team, use page allocator cannot match our requirement.
> i.MX8QM/QXP platforms have partition feature that limit M4 only access fixed
> ddr memory region. Suppose other platforms also have similar limitation for
> secure case.
> 
> So it requires to use OF reserved memory for the "shared-dma-pool" coherent
> or contiguous allocations.
> 
Do you have any other comments for the patch ? 
Current driver break remoteproc on NXP i.MX8 platform , the patch is bugfix the virtio rpmsg bus, we hope the patch enter to next and stable tree if no other comments.