On Wed, Sep 29, 2021 at 12:17:35AM +0300, Oded Gabbay wrote:

On Tue, Sep 28, 2021 at 8:36 PM Jason Gunthorpe jgg@ziepe.ca wrote:

...

On Sun, Sep 12, 2021 at 07:53:09PM +0300, Oded Gabbay wrote:

...

From: Tomer Tayar ttayar@habana.ai

Implement the calls to the dma-buf kernel api to create a dma-buf object backed by FD.

We block the option to mmap the DMA-BUF object because we don't support DIRECT_IO and implicit P2P.

This statement doesn't make sense, you can mmap your dmabuf if you like. All dmabuf mmaps are supposed to set the special bit/etc to exclude them from get_user_pages() anyhow - and since this is BAR memory not struct page memory this driver would be doing it anyhow.

But we block mmap the dmabuf fd from user-space. If you try to do it, you will get MAP_FAILED.

You do, I'm saying the above paragraph explaining *why* that was done is not correct.

...

...

We check the p2p distance using pci_p2pdma_distance_many() and refusing to map dmabuf in case the distance doesn't allow p2p.

Does this actually allow the p2p transfer for your intended use cases?

It depends on the system. If we are working bare-metal, then yes, it allows. If inside a VM, then no. The virtualized root complex is not white-listed and the kernel can't know the distance. But I remember you asked me to add this check, in v3 of the review IIRC. I don't mind removing this check if you don't object.

Yes, i tis the right code, I was curious how far along things have gotten

...

Don't write to the kernel log from user space triggered actions

at all ?

At all.

It's the first time I hear about this limitation...

Oh? It is a security issue, we don't want to allow userspace to DOS the kerne logging.

How do you tell the user it has done something wrong ?

dev_dbg is the usual way and then users doing debugging can opt in to the logging.

...

Why doesn't this return a sg_table * and an ERR_PTR?

Basically I modeled this function after amdgpu_vram_mgr_alloc_sgt() And in that function they also return int and pass the sg_table as **

If it's critical I can change.

Please follow the normal kernel style

Jason