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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Date:   Sun, 23 Aug 2020 10:01:59 +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 2:32 AM Hillf Danton <hdanton@...a.com> wrote:
>
>
> On Sat, 22 Aug 2020 19:10:07 +0300 Oded Gabbay wrote:
> > On Mon, Aug 17, 2020 at 4:48 PM Hillf Danton <hdanton@...a.com> wrote:
> > >
> > >
> > > Add dma_mmap_coherent() to match dma_alloc_coherent(), see the Link tag
> > > for why.
> > >
> > > Link: https://lore.kernel.org/lkml/20200609091727.GA23814@lst.de/
> > > 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/goya/goya.c
> > > +++ b/drivers/misc/habanalabs/goya/goya.c
> > > @@ -2642,15 +2642,14 @@ 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);
> > >
> > > --- 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;
> > >
> >
> > Hi Hillf,
> > Could you please also fix gaudi_cb_mmap() and resend this patch ?
>
> ---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/goya/goya.c
> +++ b/drivers/misc/habanalabs/goya/goya.c
> @@ -2642,15 +2642,14 @@ 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);
>
> --- 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/gaudi/gaudi.c
> +++ b/drivers/misc/habanalabs/gaudi/gaudi.c
> @@ -3063,15 +3063,14 @@ 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);
>
> --
>

Thanks!
One more small thing, can you please change the error message content
from "remap_pfn_range error" to "dma_mmap_coherent error" ?
Oded

Powered by blists - more mailing lists