| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAFCwf126app346tE0pCotHoAPTufpUr0xMgkfu2OC2sjhPUuzA@mail.gmail.com>
Date: Sun, 23 Aug 2020 14:01:23 +0300
From: Oded Gabbay <oded.gabbay@...il.com>
To: Hillf Danton <hdanton@...a.com>
Cc: Christoph Hellwig <hch@....de>,
LKML <linux-kernel@...r.kernel.org>,
SW_Drivers <SW_Drivers@...ana.ai>,
Zhang Li <li.zhang@...main.com>,
Ding Z Nan <oshack@...mail.com>
Subject: Re: [RFC PATCH] habanalabs: goya: make use of dma_mmap_coherent
On Sun, Aug 23, 2020 at 11:19 AM Hillf Danton <hdanton@...a.com> wrote:
>
>
> On Sun, 23 Aug 2020 10:01:59 +0300 Oded Gabbay wrote:
> >
> > One more small thing, can you please change the error message content
> > from "remap_pfn_range error" to "dma_mmap_coherent error" ?
>
> My fault.
>
> ---8<---
> From: Hillf Danton <hdanton@...a.com>
> Subject: [PATCH] habanalabs: make use of dma_mmap_coherent
>
> Add dma_mmap_coherent() for goya and gaudi to match their use of
> dma_alloc_coherent(), see the Link tag for why.
>
> Link: https://lore.kernel.org/lkml/20200609091727.GA23814@lst.de/
> Cc: Christoph Hellwig <hch@....de>
> Cc: Zhang Li <li.zhang@...main.com>
> Cc: Ding Z Nan <oshack@...mail.com>
> Signed-off-by: Hillf Danton <hdanton@...a.com>
> ---
>
> --- a/drivers/misc/habanalabs/common/habanalabs.h
> +++ b/drivers/misc/habanalabs/common/habanalabs.h
> @@ -679,7 +679,7 @@ struct hl_asic_funcs {
> int (*suspend)(struct hl_device *hdev);
> int (*resume)(struct hl_device *hdev);
> int (*cb_mmap)(struct hl_device *hdev, struct vm_area_struct *vma,
> - u64 kaddress, phys_addr_t paddress, u32 size);
> + void *cpu_addr, dma_addr_t dma_addr, size_t size);
> void (*ring_doorbell)(struct hl_device *hdev, u32 hw_queue_id, u32 pi);
> void (*pqe_write)(struct hl_device *hdev, __le64 *pqe,
> struct hl_bd *bd);
> --- a/drivers/misc/habanalabs/common/command_buffer.c
> +++ b/drivers/misc/habanalabs/common/command_buffer.c
> @@ -299,7 +299,6 @@ int hl_cb_mmap(struct hl_fpriv *hpriv, s
> {
> struct hl_device *hdev = hpriv->hdev;
> struct hl_cb *cb;
> - phys_addr_t address;
> u32 handle;
> int rc;
>
> @@ -344,12 +343,8 @@ int hl_cb_mmap(struct hl_fpriv *hpriv, s
>
> vma->vm_private_data = cb;
>
> - /* Calculate address for CB */
> - address = virt_to_phys((void *) (uintptr_t) cb->kernel_address);
> -
> - rc = hdev->asic_funcs->cb_mmap(hdev, vma, cb->kernel_address,
> - address, cb->size);
> -
> + rc = hdev->asic_funcs->cb_mmap(hdev, vma, (void *) cb->kernel_address,
> + cb->bus_address, cb->size);
> if (rc) {
> spin_lock(&cb->lock);
> cb->mmap = false;
> --- a/drivers/misc/habanalabs/goya/goya.c
> +++ b/drivers/misc/habanalabs/goya/goya.c
> @@ -2642,17 +2642,16 @@ int goya_resume(struct hl_device *hdev)
> }
>
> static int goya_cb_mmap(struct hl_device *hdev, struct vm_area_struct *vma,
> - u64 kaddress, phys_addr_t paddress, u32 size)
> + void *cpu_addr, dma_addr_t dma_addr, size_t size)
> {
> int rc;
>
> vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP |
> VM_DONTCOPY | VM_NORESERVE;
>
> - rc = remap_pfn_range(vma, vma->vm_start, paddress >> PAGE_SHIFT,
> - size, vma->vm_page_prot);
> + rc = dma_mmap_coherent(hdev->dev, vma, cpu_addr, dma_addr, size);
> if (rc)
> - dev_err(hdev->dev, "remap_pfn_range error %d", rc);
> + dev_err(hdev->dev, "dma_mmap_coherent error %d", rc);
>
> return rc;
> }
> --- a/drivers/misc/habanalabs/gaudi/gaudi.c
> +++ b/drivers/misc/habanalabs/gaudi/gaudi.c
> @@ -3063,17 +3063,16 @@ static int gaudi_resume(struct hl_device
> }
>
> static int gaudi_cb_mmap(struct hl_device *hdev, struct vm_area_struct *vma,
> - u64 kaddress, phys_addr_t paddress, u32 size)
> + void *cpu_addr, dma_addr_t dma_addr, size_t size)
> {
> int rc;
>
> vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP |
> VM_DONTCOPY | VM_NORESERVE;
>
> - rc = remap_pfn_range(vma, vma->vm_start, paddress >> PAGE_SHIFT,
> - size, vma->vm_page_prot);
> + rc = dma_mmap_coherent(hdev->dev, vma, cpu_addr, dma_addr, size);
> if (rc)
> - dev_err(hdev->dev, "remap_pfn_range error %d", rc);
> + dev_err(hdev->dev, "dma_mmap_coherent error %d", rc);
>
> return rc;
> }
> --
>
Hi Hillf,
I took your patch and ran a very basic test with it and unfortunately it breaks.
There are two bugs which I fixed:
1. Need to pass "&hdev->pdev->dev" instead of "hdev->dev" to
dma_mmap_coherent because we use the PCI dev, not our driver's device.
2. Need to pass "dma_addr - HOST_PHYS_BASE" instead of "dma_addr" to
dma_mmap_coherent because goya/gaudi alloc function adds
HOST_PHYS_BASE
But even after those fixes, I still get "-ENXIO" as a return value each time.
I added prints in the driver to make sure I called dma_mmap_coherent
with the exact same parameters as dma_alloc_coherent was called and I
saw they were the same.
e.g.
habanalabs hl0: alloc CB:
habanalabs hl0: kernel address == ffff9e6124691000
habanalabs hl0: bus address == 0xa24691000
habanalabs hl0: size == 0x1000
habanalabs hl0: mapping CB:
habanalabs hl0: user address == 0x7f674c4d9000
habanalabs hl0: kernel address == ffff9e6124691000
habanalabs hl0: bus address == 0xa24691000
habanalabs hl0: vm_flags == 0x42644fb
habanalabs hl0: size == 0x1000
habanalabs hl0: vm_page_prot == 0x8000000000000027
Bottom line, I can't take this patch until we find what's wrong.
Oded
Powered by blists - more mailing lists