Hi Loic, From: Loic PALLARDY <loic.pallardy@xxxxxx> Sent: 2019年1月10日 21:06 > Hi Andy, > > > -----Original Message----- > > From: Andy Duan <fugang.duan@xxxxxxx> > > Sent: jeudi 10 janvier 2019 02:45 > > To: Christoph Hellwig <hch@xxxxxxxxxxxxx>; bjorn.andersson@xxxxxxxxxx; > > ohad@xxxxxxxxxx > > Cc: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>; Andrew Morton > > <akpm@xxxxxxxxxxxxxxxxxxxx>; Linux-MM <linux-mm@xxxxxxxxx>; Robin > > Murphy <robin.murphy@xxxxxxx>; linux-remoteproc@xxxxxxxxxxxxxxx; > > anup@xxxxxxxxxxxxxx; Loic PALLARDY <loic.pallardy@xxxxxx>; > > dl-linux-imx <linux-imx@xxxxxxx>; Richard Zhu <hongxing.zhu@xxxxxxx>; > > Jason Liu <jason.hui.liu@xxxxxxx>; Peng Fan <peng.fan@xxxxxxx> > > Subject: RE: [rpmsg PATCH v2 1/1] rpmsg: virtio_rpmsg_bus: fix > > unexpected huge vmap mappings > > > > From: Andy Duan Sent: 2018年12月28日 9:48 > > > From: Christoph Hellwig <hch@xxxxxxxxxxxxx> 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 ? > > I tried your patch on ST platform and it doesn't compile neither on kernel > v5.0-rc1 nor on Bjorn's rpmsg-next. > dma_to_phys() is unknown as dma-direct.h not included and ‘struct > virtproc_info’ has no member named ‘bufs_dev’. > > Could you please send a new version fixing compilation issue. I would like to > test it on my platform to provide you feedback. > phys_to_dma, dma_to_phys and dma_capable are helpers published by architecture code for use of swiotlb and xen-swiotlb only. Drivers are not supposed to use these directly, but use the DMA API instead. So the fix is not acceptable due to arch limitation. As Christoph's said, It needs to fix the root cause. Andy
