Re: [PATCH RFC PKS/PMEM 09/58] drivers/gpu: Utilize new kmap_thread()
On Sat, Oct 10, 2020 at 12:03:49AM +0200, Daniel Vetter wrote: > On Fri, Oct 09, 2020 at 12:49:44PM -0700, ira.we...@intel.com wrote: > > From: Ira Weiny > > > > These kmap() calls in the gpu stack are localized to a single thread. > > To avoid the over head of global PKRS updates use the new kmap_thread() > > call. > > > > Cc: David Airlie > > Cc: Daniel Vetter > > Cc: Patrik Jakobsson > > Signed-off-by: Ira Weiny > > I'm guessing the entire pile goes in through some other tree. > Apologies for not realizing there were multiple maintainers here. But, I was thinking it would land together through the mm tree once the core support lands. I've tried to split these out in a way they can be easily reviewed/acked by the correct developers. > If so: > > Acked-by: Daniel Vetter > > If you want this to land through maintainer trees, then we need a > per-driver split (since aside from amdgpu and radeon they're all different > subtrees). It is just RFC for the moment. I need to get the core support accepted first then this can land. > > btw the two kmap calls in drm you highlight in the cover letter should > also be convertible to kmap_thread. We only hold vmalloc mappings for a > longer time (or it'd be quite a driver bug). So if you want maybe throw > those two as two additional patches on top, and we can do some careful > review & testing for them. Cool. I'll add them in. Ira > -Daniel > > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 12 ++-- > > drivers/gpu/drm/gma500/gma_display.c | 4 ++-- > > drivers/gpu/drm/gma500/mmu.c | 10 +- > > drivers/gpu/drm/i915/gem/i915_gem_shmem.c| 4 ++-- > > .../gpu/drm/i915/gem/selftests/i915_gem_context.c| 4 ++-- > > drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c | 8 > > drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 4 ++-- > > drivers/gpu/drm/i915/gt/intel_gtt.c | 4 ++-- > > drivers/gpu/drm/i915/gt/shmem_utils.c| 4 ++-- > > drivers/gpu/drm/i915/i915_gem.c | 8 > > drivers/gpu/drm/i915/i915_gpu_error.c| 4 ++-- > > drivers/gpu/drm/i915/selftests/i915_perf.c | 4 ++-- > > drivers/gpu/drm/radeon/radeon_ttm.c | 4 ++-- > > 13 files changed, 37 insertions(+), 37 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > > index 978bae731398..bd564bccb7a3 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > > @@ -2437,11 +2437,11 @@ static ssize_t amdgpu_ttm_gtt_read(struct file *f, > > char __user *buf, > > > > page = adev->gart.pages[p]; > > if (page) { > > - ptr = kmap(page); > > + ptr = kmap_thread(page); > > ptr += off; > > > > r = copy_to_user(buf, ptr, cur_size); > > - kunmap(adev->gart.pages[p]); > > + kunmap_thread(adev->gart.pages[p]); > > } else > > r = clear_user(buf, cur_size); > > > > @@ -2507,9 +2507,9 @@ static ssize_t amdgpu_iomem_read(struct file *f, char > > __user *buf, > > if (p->mapping != adev->mman.bdev.dev_mapping) > > return -EPERM; > > > > - ptr = kmap(p); > > + ptr = kmap_thread(p); > > r = copy_to_user(buf, ptr + off, bytes); > > - kunmap(p); > > + kunmap_thread(p); > > if (r) > > return -EFAULT; > > > > @@ -2558,9 +2558,9 @@ static ssize_t amdgpu_iomem_write(struct file *f, > > const char __user *buf, > > if (p->mapping != adev->mman.bdev.dev_mapping) > > return -EPERM; > > > > - ptr = kmap(p); > > + ptr = kmap_thread(p); > > r = copy_from_user(ptr + off, buf, bytes); > > - kunmap(p); > > + kunmap_thread(p); > > if (r) > > return -EFAULT; > > > > diff --git a/drivers/gpu/drm/gma500/gma_display.c > > b/drivers/gpu/drm/gma500/gma_display.c > > index 3df6d6e850f5..35f4e55c941f 100644 > > --- a/drivers/gpu/drm/gma500/gma_display.c > > +++ b/drivers/gpu/drm/gma500/gma_display.c > > @@ -400,9 +400,9 @@ int gma_crtc_cursor_set(struct drm_crtc *crtc, > > /* Copy the cursor to cursor mem */ > > tmp_dst = dev_priv->vram_addr + cursor_gt->offset; > > for (i = 0; i < cursor_pages; i++) { > > - tmp_src = kmap(gt->pages[i]); > > + tmp_src = kmap_thread(gt->pages[i]); > > memcpy(tmp_dst, tmp_src, PAGE_SIZE); > > - kunmap(gt->pages[i]); > > + kunmap_thread(gt->pages[i]); > > tmp_dst += PAGE_SIZE; > > } > > > > diff --git
Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn
On Sat, Oct 10, 2020 at 11:36 PM Laurent Pinchart wrote: > > Hi Tomasz, > > On Sat, Oct 10, 2020 at 07:22:48PM +0200, Tomasz Figa wrote: > > On Fri, Oct 9, 2020 at 7:52 PM Daniel Vetter wrote: > > > On Fri, Oct 9, 2020 at 2:48 PM Jason Gunthorpe wrote: > > > > On Fri, Oct 09, 2020 at 02:37:23PM +0200, Mauro Carvalho Chehab wrote: > > > > > > > > > I'm not a mm/ expert, but, from what I understood from Daniel's patch > > > > > description is that this is unsafe *only if* __GFP_MOVABLE is used. > > > > > > > > No, it is unconditionally unsafe. The CMA movable mappings are > > > > specific VMAs that will have bad issues here, but there are other > > > > types too. > > > > > > > > The only way to do something at a VMA level is to have a list of OK > > > > VMAs, eg because they were creatd via a special mmap helper from the > > > > media subsystem. > > > > > > > > > Well, no drivers inside the media subsystem uses such flag, although > > > > > they may rely on some infrastructure that could be using it behind > > > > > the bars. > > > > > > > > It doesn't matter, nothing prevents the user from calling media APIs > > > > on mmaps it gets from other subsystems. > > > > > > I think a good first step would be to disable userptr of non struct > > > page backed storage going forward for any new hw support. Even on > > > existing drivers. dma-buf sharing has been around for long enough now > > > that this shouldn't be a problem. Unfortunately right now this doesn't > > > seem to exist, so the entire problem keeps getting perpetuated. > > > > > > > > If this is the case, the proper fix seems to have a GFP_NOT_MOVABLE > > > > > flag that it would be denying the core mm code to set __GFP_MOVABLE. > > > > > > > > We can't tell from the VMA these kinds of details.. > > > > > > > > It has to go the other direction, evey mmap that might be used as a > > > > userptr here has to be found and the VMA specially created to allow > > > > its use. At least that is a kernel only change, but will need people > > > > with the HW to do this work. > > > > > > I think the only reasonable way to keep this working is: > > > - add a struct dma_buf *vma_tryget_dma_buf(struct vm_area_struct *vma); > > > - add dma-buf export support to fbdev and v4l > > > > I assume you mean V4L2 and not the obsolete V4L that is emulated in > > the userspace by libv4l. If so, every video device that uses videobuf2 > > gets DMA-buf export for free and there is nothing needed to enable it. Yeah. And I missed that v4l2 added dma-buf export too. > > We probably still have a few legacy drivers using videobuf (non-2), > > but IMHO those should be safe to put behind some disabled-by-default > > Kconfig symbol or even completely drop, as the legacy framework has > > been deprecated for many years already. > > There's 8 drivers left, and they support a very large number of devices. > I expect unhappy users distros stop shipping them. On the other hand, > videobuf has been deprecated for a lng time, so there has been > plenty of time to convert the remaining drivers to videobuf2. If nobody > can do it, then we'll have to drop support for these devices given the > security issues. Again, the issue here is _only_ with follow_pfn. For videobuf1 this means videbuf-dma-contig.c userptr support is broken. Unlike videobuf2 it means it's broken for all usage (not just zero-copy userptr), because videbuf-dma-contig.c lacks the pin_user_pages path. But that would be easy to add if this poses a problem I think - we just need to carry over the pin_user_pages_fast logic from videbuf2, no driver changes required. But of course I don't think we should do that before someone reports the regression, since videobuf1 userptr is doubly deprecated :-) Everything else keeps working with videobuf1 with my patch series. So depending upon which videobuf1 implementations these 8 drivers use, you might not even have any real breakage there. > We have moved media drivers to staging in the past when there wasn't > enough maintenance effort, we could do so here too. I'm not breaking the world with this, it's really very minimal use-case. At least as far as I'm understanding the entire media subsystem here. -Daniel > > > - roll this out everywhere we still need it. > > > > > > Realistically this just isn't going to happen. And anything else just > > > reimplements half of dma-buf, which is kinda pointless (you need > > > minimally refcounting and some way to get at a promise of a permanent > > > sg list for dma. Plus probably the vmap for kernel cpu access. > > > > > > > > Please let address the issue on this way, instead of broken an > > > > > userspace API that it is there since 1991. > > > > > > > > It has happened before :( It took 4 years for RDMA to undo the uAPI > > > > breakage caused by a security fix for something that was a 15 years > > > > old bug. > > > > > > Yeah we have a bunch of these on the drm side too. Some of them are > > > really just "you have to upgrade
Re: [PATCH v2 03/17] misc/habana: Stop using frame_vector helpers
On Sun, Oct 11, 2020 at 12:41 AM Daniel Vetter wrote: > > On Sat, Oct 10, 2020 at 11:32 PM Daniel Vetter wrote: > > > > On Sat, Oct 10, 2020 at 10:27 PM Oded Gabbay wrote: > > > > > > On Fri, Oct 9, 2020 at 10:59 AM Daniel Vetter > > > wrote: > > > > > > > > All we need are a pages array, pin_user_pages_fast can give us that > > > > directly. Plus this avoids the entire raw pfn side of get_vaddr_frames. > > > > > > > Thanks for the patch Daniel. > > > > > > > Signed-off-by: Daniel Vetter > > > > Cc: Jason Gunthorpe > > > > Cc: Andrew Morton > > > > Cc: John Hubbard > > > > Cc: Jérôme Glisse > > > > Cc: Jan Kara > > > > Cc: Dan Williams > > > > Cc: linux...@kvack.org > > > > Cc: linux-arm-ker...@lists.infradead.org > > > > Cc: linux-samsung-...@vger.kernel.org > > > > Cc: linux-me...@vger.kernel.org > > > > Cc: Oded Gabbay > > > > Cc: Omer Shpigelman > > > > Cc: Ofir Bitton > > > > Cc: Tomer Tayar > > > > Cc: Moti Haimovski > > > > Cc: Daniel Vetter > > > > Cc: Greg Kroah-Hartman > > > > Cc: Pawel Piskorski > > > > -- > > > > v2: Use unpin_user_pages_dirty_lock (John) > > > > --- > > > > drivers/misc/habanalabs/Kconfig | 1 - > > > > drivers/misc/habanalabs/common/habanalabs.h | 3 +- > > > > drivers/misc/habanalabs/common/memory.c | 49 - > > > > 3 files changed, 20 insertions(+), 33 deletions(-) > > > > > > > > diff --git a/drivers/misc/habanalabs/Kconfig > > > > b/drivers/misc/habanalabs/Kconfig > > > > index 8eb5d38c618e..2f04187f7167 100644 > > > > --- a/drivers/misc/habanalabs/Kconfig > > > > +++ b/drivers/misc/habanalabs/Kconfig > > > > @@ -6,7 +6,6 @@ > > > > config HABANA_AI > > > > tristate "HabanaAI accelerators (habanalabs)" > > > > depends on PCI && HAS_IOMEM > > > > - select FRAME_VECTOR > > > > select DMA_SHARED_BUFFER > > > > select GENERIC_ALLOCATOR > > > > select HWMON > > > > diff --git a/drivers/misc/habanalabs/common/habanalabs.h > > > > b/drivers/misc/habanalabs/common/habanalabs.h > > > > index edbd627b29d2..c1b3ad613b15 100644 > > > > --- a/drivers/misc/habanalabs/common/habanalabs.h > > > > +++ b/drivers/misc/habanalabs/common/habanalabs.h > > > > @@ -881,7 +881,8 @@ struct hl_ctx_mgr { > > > > struct hl_userptr { > > > > enum vm_type_t vm_type; /* must be first */ > > > > struct list_headjob_node; > > > > - struct frame_vector *vec; > > > > + struct page **pages; > > > > + unsigned intnpages; > > > Can you please update the kerneldoc comment section of this structure > > > according to your changes ? > > > > Apologies I missed the nice kerneldoc. I'll fix that in the next round. > > > > > > > > struct sg_table *sgt; > > > > enum dma_data_direction dir; > > > > struct list_headdebugfs_list; > > > > diff --git a/drivers/misc/habanalabs/common/memory.c > > > > b/drivers/misc/habanalabs/common/memory.c > > > > index 5ff4688683fd..327b64479f97 100644 > > > > --- a/drivers/misc/habanalabs/common/memory.c > > > > +++ b/drivers/misc/habanalabs/common/memory.c > > > > @@ -1281,45 +1281,41 @@ static int get_user_memory(struct hl_device > > > > *hdev, u64 addr, u64 size, > > > > return -EFAULT; > > > > } > > > > > > > > - userptr->vec = frame_vector_create(npages); > > > > - if (!userptr->vec) { > > > > + userptr->pages = kvmalloc_array(npages, sizeof(*userptr->pages), > > > > + GFP_KERNEL); > > > > + if (!userptr->pages) { > > > > dev_err(hdev->dev, "Failed to create frame vector\n"); > > > > return -ENOMEM; > > > > } > > > > > > > > - rc = get_vaddr_frames(start, npages, FOLL_FORCE | FOLL_WRITE, > > > > - userptr->vec); > > > > + rc = pin_user_pages_fast(start, npages, FOLL_FORCE | FOLL_WRITE, > > > > +userptr->pages); > > > > > > > > if (rc != npages) { > > > > dev_err(hdev->dev, > > > > "Failed to map host memory, user ptr probably > > > > wrong\n"); > > > > if (rc < 0) > > > > - goto destroy_framevec; > > > > + goto destroy_pages; > > > > + npages = rc; > > > > rc = -EFAULT; > > > > - goto put_framevec; > > > > - } > > > > - > > > > - if (frame_vector_to_pages(userptr->vec) < 0) { > > > > - dev_err(hdev->dev, > > > > - "Failed to translate frame vector to pages\n"); > > > > - rc = -EFAULT; > > > > - goto put_framevec; > > > > + goto put_pages; > > > > } > > > > + userptr->npages = npages; > > > > > > > > rc = sg_alloc_table_from_pages(userptr->sgt, > > > > - > > > >
Re: [PATCH v2 03/17] misc/habana: Stop using frame_vector helpers
On Sat, Oct 10, 2020 at 11:32 PM Daniel Vetter wrote: > > On Sat, Oct 10, 2020 at 10:27 PM Oded Gabbay wrote: > > > > On Fri, Oct 9, 2020 at 10:59 AM Daniel Vetter > > wrote: > > > > > > All we need are a pages array, pin_user_pages_fast can give us that > > > directly. Plus this avoids the entire raw pfn side of get_vaddr_frames. > > > > > Thanks for the patch Daniel. > > > > > Signed-off-by: Daniel Vetter > > > Cc: Jason Gunthorpe > > > Cc: Andrew Morton > > > Cc: John Hubbard > > > Cc: Jérôme Glisse > > > Cc: Jan Kara > > > Cc: Dan Williams > > > Cc: linux...@kvack.org > > > Cc: linux-arm-ker...@lists.infradead.org > > > Cc: linux-samsung-...@vger.kernel.org > > > Cc: linux-me...@vger.kernel.org > > > Cc: Oded Gabbay > > > Cc: Omer Shpigelman > > > Cc: Ofir Bitton > > > Cc: Tomer Tayar > > > Cc: Moti Haimovski > > > Cc: Daniel Vetter > > > Cc: Greg Kroah-Hartman > > > Cc: Pawel Piskorski > > > -- > > > v2: Use unpin_user_pages_dirty_lock (John) > > > --- > > > drivers/misc/habanalabs/Kconfig | 1 - > > > drivers/misc/habanalabs/common/habanalabs.h | 3 +- > > > drivers/misc/habanalabs/common/memory.c | 49 - > > > 3 files changed, 20 insertions(+), 33 deletions(-) > > > > > > diff --git a/drivers/misc/habanalabs/Kconfig > > > b/drivers/misc/habanalabs/Kconfig > > > index 8eb5d38c618e..2f04187f7167 100644 > > > --- a/drivers/misc/habanalabs/Kconfig > > > +++ b/drivers/misc/habanalabs/Kconfig > > > @@ -6,7 +6,6 @@ > > > config HABANA_AI > > > tristate "HabanaAI accelerators (habanalabs)" > > > depends on PCI && HAS_IOMEM > > > - select FRAME_VECTOR > > > select DMA_SHARED_BUFFER > > > select GENERIC_ALLOCATOR > > > select HWMON > > > diff --git a/drivers/misc/habanalabs/common/habanalabs.h > > > b/drivers/misc/habanalabs/common/habanalabs.h > > > index edbd627b29d2..c1b3ad613b15 100644 > > > --- a/drivers/misc/habanalabs/common/habanalabs.h > > > +++ b/drivers/misc/habanalabs/common/habanalabs.h > > > @@ -881,7 +881,8 @@ struct hl_ctx_mgr { > > > struct hl_userptr { > > > enum vm_type_t vm_type; /* must be first */ > > > struct list_headjob_node; > > > - struct frame_vector *vec; > > > + struct page **pages; > > > + unsigned intnpages; > > Can you please update the kerneldoc comment section of this structure > > according to your changes ? > > Apologies I missed the nice kerneldoc. I'll fix that in the next round. > > > > > struct sg_table *sgt; > > > enum dma_data_direction dir; > > > struct list_headdebugfs_list; > > > diff --git a/drivers/misc/habanalabs/common/memory.c > > > b/drivers/misc/habanalabs/common/memory.c > > > index 5ff4688683fd..327b64479f97 100644 > > > --- a/drivers/misc/habanalabs/common/memory.c > > > +++ b/drivers/misc/habanalabs/common/memory.c > > > @@ -1281,45 +1281,41 @@ static int get_user_memory(struct hl_device > > > *hdev, u64 addr, u64 size, > > > return -EFAULT; > > > } > > > > > > - userptr->vec = frame_vector_create(npages); > > > - if (!userptr->vec) { > > > + userptr->pages = kvmalloc_array(npages, sizeof(*userptr->pages), > > > + GFP_KERNEL); > > > + if (!userptr->pages) { > > > dev_err(hdev->dev, "Failed to create frame vector\n"); > > > return -ENOMEM; > > > } > > > > > > - rc = get_vaddr_frames(start, npages, FOLL_FORCE | FOLL_WRITE, > > > - userptr->vec); > > > + rc = pin_user_pages_fast(start, npages, FOLL_FORCE | FOLL_WRITE, > > > +userptr->pages); > > > > > > if (rc != npages) { > > > dev_err(hdev->dev, > > > "Failed to map host memory, user ptr probably > > > wrong\n"); > > > if (rc < 0) > > > - goto destroy_framevec; > > > + goto destroy_pages; > > > + npages = rc; > > > rc = -EFAULT; > > > - goto put_framevec; > > > - } > > > - > > > - if (frame_vector_to_pages(userptr->vec) < 0) { > > > - dev_err(hdev->dev, > > > - "Failed to translate frame vector to pages\n"); > > > - rc = -EFAULT; > > > - goto put_framevec; > > > + goto put_pages; > > > } > > > + userptr->npages = npages; > > > > > > rc = sg_alloc_table_from_pages(userptr->sgt, > > > - frame_vector_pages(userptr->vec), > > > - npages, offset, size, GFP_ATOMIC); > > > + userptr->pages, > > > + npages, offset, size, GFP_ATOMIC); > > I think that because the call to
Re: [PATCH] ARM: davinci_all_defconfig: Add CONFIG_DRM_DISPLAY_CONNECTOR=m
Hi Jyri, Thank you for the patch. On Sat, Oct 10, 2020 at 07:08:50PM +0300, Jyri Sarha wrote: > Current dumb-vga-dac driver requires CONFIG_DRM_DISPLAY_CONNECTOR > for it to work. > > Signed-off-by: Jyri Sarha Reviewed-by: Laurent Pinchart > --- > An alternative would be selecting CONFIG_DRM_DISPLAY_CONNECTOR from all > bridges requiring DRM connector. > > arch/arm/configs/davinci_all_defconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/configs/davinci_all_defconfig > b/arch/arm/configs/davinci_all_defconfig > index e849367c0566..28471757c129 100644 > --- a/arch/arm/configs/davinci_all_defconfig > +++ b/arch/arm/configs/davinci_all_defconfig > @@ -160,6 +160,7 @@ CONFIG_DRM=m > CONFIG_DRM_TILCDC=m > CONFIG_DRM_SIMPLE_BRIDGE=m > CONFIG_DRM_TINYDRM=m > +CONFIG_DRM_DISPLAY_CONNECTOR=m > CONFIG_TINYDRM_ST7586=m > CONFIG_FB=y > CONFIG_FIRMWARE_EDID=y -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn
Hi Tomasz, On Sat, Oct 10, 2020 at 07:22:48PM +0200, Tomasz Figa wrote: > On Fri, Oct 9, 2020 at 7:52 PM Daniel Vetter wrote: > > On Fri, Oct 9, 2020 at 2:48 PM Jason Gunthorpe wrote: > > > On Fri, Oct 09, 2020 at 02:37:23PM +0200, Mauro Carvalho Chehab wrote: > > > > > > > I'm not a mm/ expert, but, from what I understood from Daniel's patch > > > > description is that this is unsafe *only if* __GFP_MOVABLE is used. > > > > > > No, it is unconditionally unsafe. The CMA movable mappings are > > > specific VMAs that will have bad issues here, but there are other > > > types too. > > > > > > The only way to do something at a VMA level is to have a list of OK > > > VMAs, eg because they were creatd via a special mmap helper from the > > > media subsystem. > > > > > > > Well, no drivers inside the media subsystem uses such flag, although > > > > they may rely on some infrastructure that could be using it behind > > > > the bars. > > > > > > It doesn't matter, nothing prevents the user from calling media APIs > > > on mmaps it gets from other subsystems. > > > > I think a good first step would be to disable userptr of non struct > > page backed storage going forward for any new hw support. Even on > > existing drivers. dma-buf sharing has been around for long enough now > > that this shouldn't be a problem. Unfortunately right now this doesn't > > seem to exist, so the entire problem keeps getting perpetuated. > > > > > > If this is the case, the proper fix seems to have a GFP_NOT_MOVABLE > > > > flag that it would be denying the core mm code to set __GFP_MOVABLE. > > > > > > We can't tell from the VMA these kinds of details.. > > > > > > It has to go the other direction, evey mmap that might be used as a > > > userptr here has to be found and the VMA specially created to allow > > > its use. At least that is a kernel only change, but will need people > > > with the HW to do this work. > > > > I think the only reasonable way to keep this working is: > > - add a struct dma_buf *vma_tryget_dma_buf(struct vm_area_struct *vma); > > - add dma-buf export support to fbdev and v4l > > I assume you mean V4L2 and not the obsolete V4L that is emulated in > the userspace by libv4l. If so, every video device that uses videobuf2 > gets DMA-buf export for free and there is nothing needed to enable it. > > We probably still have a few legacy drivers using videobuf (non-2), > but IMHO those should be safe to put behind some disabled-by-default > Kconfig symbol or even completely drop, as the legacy framework has > been deprecated for many years already. There's 8 drivers left, and they support a very large number of devices. I expect unhappy users distros stop shipping them. On the other hand, videobuf has been deprecated for a lng time, so there has been plenty of time to convert the remaining drivers to videobuf2. If nobody can do it, then we'll have to drop support for these devices given the security issues. We have moved media drivers to staging in the past when there wasn't enough maintenance effort, we could do so here too. > > - roll this out everywhere we still need it. > > > > Realistically this just isn't going to happen. And anything else just > > reimplements half of dma-buf, which is kinda pointless (you need > > minimally refcounting and some way to get at a promise of a permanent > > sg list for dma. Plus probably the vmap for kernel cpu access. > > > > > > Please let address the issue on this way, instead of broken an > > > > userspace API that it is there since 1991. > > > > > > It has happened before :( It took 4 years for RDMA to undo the uAPI > > > breakage caused by a security fix for something that was a 15 years > > > old bug. > > > > Yeah we have a bunch of these on the drm side too. Some of them are > > really just "you have to upgrade userspace", and there's no real fix > > for the security nightmare without that. > > I think we need to phase out such userspace indeed. The Kconfig symbol > allows enabling the unsafe functionality for anyone who still needs > it, so I think it's not entirely a breakage. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 03/17] misc/habana: Stop using frame_vector helpers
On Sat, Oct 10, 2020 at 10:27 PM Oded Gabbay wrote: > > On Fri, Oct 9, 2020 at 10:59 AM Daniel Vetter wrote: > > > > All we need are a pages array, pin_user_pages_fast can give us that > > directly. Plus this avoids the entire raw pfn side of get_vaddr_frames. > > > Thanks for the patch Daniel. > > > Signed-off-by: Daniel Vetter > > Cc: Jason Gunthorpe > > Cc: Andrew Morton > > Cc: John Hubbard > > Cc: Jérôme Glisse > > Cc: Jan Kara > > Cc: Dan Williams > > Cc: linux...@kvack.org > > Cc: linux-arm-ker...@lists.infradead.org > > Cc: linux-samsung-...@vger.kernel.org > > Cc: linux-me...@vger.kernel.org > > Cc: Oded Gabbay > > Cc: Omer Shpigelman > > Cc: Ofir Bitton > > Cc: Tomer Tayar > > Cc: Moti Haimovski > > Cc: Daniel Vetter > > Cc: Greg Kroah-Hartman > > Cc: Pawel Piskorski > > -- > > v2: Use unpin_user_pages_dirty_lock (John) > > --- > > drivers/misc/habanalabs/Kconfig | 1 - > > drivers/misc/habanalabs/common/habanalabs.h | 3 +- > > drivers/misc/habanalabs/common/memory.c | 49 - > > 3 files changed, 20 insertions(+), 33 deletions(-) > > > > diff --git a/drivers/misc/habanalabs/Kconfig > > b/drivers/misc/habanalabs/Kconfig > > index 8eb5d38c618e..2f04187f7167 100644 > > --- a/drivers/misc/habanalabs/Kconfig > > +++ b/drivers/misc/habanalabs/Kconfig > > @@ -6,7 +6,6 @@ > > config HABANA_AI > > tristate "HabanaAI accelerators (habanalabs)" > > depends on PCI && HAS_IOMEM > > - select FRAME_VECTOR > > select DMA_SHARED_BUFFER > > select GENERIC_ALLOCATOR > > select HWMON > > diff --git a/drivers/misc/habanalabs/common/habanalabs.h > > b/drivers/misc/habanalabs/common/habanalabs.h > > index edbd627b29d2..c1b3ad613b15 100644 > > --- a/drivers/misc/habanalabs/common/habanalabs.h > > +++ b/drivers/misc/habanalabs/common/habanalabs.h > > @@ -881,7 +881,8 @@ struct hl_ctx_mgr { > > struct hl_userptr { > > enum vm_type_t vm_type; /* must be first */ > > struct list_headjob_node; > > - struct frame_vector *vec; > > + struct page **pages; > > + unsigned intnpages; > Can you please update the kerneldoc comment section of this structure > according to your changes ? Apologies I missed the nice kerneldoc. I'll fix that in the next round. > > struct sg_table *sgt; > > enum dma_data_direction dir; > > struct list_headdebugfs_list; > > diff --git a/drivers/misc/habanalabs/common/memory.c > > b/drivers/misc/habanalabs/common/memory.c > > index 5ff4688683fd..327b64479f97 100644 > > --- a/drivers/misc/habanalabs/common/memory.c > > +++ b/drivers/misc/habanalabs/common/memory.c > > @@ -1281,45 +1281,41 @@ static int get_user_memory(struct hl_device *hdev, > > u64 addr, u64 size, > > return -EFAULT; > > } > > > > - userptr->vec = frame_vector_create(npages); > > - if (!userptr->vec) { > > + userptr->pages = kvmalloc_array(npages, sizeof(*userptr->pages), > > + GFP_KERNEL); > > + if (!userptr->pages) { > > dev_err(hdev->dev, "Failed to create frame vector\n"); > > return -ENOMEM; > > } > > > > - rc = get_vaddr_frames(start, npages, FOLL_FORCE | FOLL_WRITE, > > - userptr->vec); > > + rc = pin_user_pages_fast(start, npages, FOLL_FORCE | FOLL_WRITE, > > +userptr->pages); > > > > if (rc != npages) { > > dev_err(hdev->dev, > > "Failed to map host memory, user ptr probably > > wrong\n"); > > if (rc < 0) > > - goto destroy_framevec; > > + goto destroy_pages; > > + npages = rc; > > rc = -EFAULT; > > - goto put_framevec; > > - } > > - > > - if (frame_vector_to_pages(userptr->vec) < 0) { > > - dev_err(hdev->dev, > > - "Failed to translate frame vector to pages\n"); > > - rc = -EFAULT; > > - goto put_framevec; > > + goto put_pages; > > } > > + userptr->npages = npages; > > > > rc = sg_alloc_table_from_pages(userptr->sgt, > > - frame_vector_pages(userptr->vec), > > - npages, offset, size, GFP_ATOMIC); > > + userptr->pages, > > + npages, offset, size, GFP_ATOMIC); > I think that because the call to kvmalloc_array() is done with > GFP_KERNEL, there is no point in using GFP_ATOMIC here. > And actually, this path only needs to avoid yielding when using a > special debug mode. > So I suggest putting here GFP_KERNEL. Huh, I didn't even notice the GFP_ATOMIC here. This looks indeed strange and GFP_KERNEL should
Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn
Hi Daniel, On Fri, Oct 09, 2020 at 07:52:05PM +0200, Daniel Vetter wrote: > On Fri, Oct 9, 2020 at 2:48 PM Jason Gunthorpe wrote: > > On Fri, Oct 09, 2020 at 02:37:23PM +0200, Mauro Carvalho Chehab wrote: > > > > > I'm not a mm/ expert, but, from what I understood from Daniel's patch > > > description is that this is unsafe *only if* __GFP_MOVABLE is used. > > > > No, it is unconditionally unsafe. The CMA movable mappings are > > specific VMAs that will have bad issues here, but there are other > > types too. > > > > The only way to do something at a VMA level is to have a list of OK > > VMAs, eg because they were creatd via a special mmap helper from the > > media subsystem. > > > > > Well, no drivers inside the media subsystem uses such flag, although > > > they may rely on some infrastructure that could be using it behind > > > the bars. > > > > It doesn't matter, nothing prevents the user from calling media APIs > > on mmaps it gets from other subsystems. > > I think a good first step would be to disable userptr of non struct > page backed storage going forward for any new hw support. Even on > existing drivers. dma-buf sharing has been around for long enough now > that this shouldn't be a problem. Unfortunately right now this doesn't > seem to exist, so the entire problem keeps getting perpetuated. On the V4L2 side, I think we should disable USERPTR for any new driver, period. That's what I've been recommended when reviewing patches for several years already. It's a deprecated API, it should be phased out, which starts by not allowing any new use case. > > > If this is the case, the proper fix seems to have a GFP_NOT_MOVABLE > > > flag that it would be denying the core mm code to set __GFP_MOVABLE. > > > > We can't tell from the VMA these kinds of details.. > > > > It has to go the other direction, evey mmap that might be used as a > > userptr here has to be found and the VMA specially created to allow > > its use. At least that is a kernel only change, but will need people > > with the HW to do this work. > > I think the only reasonable way to keep this working is: > - add a struct dma_buf *vma_tryget_dma_buf(struct vm_area_struct *vma); > - add dma-buf export support to fbdev and v4l > - roll this out everywhere we still need it. > > Realistically this just isn't going to happen. And anything else just > reimplements half of dma-buf, which is kinda pointless (you need > minimally refcounting and some way to get at a promise of a permanent > sg list for dma. Plus probably the vmap for kernel cpu access. > > > > Please let address the issue on this way, instead of broken an > > > userspace API that it is there since 1991. > > > > It has happened before :( It took 4 years for RDMA to undo the uAPI > > breakage caused by a security fix for something that was a 15 years > > old bug. > > Yeah we have a bunch of these on the drm side too. Some of them are > really just "you have to upgrade userspace", and there's no real fix > for the security nightmare without that. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 0/2] drm/tilcdc: Couple of minor feature improvements
Since v2: - "drm/tilcdc: Remove tilcdc_crtc_max_width(), use private data" - Add reviewed-by tag - fix: -:106: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis #106: FILE: drivers/gpu/drm/tilcdc/tilcdc_drv.c:289: + if (of_property_read_u32(node, "max-pixelclock", + >max_pixelclock)) Since first version. - "drm/tilcdc: Do not keep vblank interrupts enabled all the time" - Add reviewed-by tags - "drm/tilcdc: Remove tilcdc_crtc_max_width(), use private data" - Put TILCDC_DEFAULT_MAX_WIDTH_V1 before *_V2 and The vblank interrupts have been always on when the display is on for a very long time, so I decided that it is about time to fix it. Then the following patch is just a cleanup. BR, Jyri Jyri Sarha (2): drm/tilcdc: Do not keep vblank interrupts enabled all the time drm/tilcdc: Remove tilcdc_crtc_max_width(), use private data drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 52 ++-- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 38 +++- drivers/gpu/drm/tilcdc/tilcdc_drv.h | 7 ++-- 3 files changed, 59 insertions(+), 38 deletions(-) -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 2/2] drm/tilcdc: Remove tilcdc_crtc_max_width(), use private data
We already have a private data member for maximum display width so let's use it and get rid of the redundant tilcdc_crtc_max_width(). The LCDC version probing is moved to before reading the device tree properties so that the version information is available when private data maximum width is initialized, if "max-width" property is not found. Signed-off-by: Jyri Sarha Reviewed-by: Tomi Valkeinen --- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 16 +--- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 38 +++- drivers/gpu/drm/tilcdc/tilcdc_drv.h | 7 ++--- 3 files changed, 26 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c index 0fd3dafe6404..da2ab2aa3577 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c @@ -754,20 +754,6 @@ static const struct drm_crtc_funcs tilcdc_crtc_funcs = { .disable_vblank = tilcdc_crtc_disable_vblank, }; -int tilcdc_crtc_max_width(struct drm_crtc *crtc) -{ - struct drm_device *dev = crtc->dev; - struct tilcdc_drm_private *priv = dev->dev_private; - int max_width = 0; - - if (priv->rev == 1) - max_width = 1024; - else if (priv->rev == 2) - max_width = 2048; - - return max_width; -} - static enum drm_mode_status tilcdc_crtc_mode_valid(struct drm_crtc *crtc, const struct drm_display_mode *mode) @@ -780,7 +766,7 @@ tilcdc_crtc_mode_valid(struct drm_crtc *crtc, * check to see if the width is within the range that * the LCD Controller physically supports */ - if (mode->hdisplay > tilcdc_crtc_max_width(crtc)) + if (mode->hdisplay > priv->max_width) return MODE_VIRTUAL_X; /* width must be multiple of 16 */ diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index 4f5fc3e87383..c5f82e693f1a 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -105,7 +105,7 @@ static void modeset_init(struct drm_device *dev) dev->mode_config.min_width = 0; dev->mode_config.min_height = 0; - dev->mode_config.max_width = tilcdc_crtc_max_width(priv->crtc); + dev->mode_config.max_width = priv->max_width; dev->mode_config.max_height = 2048; dev->mode_config.funcs = _config_funcs; } @@ -218,22 +218,6 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev) goto init_failed; } - if (of_property_read_u32(node, "max-bandwidth", >max_bandwidth)) - priv->max_bandwidth = TILCDC_DEFAULT_MAX_BANDWIDTH; - - DBG("Maximum Bandwidth Value %d", priv->max_bandwidth); - - if (of_property_read_u32(node, "max-width", >max_width)) - priv->max_width = TILCDC_DEFAULT_MAX_WIDTH; - - DBG("Maximum Horizontal Pixel Width Value %dpixels", priv->max_width); - - if (of_property_read_u32(node, "max-pixelclock", - >max_pixelclock)) - priv->max_pixelclock = TILCDC_DEFAULT_MAX_PIXELCLOCK; - - DBG("Maximum Pixel Clock Value %dKHz", priv->max_pixelclock); - pm_runtime_enable(dev); /* Determine LCD IP Version */ @@ -287,6 +271,26 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev) } } + if (of_property_read_u32(node, "max-bandwidth", >max_bandwidth)) + priv->max_bandwidth = TILCDC_DEFAULT_MAX_BANDWIDTH; + + DBG("Maximum Bandwidth Value %d", priv->max_bandwidth); + + if (of_property_read_u32(node, "max-width", >max_width)) { + if (priv->rev == 1) + priv->max_width = TILCDC_DEFAULT_MAX_WIDTH_V1; + else + priv->max_width = TILCDC_DEFAULT_MAX_WIDTH_V2; + } + + DBG("Maximum Horizontal Pixel Width Value %dpixels", priv->max_width); + + if (of_property_read_u32(node, "max-pixelclock", +>max_pixelclock)) + priv->max_pixelclock = TILCDC_DEFAULT_MAX_PIXELCLOCK; + + DBG("Maximum Pixel Clock Value %dKHz", priv->max_pixelclock); + ret = tilcdc_crtc_create(ddev); if (ret < 0) { dev_err(dev, "failed to create crtc\n"); diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h index 18815e75ca4f..d29806ca8817 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h @@ -28,8 +28,10 @@ struct drm_plane; /* Defaulting to pixel clock defined on AM335x */ #define TILCDC_DEFAULT_MAX_PIXELCLOCK 126000 -/* Defaulting to max width as defined on AM335x */ -#define TILCDC_DEFAULT_MAX_WIDTH 2048 +/* Maximum display width for LCDC V1 */ +#define TILCDC_DEFAULT_MAX_WIDTH_V1 1024 +/* ... and for LCDC V2 found on AM335x: */ +#define TILCDC_DEFAULT_MAX_WIDTH_V2 2048 /*
[PATCH v3 1/2] drm/tilcdc: Do not keep vblank interrupts enabled all the time
END_OF_FRAME interrupts have been enabled all the time since the beginning of this driver. It is about time to add this feature. Signed-off-by: Jyri Sarha Reviewed-by: Tomi Valkeinen Reviewed-by: Laurent Pinchart --- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 36 +--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c index cf4ead0dc2e3..0fd3dafe6404 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c @@ -147,12 +147,9 @@ static void tilcdc_crtc_enable_irqs(struct drm_device *dev) tilcdc_set(dev, LCDC_RASTER_CTRL_REG, LCDC_V1_SYNC_LOST_INT_ENA | LCDC_V1_FRAME_DONE_INT_ENA | LCDC_V1_UNDERFLOW_INT_ENA); - tilcdc_set(dev, LCDC_DMA_CTRL_REG, - LCDC_V1_END_OF_FRAME_INT_ENA); } else { tilcdc_write(dev, LCDC_INT_ENABLE_SET_REG, LCDC_V2_UNDERFLOW_INT_ENA | - LCDC_V2_END_OF_FRAME0_INT_ENA | LCDC_FRAME_DONE | LCDC_SYNC_LOST); } } @@ -678,11 +675,44 @@ static int tilcdc_crtc_atomic_check(struct drm_crtc *crtc, static int tilcdc_crtc_enable_vblank(struct drm_crtc *crtc) { + struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc); + struct drm_device *dev = crtc->dev; + struct tilcdc_drm_private *priv = dev->dev_private; + unsigned long flags; + + spin_lock_irqsave(_crtc->irq_lock, flags); + + tilcdc_clear_irqstatus(dev, LCDC_END_OF_FRAME0); + + if (priv->rev == 1) + tilcdc_set(dev, LCDC_DMA_CTRL_REG, + LCDC_V1_END_OF_FRAME_INT_ENA); + else + tilcdc_set(dev, LCDC_INT_ENABLE_SET_REG, + LCDC_V2_END_OF_FRAME0_INT_ENA); + + spin_unlock_irqrestore(_crtc->irq_lock, flags); + return 0; } static void tilcdc_crtc_disable_vblank(struct drm_crtc *crtc) { + struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc); + struct drm_device *dev = crtc->dev; + struct tilcdc_drm_private *priv = dev->dev_private; + unsigned long flags; + + spin_lock_irqsave(_crtc->irq_lock, flags); + + if (priv->rev == 1) + tilcdc_clear(dev, LCDC_DMA_CTRL_REG, +LCDC_V1_END_OF_FRAME_INT_ENA); + else + tilcdc_clear(dev, LCDC_INT_ENABLE_SET_REG, +LCDC_V2_END_OF_FRAME0_INT_ENA); + + spin_unlock_irqrestore(_crtc->irq_lock, flags); } static void tilcdc_crtc_reset(struct drm_crtc *crtc) -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 03/17] misc/habana: Stop using frame_vector helpers
On Fri, Oct 9, 2020 at 10:59 AM Daniel Vetter wrote: > > All we need are a pages array, pin_user_pages_fast can give us that > directly. Plus this avoids the entire raw pfn side of get_vaddr_frames. > Thanks for the patch Daniel. > Signed-off-by: Daniel Vetter > Cc: Jason Gunthorpe > Cc: Andrew Morton > Cc: John Hubbard > Cc: Jérôme Glisse > Cc: Jan Kara > Cc: Dan Williams > Cc: linux...@kvack.org > Cc: linux-arm-ker...@lists.infradead.org > Cc: linux-samsung-...@vger.kernel.org > Cc: linux-me...@vger.kernel.org > Cc: Oded Gabbay > Cc: Omer Shpigelman > Cc: Ofir Bitton > Cc: Tomer Tayar > Cc: Moti Haimovski > Cc: Daniel Vetter > Cc: Greg Kroah-Hartman > Cc: Pawel Piskorski > -- > v2: Use unpin_user_pages_dirty_lock (John) > --- > drivers/misc/habanalabs/Kconfig | 1 - > drivers/misc/habanalabs/common/habanalabs.h | 3 +- > drivers/misc/habanalabs/common/memory.c | 49 - > 3 files changed, 20 insertions(+), 33 deletions(-) > > diff --git a/drivers/misc/habanalabs/Kconfig b/drivers/misc/habanalabs/Kconfig > index 8eb5d38c618e..2f04187f7167 100644 > --- a/drivers/misc/habanalabs/Kconfig > +++ b/drivers/misc/habanalabs/Kconfig > @@ -6,7 +6,6 @@ > config HABANA_AI > tristate "HabanaAI accelerators (habanalabs)" > depends on PCI && HAS_IOMEM > - select FRAME_VECTOR > select DMA_SHARED_BUFFER > select GENERIC_ALLOCATOR > select HWMON > diff --git a/drivers/misc/habanalabs/common/habanalabs.h > b/drivers/misc/habanalabs/common/habanalabs.h > index edbd627b29d2..c1b3ad613b15 100644 > --- a/drivers/misc/habanalabs/common/habanalabs.h > +++ b/drivers/misc/habanalabs/common/habanalabs.h > @@ -881,7 +881,8 @@ struct hl_ctx_mgr { > struct hl_userptr { > enum vm_type_t vm_type; /* must be first */ > struct list_headjob_node; > - struct frame_vector *vec; > + struct page **pages; > + unsigned intnpages; Can you please update the kerneldoc comment section of this structure according to your changes ? > struct sg_table *sgt; > enum dma_data_direction dir; > struct list_headdebugfs_list; > diff --git a/drivers/misc/habanalabs/common/memory.c > b/drivers/misc/habanalabs/common/memory.c > index 5ff4688683fd..327b64479f97 100644 > --- a/drivers/misc/habanalabs/common/memory.c > +++ b/drivers/misc/habanalabs/common/memory.c > @@ -1281,45 +1281,41 @@ static int get_user_memory(struct hl_device *hdev, > u64 addr, u64 size, > return -EFAULT; > } > > - userptr->vec = frame_vector_create(npages); > - if (!userptr->vec) { > + userptr->pages = kvmalloc_array(npages, sizeof(*userptr->pages), > + GFP_KERNEL); > + if (!userptr->pages) { > dev_err(hdev->dev, "Failed to create frame vector\n"); > return -ENOMEM; > } > > - rc = get_vaddr_frames(start, npages, FOLL_FORCE | FOLL_WRITE, > - userptr->vec); > + rc = pin_user_pages_fast(start, npages, FOLL_FORCE | FOLL_WRITE, > +userptr->pages); > > if (rc != npages) { > dev_err(hdev->dev, > "Failed to map host memory, user ptr probably > wrong\n"); > if (rc < 0) > - goto destroy_framevec; > + goto destroy_pages; > + npages = rc; > rc = -EFAULT; > - goto put_framevec; > - } > - > - if (frame_vector_to_pages(userptr->vec) < 0) { > - dev_err(hdev->dev, > - "Failed to translate frame vector to pages\n"); > - rc = -EFAULT; > - goto put_framevec; > + goto put_pages; > } > + userptr->npages = npages; > > rc = sg_alloc_table_from_pages(userptr->sgt, > - frame_vector_pages(userptr->vec), > - npages, offset, size, GFP_ATOMIC); > + userptr->pages, > + npages, offset, size, GFP_ATOMIC); I think that because the call to kvmalloc_array() is done with GFP_KERNEL, there is no point in using GFP_ATOMIC here. And actually, this path only needs to avoid yielding when using a special debug mode. So I suggest putting here GFP_KERNEL. In the meanwhile, I'll run this patch (coupled with the next patch) in our C/I to make sure there are no regressions. Thanks, Oded > if (rc < 0) { > dev_err(hdev->dev, "failed to create SG table from pages\n"); > - goto put_framevec; > + goto put_pages; > } > > return 0; > > -put_framevec: > - put_vaddr_frames(userptr->vec); > -destroy_framevec: > -
[PATCH] ARM: davinci_all_defconfig: Add CONFIG_DRM_DISPLAY_CONNECTOR=m
Current dumb-vga-dac driver requires CONFIG_DRM_DISPLAY_CONNECTOR for it to work. Signed-off-by: Jyri Sarha --- An alternative would be selecting CONFIG_DRM_DISPLAY_CONNECTOR from all bridges requiring DRM connector. arch/arm/configs/davinci_all_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/configs/davinci_all_defconfig b/arch/arm/configs/davinci_all_defconfig index e849367c0566..28471757c129 100644 --- a/arch/arm/configs/davinci_all_defconfig +++ b/arch/arm/configs/davinci_all_defconfig @@ -160,6 +160,7 @@ CONFIG_DRM=m CONFIG_DRM_TILCDC=m CONFIG_DRM_SIMPLE_BRIDGE=m CONFIG_DRM_TINYDRM=m +CONFIG_DRM_DISPLAY_CONNECTOR=m CONFIG_TINYDRM_ST7586=m CONFIG_FB=y CONFIG_FIRMWARE_EDID=y -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn
Hi Mauro, On Fri, Oct 9, 2020 at 2:37 PM Mauro Carvalho Chehab wrote: > > Em Fri, 9 Oct 2020 09:21:11 -0300 > Jason Gunthorpe escreveu: > > > On Fri, Oct 09, 2020 at 12:34:21PM +0200, Mauro Carvalho Chehab wrote: > > > Hi, > > > > > > Em Fri, 9 Oct 2020 09:59:26 +0200 > > > Daniel Vetter escreveu: > > > > > > > Way back it was a reasonable assumptions that iomem mappings never > > > > change the pfn range they point at. But this has changed: > > > > > > > > - gpu drivers dynamically manage their memory nowadays, invalidating > > > > ptes with unmap_mapping_range when buffers get moved > > > > > > > > - contiguous dma allocations have moved from dedicated carvetouts to > > > > cma regions. This means if we miss the unmap the pfn might contain > > > > pagecache or anon memory (well anything allocated with GFP_MOVEABLE) > > > > > > > > - even /dev/mem now invalidates mappings when the kernel requests that > > > > iomem region when CONFIG_IO_STRICT_DEVMEM is set, see 3234ac664a87 > > > > ("/dev/mem: Revoke mappings when a driver claims the region") > > > > > > > > Accessing pfns obtained from ptes without holding all the locks is > > > > therefore no longer a good idea. > > > > > > > > Unfortunately there's some users where this is not fixable (like v4l > > > > userptr of iomem mappings) or involves a pile of work (vfio type1 > > > > iommu). For now annotate these as unsafe and splat appropriately. > > > > > > > > This patch adds an unsafe_follow_pfn, which later patches will then > > > > roll out to all appropriate places. > > > > > > NACK, as this breaks an existing userspace API on media. > > > > It doesn't break it. You get a big warning the thing is broken and it > > keeps working. > > > > We can't leave such a huge security hole open - it impacts other > > subsystems, distros need to be able to run in a secure mode. > > Well, if distros disable it, then apps will break. > Do we have any information on userspace that actually needs this functionality? Note that we're _not_ talking here about the complete USERPTR functionality, but rather just the very corner case of carveout memory not backed by struct pages. Given that the current in-tree ways of reserving carveout memory, such as shared-dma-pool, actually give memory backed by struct pages, do we even have a source of such legacy memory in the kernel today? I think that given that this is a very niche functionality, we could have it disabled by default for security reasons and if someone _really_ (i.e. there is no replacement) needs it, they probably need to use a custom kernel build anyway for their exotic hardware setup (with PFN-backed carveout memory), so they can enable it. > > > While I agree that using the userptr on media is something that > > > new drivers may not support, as DMABUF is a better way of > > > handling it, changing this for existing ones is a big no, > > > as it may break usersapace. > > > > media community needs to work to fix this, not pretend it is OK to > > keep going as-is. > > > Dealing with security issues is the one case where an uABI break might > > be acceptable. > > > > If you want to NAK it then you need to come up with the work to do > > something here correctly that will support the old drivers without the > > kernel taint. > > > > Unfortunately making things uncomfortable for the subsystem is the big > > hammer the core kernel needs to use to actually get this security work > > done by those responsible. > > > I'm not pretending that this is ok. Just pointing that the approach > taken is NOT OK. > > I'm not a mm/ expert, but, from what I understood from Daniel's patch > description is that this is unsafe *only if* __GFP_MOVABLE is used. > > Well, no drivers inside the media subsystem uses such flag, although > they may rely on some infrastructure that could be using it behind > the bars. > > If this is the case, the proper fix seems to have a GFP_NOT_MOVABLE > flag that it would be denying the core mm code to set __GFP_MOVABLE. > > Please let address the issue on this way, instead of broken an > userspace API that it is there since 1991. Note that USERPTR as a whole generally has been considered deprecated in V4L2 for many years and people have been actively discouraged to use it. And, still, we're just talking here about the very rare corner case, not the whole USERPTR API. Best regards, Tomasz ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn
Hi Daniel, On Fri, Oct 9, 2020 at 7:52 PM Daniel Vetter wrote: > > On Fri, Oct 9, 2020 at 2:48 PM Jason Gunthorpe wrote: > > > > On Fri, Oct 09, 2020 at 02:37:23PM +0200, Mauro Carvalho Chehab wrote: > > > > > I'm not a mm/ expert, but, from what I understood from Daniel's patch > > > description is that this is unsafe *only if* __GFP_MOVABLE is used. > > > > No, it is unconditionally unsafe. The CMA movable mappings are > > specific VMAs that will have bad issues here, but there are other > > types too. > > > > The only way to do something at a VMA level is to have a list of OK > > VMAs, eg because they were creatd via a special mmap helper from the > > media subsystem. > > > > > Well, no drivers inside the media subsystem uses such flag, although > > > they may rely on some infrastructure that could be using it behind > > > the bars. > > > > It doesn't matter, nothing prevents the user from calling media APIs > > on mmaps it gets from other subsystems. > > I think a good first step would be to disable userptr of non struct > page backed storage going forward for any new hw support. Even on > existing drivers. dma-buf sharing has been around for long enough now > that this shouldn't be a problem. Unfortunately right now this doesn't > seem to exist, so the entire problem keeps getting perpetuated. > > > > If this is the case, the proper fix seems to have a GFP_NOT_MOVABLE > > > flag that it would be denying the core mm code to set __GFP_MOVABLE. > > > > We can't tell from the VMA these kinds of details.. > > > > It has to go the other direction, evey mmap that might be used as a > > userptr here has to be found and the VMA specially created to allow > > its use. At least that is a kernel only change, but will need people > > with the HW to do this work. > > I think the only reasonable way to keep this working is: > - add a struct dma_buf *vma_tryget_dma_buf(struct vm_area_struct *vma); > - add dma-buf export support to fbdev and v4l I assume you mean V4L2 and not the obsolete V4L that is emulated in the userspace by libv4l. If so, every video device that uses videobuf2 gets DMA-buf export for free and there is nothing needed to enable it. We probably still have a few legacy drivers using videobuf (non-2), but IMHO those should be safe to put behind some disabled-by-default Kconfig symbol or even completely drop, as the legacy framework has been deprecated for many years already. > - roll this out everywhere we still need it. > > Realistically this just isn't going to happen. And anything else just > reimplements half of dma-buf, which is kinda pointless (you need > minimally refcounting and some way to get at a promise of a permanent > sg list for dma. Plus probably the vmap for kernel cpu access. > > > > Please let address the issue on this way, instead of broken an > > > userspace API that it is there since 1991. > > > > It has happened before :( It took 4 years for RDMA to undo the uAPI > > breakage caused by a security fix for something that was a 15 years > > old bug. > > Yeah we have a bunch of these on the drm side too. Some of them are > really just "you have to upgrade userspace", and there's no real fix > for the security nightmare without that. I think we need to phase out such userspace indeed. The Kconfig symbol allows enabling the unsafe functionality for anyone who still needs it, so I think it's not entirely a breakage. Best regards, Tomasz ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 205649] Daisy Chain (MST) Session Crash after Screen Lock Resume
https://bugzilla.kernel.org/show_bug.cgi?id=205649 Michael Rauch (mich...@rauch.be) changed: What|Removed |Added Kernel Version|5.3, 5.4, 5.5-rc3 |5.3, 5.4, 5.5-rc3, 5.9-rc7 --- Comment #4 from Michael Rauch (mich...@rauch.be) --- Error message: [drm] DM_MST: stopping TM on aconnector: 9b31f0b7 [id: 74] -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 205649] Daisy Chain (MST) Session Crash after Screen Lock Resume
https://bugzilla.kernel.org/show_bug.cgi?id=205649 --- Comment #3 from Michael Rauch (mich...@rauch.be) --- Same behavior with Kernel 5.9.0-050900rc7-generic and Mesa 20.0.8. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn
On Sat, Oct 10, 2020 at 1:39 PM Mauro Carvalho Chehab wrote: > > Em Sat, 10 Oct 2020 12:53:49 +0200 > Daniel Vetter escreveu: > > > Hi Mauro, > > > > You might want to read the patches more carefully, because what you're > > demanding is what my patches do. Short summary: > > > > - if STRICT_FOLLOW_PFN is set: > > a) normal memory is handled as-is (i.e. your example works) through > > the addition of FOLL_LONGTERM. This is the "pin the pages correctly" > > approach you're demanding > > b) for non-page memory (zerocopy sharing before dma-buf was upstreamed > > is the only use-case for this) it is correctly rejected with -EINVAL > > > > - if you do have blobby userspace which requires the zero-copy using > > userptr to work, and doesn't have any of the fallbacks implemented > > that you describe, this would be a regression. That's why > > STRICT_FOLLOW_PFN can be unset. And yes there's a real security issue > > in this usage, Marek also confirmed that the removal of the vma copy > > code a few years ago essentially broke even the weak assumptions that > > made the code work 10+ years ago when it was merged. > > > > so tdlr; Everything you described will keep working even with the new > > flag set, and everything you demand must be implemented _is_ > > implemented in this patch series. > > > > Also please keep in mind that we are _not_ talking about the general > > userptr support that was merge ~20 years ago. This patch series here > > is _only_ about the zerocpy userptr support merged with 50ac952d2263 > > ("[media] videobuf2-dma-sg: Support io userptr operations on io > > memory") in 2013. > > Ok, now it is making more sense. Please update the comments for > patch 10/17 to describe the above. Will do. > We need some time to test this though, in order to check if no > regressions were added (except the ones due to changeset 50ac952d2263). Yeah testing of the previous patches to switch to FOLL_LONGTERM would be really good. I also need that for habanalabs and ideally exynos too. All the userptr for normal memory should keep working, and with FOLL_LONGTERM it should actually work better, since with that it should now correctly interact with pagecache and fs code, not just with anon memory from malloc. Thanks, Daniel > > Why this hack was merged in 2013 when we merged dma-buf almost 2 years > > before that I have no idea about. Imo that patch simply should never > > have landed, and instead dma-buf support prioritized. > > If I recall correctly, we didn't have any DMABUF support > at the media subsystem, back on 2013. > > It took some time for the DMA-BUF to arrive at media, as this > was not a top priority. Also, there aren't many developers that > understand the memory model well enough to implement DMA-BUF support > and touch the VB2 code, which is quite complex, as it supports > lots of different ways for I/O, plus works with vmalloc, DMA > contig and DMA scatter/gather. > > Changes there should carefully be tested against different > drivers, in order to avoid regressions on it. > > > Cheers, Daniel > > Thanks, > Mauro -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn
Em Sat, 10 Oct 2020 12:53:49 +0200 Daniel Vetter escreveu: > Hi Mauro, > > You might want to read the patches more carefully, because what you're > demanding is what my patches do. Short summary: > > - if STRICT_FOLLOW_PFN is set: > a) normal memory is handled as-is (i.e. your example works) through > the addition of FOLL_LONGTERM. This is the "pin the pages correctly" > approach you're demanding > b) for non-page memory (zerocopy sharing before dma-buf was upstreamed > is the only use-case for this) it is correctly rejected with -EINVAL > > - if you do have blobby userspace which requires the zero-copy using > userptr to work, and doesn't have any of the fallbacks implemented > that you describe, this would be a regression. That's why > STRICT_FOLLOW_PFN can be unset. And yes there's a real security issue > in this usage, Marek also confirmed that the removal of the vma copy > code a few years ago essentially broke even the weak assumptions that > made the code work 10+ years ago when it was merged. > > so tdlr; Everything you described will keep working even with the new > flag set, and everything you demand must be implemented _is_ > implemented in this patch series. > > Also please keep in mind that we are _not_ talking about the general > userptr support that was merge ~20 years ago. This patch series here > is _only_ about the zerocpy userptr support merged with 50ac952d2263 > ("[media] videobuf2-dma-sg: Support io userptr operations on io > memory") in 2013. Ok, now it is making more sense. Please update the comments for patch 10/17 to describe the above. We need some time to test this though, in order to check if no regressions were added (except the ones due to changeset 50ac952d2263). > > Why this hack was merged in 2013 when we merged dma-buf almost 2 years > before that I have no idea about. Imo that patch simply should never > have landed, and instead dma-buf support prioritized. If I recall correctly, we didn't have any DMABUF support at the media subsystem, back on 2013. It took some time for the DMA-BUF to arrive at media, as this was not a top priority. Also, there aren't many developers that understand the memory model well enough to implement DMA-BUF support and touch the VB2 code, which is quite complex, as it supports lots of different ways for I/O, plus works with vmalloc, DMA contig and DMA scatter/gather. Changes there should carefully be tested against different drivers, in order to avoid regressions on it. > Cheers, Daniel Thanks, Mauro ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn
Hi Mauro, You might want to read the patches more carefully, because what you're demanding is what my patches do. Short summary: - if STRICT_FOLLOW_PFN is set: a) normal memory is handled as-is (i.e. your example works) through the addition of FOLL_LONGTERM. This is the "pin the pages correctly" approach you're demanding b) for non-page memory (zerocopy sharing before dma-buf was upstreamed is the only use-case for this) it is correctly rejected with -EINVAL - if you do have blobby userspace which requires the zero-copy using userptr to work, and doesn't have any of the fallbacks implemented that you describe, this would be a regression. That's why STRICT_FOLLOW_PFN can be unset. And yes there's a real security issue in this usage, Marek also confirmed that the removal of the vma copy code a few years ago essentially broke even the weak assumptions that made the code work 10+ years ago when it was merged. so tdlr; Everything you described will keep working even with the new flag set, and everything you demand must be implemented _is_ implemented in this patch series. Also please keep in mind that we are _not_ talking about the general userptr support that was merge ~20 years ago. This patch series here is _only_ about the zerocpy userptr support merged with 50ac952d2263 ("[media] videobuf2-dma-sg: Support io userptr operations on io memory") in 2013. Why this hack was merged in 2013 when we merged dma-buf almost 2 years before that I have no idea about. Imo that patch simply should never have landed, and instead dma-buf support prioritized. Cheers, Daniel On Sat, Oct 10, 2020 at 11:21 AM Mauro Carvalho Chehab wrote: > > Em Fri, 9 Oct 2020 19:52:05 +0200 > Daniel Vetter escreveu: > > > On Fri, Oct 9, 2020 at 2:48 PM Jason Gunthorpe wrote: > > > > > > On Fri, Oct 09, 2020 at 02:37:23PM +0200, Mauro Carvalho Chehab wrote: > > > > > > > I'm not a mm/ expert, but, from what I understood from Daniel's patch > > > > description is that this is unsafe *only if* __GFP_MOVABLE is used. > > > > > > No, it is unconditionally unsafe. The CMA movable mappings are > > > specific VMAs that will have bad issues here, but there are other > > > types too. > > I didn't check the mm dirty details, but I strongly suspect that the mm > code has a way to prevent moving a mmapped page while it is still in usage. > > If not, then the entire movable pages concept sounds broken to me, and > has to be fixed at mm subsystem. > > > > > > > The only way to do something at a VMA level is to have a list of OK > > > VMAs, eg because they were creatd via a special mmap helper from the > > > media subsystem. > > I'm not sure if I'm following you on that. The media API can work with > different ways of sending buffers to userspace: > > - read(); > > - using the overlay mode. This interface is deprecated in > practice, being replaced by DMABUF. Only a few older hardware > supports it, and it depends on an special X11 helper driver > for it to work. > > - via DMABUF: > > https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/dmabuf.html > > - via mmap, using a mmap helper: > > https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/mmap.html > > - via mmap, using userspace-provided pointers: > > https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/userp.html > > The existing open-source programs usually chose one or more of the above > modes. if the Kernel driver returns -EINVAL when an mmapped streaming I/O > mode is not supported, userspace has to select a different method. > > Most userspace open source programs have fallback support: if one > mmap I/O method fails, it selects another one, although this is not > a mandatory requirement. I found (and fixed) a few ones that were > relying exclusively on userptr support, but I didn't make a > comprehensive check. > > Also there are a number of relevant closed-source apps that we have no > idea about what methods they use, like Skype, and other similar > videoconferencing programs. Breaking support for those, specially at > a time where people are relying on it in order to participate on > conferences and doing internal meetings is a **very bad** idea. > > So, whatever solution is taken, it should not be dumping warning > messages at the system and tainting the Kernel, but, instead, checking > if the userspace request is valid or not. If it is invalid, return the > proper error code via the right V4L2 ioctl, in order to let userspace > choose a different method. I the request is valid, refcount the pages > for them to not be moved while they're still in usage. > > - > > Let me provide some background about how things work at the media > subsytem. If you want to know more, the userspace-provided memory > mapped pointers work is described here: > > >
[PATCH] drm/amd/display: remove no need return value
Functions (disable_all_writeback_pipes_for_stream & dc_enable_stereo & dc_post_update_surfaces_to_stream) always return true, there is no need to keep the return value. This change is to make the code a bit more readable. Signed-off-by: Bernard Zhao --- drivers/gpu/drm/amd/display/dc/core/dc.c | 17 + drivers/gpu/drm/amd/display/dc/dc.h| 2 +- drivers/gpu/drm/amd/display/dc/dc_stream.h | 2 +- 3 files changed, 7 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c index 92eb1ca1634f..8dc598a632b5 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c @@ -761,7 +761,7 @@ static bool dc_construct(struct dc *dc, return false; } -static bool disable_all_writeback_pipes_for_stream( +static void disable_all_writeback_pipes_for_stream( const struct dc *dc, struct dc_stream_state *stream, struct dc_state *context) @@ -770,8 +770,6 @@ static bool disable_all_writeback_pipes_for_stream( for (i = 0; i < stream->num_wb_info; i++) stream->writeback_info[i].wb_enabled = false; - - return true; } void apply_ctx_interdependent_lock(struct dc *dc, struct dc_state *context, struct dc_stream_state *stream, bool lock) @@ -1213,13 +1211,12 @@ bool dc_validate_seamless_boot_timing(const struct dc *dc, return true; } -bool dc_enable_stereo( +void dc_enable_stereo( struct dc *dc, struct dc_state *context, struct dc_stream_state *streams[], uint8_t stream_count) { - bool ret = true; int i, j; struct pipe_ctx *pipe; @@ -1234,8 +1231,6 @@ bool dc_enable_stereo( dc->hwss.setup_stereo(pipe, dc); } } - - return ret; } /* @@ -1448,18 +1443,18 @@ static bool is_flip_pending_in_pipes(struct dc *dc, struct dc_state *context) return false; } -bool dc_post_update_surfaces_to_stream(struct dc *dc) +void dc_post_update_surfaces_to_stream(struct dc *dc) { int i; struct dc_state *context = dc->current_state; if ((!dc->optimized_required) || dc->optimize_seamless_boot_streams > 0) - return true; + return; post_surface_trace(dc); if (is_flip_pending_in_pipes(dc, context)) - return true; + return; for (i = 0; i < dc->res_pool->pipe_count; i++) if (context->res_ctx.pipe_ctx[i].stream == NULL || @@ -1472,8 +1467,6 @@ bool dc_post_update_surfaces_to_stream(struct dc *dc) dc->optimized_required = false; dc->wm_optimized_required = false; - - return true; } struct dc_state *dc_create_state(struct dc *dc) diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h index f50ef4255020..f79a3c318757 100644 --- a/drivers/gpu/drm/amd/display/dc/dc.h +++ b/drivers/gpu/drm/amd/display/dc/dc.h @@ -962,7 +962,7 @@ struct dc_flip_addrs { bool triplebuffer_flips; }; -bool dc_post_update_surfaces_to_stream( +void dc_post_update_surfaces_to_stream( struct dc *dc); #include "dc_stream.h" diff --git a/drivers/gpu/drm/amd/display/dc/dc_stream.h b/drivers/gpu/drm/amd/display/dc/dc_stream.h index d9888f316da6..0047ab33f88e 100644 --- a/drivers/gpu/drm/amd/display/dc/dc_stream.h +++ b/drivers/gpu/drm/amd/display/dc/dc_stream.h @@ -391,7 +391,7 @@ enum dc_status dc_validate_stream(struct dc *dc, struct dc_stream_state *stream) * Enable stereo when commit_streams is not required, * for example, frame alternate. */ -bool dc_enable_stereo( +void dc_enable_stereo( struct dc *dc, struct dc_state *context, struct dc_stream_state *streams[], -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-misc-next-fixes
Hi Dave, Daniel, Here's this week PR for drm-misc-next-fixes Maxime drm-misc-next-fixes-2020-10-09: One MAINTAINERS change and a revert for a compilation breakage in next for ingenic The following changes since commit 089d83418914abd4d908db117d9a3eca7f51a68c: drm/vc4: hvs: Pull the state of all the CRTCs prior to PV muxing (2020-09-21 16:43:11 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-fixes-2020-10-09 for you to fetch changes up to 6561e0aa4627da90f59076fec5e3a1b72a8aa63f: MAINTAINERS: Update entry for st7703 driver after the rename (2020-10-09 08:55:00 +0200) One MAINTAINERS change and a revert for a compilation breakage in next for ingenic Maxime Ripard (3): drm/vc4: kms: Assign a FIFO to enabled CRTCs instead of active drm/vc4: crtc: Rework a bit the CRTC state code drm/vc4: crtc: Keep the previously assigned HVS FIFO Ondrej Jirman (1): MAINTAINERS: Update entry for st7703 driver after the rename Paul Cercueil (1): Revert "gpu/drm: ingenic: Add option to mmap GEM buffers cached" MAINTAINERS | 7 +- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 116 +++--- drivers/gpu/drm/ingenic/ingenic-drm.h | 4 -- drivers/gpu/drm/ingenic/ingenic-ipu.c | 12 +--- drivers/gpu/drm/vc4/vc4_crtc.c| 14 +++- drivers/gpu/drm/vc4/vc4_drv.h | 2 + drivers/gpu/drm/vc4/vc4_kms.c | 22 -- 7 files changed, 46 insertions(+), 131 deletions(-) signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/6] mm: mmap: fix fput in error path
On Fri, Oct 09, 2020 at 05:03:37PM +0200, Christian König wrote: > Patch "495c10cc1c0c CHROMIUM: dma-buf: restore args..." > adds a workaround for a bug in mmap_region. > > As the comment states ->mmap() callback can change > vma->vm_file and so we might call fput() on the wrong file. > > Revert the workaround and proper fix this in mmap_region. > > Signed-off-by: Christian König > drivers/dma-buf/dma-buf.c | 22 +- > mm/mmap.c | 2 +- > 2 files changed, 6 insertions(+), 18 deletions(-) > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > index a6ba4d598f0e..edd57402a48a 100644 > +++ b/drivers/dma-buf/dma-buf.c > @@ -1143,9 +1143,6 @@ EXPORT_SYMBOL_GPL(dma_buf_end_cpu_access); > int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma, >unsigned long pgoff) > { > - struct file *oldfile; > - int ret; > - > if (WARN_ON(!dmabuf || !vma)) > return -EINVAL; > > @@ -1163,22 +1160,13 @@ int dma_buf_mmap(struct dma_buf *dmabuf, struct > vm_area_struct *vma, > return -EINVAL; > > /* readjust the vma */ > - get_file(dmabuf->file); > - oldfile = vma->vm_file; > - vma->vm_file = dmabuf->file; > - vma->vm_pgoff = pgoff; > + if (vma->vm_file) > + fput(vma->vm_file); This if is redundant too But otherwise Reviewed-by: Jason Gunthorpe Thanks, Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] mm: introduce vma_set_file function v2
On Fri, Oct 09, 2020 at 09:39:00AM +0200, Daniel Vetter wrote: > I just noticed this here in the patch because everyone else does not do > this. But looking at the mmap_region() code in mmap.c we seem to indeed > have this problem for the error path: > > unmap_and_free_vma: > vma->vm_file = NULL; > fput(file); > > Note that the success path does things correctly (a bit above): > > file = vma->vm_file; > out: > > So it indeed looks like dma-buf is the only one that does this fully > correctly. So maybe we should do a follow-up patch to change the > mmap_region exit code to pick up whatever vma->vm_file was set instead, > and fput that? Given that this new vma_set_file() should be the only way to manipulate vm_file from the mmap op, I think this reflects a bug in mm/mmap.c.. Should be: unmap_and_free_vma: fput(vma->vm_file); vma->vm_file = NULL; Then everything works the way you'd expect without tricky error handling Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/i915: Force DPCD backlight mode for BOE 2270 panel
BOE 2270 panel failed to control backlight brightness. Add it in edid quirks to force using DPCD backlight control. Then the brightness can be controlled. Signed-off-by: Aaron Ma --- drivers/gpu/drm/drm_dp_helper.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 092c8c985911..417ed10bbf83 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -1324,6 +1324,7 @@ static const struct edid_quirk edid_quirk_list[] = { { MFG(0x4d, 0x10), PROD_ID(0xc7, 0x14), BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) }, { MFG(0x4d, 0x10), PROD_ID(0xe6, 0x14), BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) }, { MFG(0x4c, 0x83), PROD_ID(0x47, 0x41), BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) }, + { MFG(0x09, 0xe5), PROD_ID(0xde, 0x08), BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) }, }; #undef MFG -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/7] dt-bindings: display: mxsfb: Add a bus-width endpoint property
On 10/10/20 1:58 AM, Laurent Pinchart wrote: > Hi Marek, Hi, > On Wed, Oct 07, 2020 at 10:40:26AM +0200, Marek Vasut wrote: >> On 10/7/20 3:24 AM, Laurent Pinchart wrote: >> >> [...] >> >>> + bus-width: >>> +enum: [16, 18, 24] >>> +description: | >>> + The output bus width. This value overrides the configuration >>> + derived from the connected device (encoder or panel). It >>> should >>> + only be specified when PCB routing of the data signals >>> require a >>> + different bus width on the LCDIF and the connected device. >>> For >>> + instance, when a 18-bit RGB panel has its R[5:0], G[5:0] and >>> + B[5:0] signals connected to LCD_DATA[7:2], LCD_DATA[15:10] >>> and >>> + LCD_DATA[23:18] instead of LCD_DATA[5:0], LCD_DATA[11:6] and >>> + LCD_DATA[17:12], bus-width should be set to 24. >> >> The iMX6 IPUv3 uses interface-pix-fmt which is a bit more flexible, but >> I'm not sure whether it's the right way to go about this, see: >> Documentation/devicetree/bindings/display/imx/fsl-imx-drm.txt > > I think specifying the bus with is better. It's a standard property, but > more than that, a given bus width can carry different formats. For > instance, a 24-bus could carry RGB666 data (with dithering for the > LSBs). I think that's exactly what the interface-pix-fmt was trying to solve for the IPUv3, there you could have e.g. both RGB666 and LVDS666 , which were different. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC PKS/PMEM 51/58] kernel: Utilize new kmap_thread()
ira.we...@intel.com writes: > From: Ira Weiny > > This kmap() call is localized to a single thread. To avoid the over > head of global PKRS updates use the new kmap_thread() call. Acked-by: "Eric W. Biederman" > > Cc: Eric Biederman > Signed-off-by: Ira Weiny > --- > kernel/kexec_core.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c > index c19c0dad1ebe..272a9920c0d6 100644 > --- a/kernel/kexec_core.c > +++ b/kernel/kexec_core.c > @@ -815,7 +815,7 @@ static int kimage_load_normal_segment(struct kimage > *image, > if (result < 0) > goto out; > > - ptr = kmap(page); > + ptr = kmap_thread(page); > /* Start with a clear page */ > clear_page(ptr); > ptr += maddr & ~PAGE_MASK; > @@ -828,7 +828,7 @@ static int kimage_load_normal_segment(struct kimage > *image, > memcpy(ptr, kbuf, uchunk); > else > result = copy_from_user(ptr, buf, uchunk); > - kunmap(page); > + kunmap_thread(page); > if (result) { > result = -EFAULT; > goto out; > @@ -879,7 +879,7 @@ static int kimage_load_crash_segment(struct kimage *image, > goto out; > } > arch_kexec_post_alloc_pages(page_address(page), 1, 0); > - ptr = kmap(page); > + ptr = kmap_thread(page); > ptr += maddr & ~PAGE_MASK; > mchunk = min_t(size_t, mbytes, > PAGE_SIZE - (maddr & ~PAGE_MASK)); > @@ -895,7 +895,7 @@ static int kimage_load_crash_segment(struct kimage *image, > else > result = copy_from_user(ptr, buf, uchunk); > kexec_flush_icache_page(page); > - kunmap(page); > + kunmap_thread(page); > arch_kexec_pre_free_pages(page_address(page), 1); > if (result) { > result = -EFAULT; ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] nouveau broken on Riva TNT2 in 5.9.0-rc8: GPU not supported on big-endian
On Saturday 10 October 2020 00:23:38 Ilia Mirkin wrote: > On Fri, Oct 9, 2020 at 5:54 PM Karol Herbst wrote: > > > > On Fri, Oct 9, 2020 at 11:35 PM Ondrej Zary wrote: > > > > > > Hello, > > > I'm testing 5.9.0-rc8 and found that Riva TNT2 stopped working: > > > [0.00] Linux version 5.9.0-rc8+ (zary@gsql) (gcc (Debian 8.3.0-6) > > > 8.3.0, GNU ld (GNU Binutils for Debian) 2.31.1) #326 SMP Fri Oct 9 > > > 22:31:40 CEST 2020 > > > ... > > > [ 14.771464] nouveau :01:00.0: GPU not supported on big-endian > > > [ 14.771782] nouveau: probe of :01:00.0 failed with error -38 > > > > > > big-endian? WTF? The machine is x86. > > > > > > > mhh, we reworked the endianess checks a bit and apparently that broke > > something... I will give it some thoughts, but could you be so kind > > and create an mmiotrace under 5.9 with nouveau? You won't need to > > start X or anything while doing it. Just enable the trace and modprobe > > nouveau and collect the trace. > > Looks like nvkm_device_endianness unconditionally reads out 0x4. I > don't think that reg is there pre-NV11. At least NV4, NV5, NV10 and > maybe NV15 (which is logically pre-NV11) don't support big-endian > mode. Not sure about NV1A, which was the IGP of the series and IIRC > logically pre-NV11 as well (but clearly could only be used with x86 > chips, since it was part of the motherboard). Yes, you're right. Forcing nvkm_device_endianness to return true allows 5.9.0-rc8 to work: [0.00] Linux version 5.9.0-rc8+ (zary@gsql) (gcc (Debian 8.3.0-6) 8.3.0, GNU ld (GNU Binutils for Debian) 2.31.1) #326 SMP Fri Oct 9 22:31:40 CEST 2020 ... [ 12.311258] nouveau :01:00.0: bios: DCB table not found [ 12.311583] nouveau :01:00.0: bios: DCB table not found [ 12.311834] nouveau :01:00.0: bios: DCB table not found [ 12.311847] nouveau :01:00.0: bios: DCB table not found [ 12.311989] agpgart-intel :00:00.0: AGP 3.0 bridge [ 12.312017] agpgart-intel :00:00.0: bridge is in legacy mode, falling back to 2.x [ 12.312031] agpgart-intel :00:00.0: putting AGP V2 device into 4x mode [ 12.312066] nouveau :01:00.0: putting AGP V2 device into 4x mode [ 12.312162] agpgart-intel :00:00.0: AGP 3.0 bridge [ 12.312182] agpgart-intel :00:00.0: bridge is in legacy mode, falling back to 2.x [ 12.312195] agpgart-intel :00:00.0: putting AGP V2 device into 4x mode [ 12.312230] nouveau :01:00.0: putting AGP V2 device into 4x mode [ 12.312247] nouveau :01:00.0: tmr: unknown input clock freq [ 12.318341] nouveau :01:00.0: fb: 32 MiB SDRAM [ 12.76] [TTM] Zone kernel: Available graphics memory: 385048 KiB [ 12.92] [TTM] Initializing pool allocator [ 12.333434] nouveau :01:00.0: DRM: VRAM: 31 MiB [ 12.333443] nouveau :01:00.0: DRM: GART: 128 MiB [ 12.333453] nouveau :01:00.0: DRM: BMP version 5.6 [ 12.333460] nouveau :01:00.0: DRM: No DCB data found in VBIOS [ 12.335355] nouveau :01:00.0: DRM: MM: using M2MF for buffer copies [ 12.335443] nouveau :01:00.0: bios: DCB table not found [ 12.336033] nouveau :01:00.0: DRM: Saving VGA fonts [ 12.376420] nouveau :01:00.0: DRM: No DCB data found in VBIOS [ 12.410397] nouveau :01:00.0: DRM: allocated 1280x1024 fb: 0x4000, bo b68d2ac4 [ 12.441217] fbcon: nouveaudrmfb (fb0) is primary device [ 12.591964] Console: switching to colour frame buffer device 160x64 [ 12.593876] nouveau :01:00.0: [drm] fb0: nouveaudrmfb frame buffer device [ 12.594944] [drm] Initialized nouveau 1.3.1 20120801 for :01:00.0 on minor 0 BTW. 5.8 kernel (that appeared today in Debian packports) is broken the same way. > Aha, it's documented in rnndb: > > https://github.com/envytools/envytools/blob/master/rnndb/bus/pmc.xml > > > -ilia > -- Ondrej Zary ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 14/17] resource: Move devmem revoke code to resource framework
On Fri, Oct 09, 2020 at 04:24:45PM +0200, Daniel Vetter wrote: > On Fri, Oct 9, 2020 at 2:31 PM Jason Gunthorpe wrote: > > > > On Fri, Oct 09, 2020 at 09:59:31AM +0200, Daniel Vetter wrote: > > > > > +struct address_space *iomem_get_mapping(void) > > > +{ > > > + return iomem_inode->i_mapping; > > > > This should pair an acquire with the release below > > > > > + /* > > > + * Publish /dev/mem initialized. > > > + * Pairs with smp_load_acquire() in revoke_iomem(). > > > + */ > > > + smp_store_release(_inode, inode); > > > > However, this seems abnormal, initcalls rarely do this kind of stuff > > with global data.. > > > > The kernel crashes if this fs_initcall is raced with > > iomem_get_mapping() due to the unconditional dereference, so I think > > it can be safely switched to a simple assignment. > > Ah yes I checked this all, but forgot to correctly annotate the > iomem_get_mapping access. For reference, see b34e7e298d7a ("/dev/mem: > Add missing memory barriers for devmem_inode"). Oh yikes, so revoke_iomem can run concurrently during early boot, tricky. > The reasons for the annotations is that iomem requests can happen > fairly early, way before fs_initcalls happen. That means revoke_iomem > needs to check for that and bail out if we race - nothing bad can > happen since userspace isn't running at this point anyway. And > apparently it needs to be a full acquire fence since we don't just > write a value, but need a barrier for the struct stuff. Yes, if that is what is happening it release/acquire is needed. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/nouveau/kms: Fix NULL pointer dereference in nouveau_connector_detect_depth
This oops manifests itself on the following hardware: 01:00.0 VGA compatible controller: NVIDIA Corporation G98M [GeForce G 103M] (rev a1) Oct 09 14:17:46 lp-sasha kernel: BUG: kernel NULL pointer dereference, address: Oct 09 14:17:46 lp-sasha kernel: #PF: supervisor read access in kernel mode Oct 09 14:17:46 lp-sasha kernel: #PF: error_code(0x) - not-present page Oct 09 14:17:46 lp-sasha kernel: PGD 0 P4D 0 Oct 09 14:17:46 lp-sasha kernel: Oops: [#1] SMP PTI Oct 09 14:17:46 lp-sasha kernel: CPU: 1 PID: 191 Comm: systemd-udevd Not tainted 5.9.0-rc8-next-20201009 #38 Oct 09 14:17:46 lp-sasha kernel: Hardware name: Hewlett-Packard Compaq Presario CQ61 Notebook PC/306A, BIOS F.03 03/23/2009 Oct 09 14:17:46 lp-sasha kernel: RIP: 0010:nouveau_connector_detect_depth+0x71/0xc0 [nouveau] Oct 09 14:17:46 lp-sasha kernel: Code: 0a 00 00 48 8b 49 48 c7 87 b8 00 00 00 06 00 00 00 80 b9 4d 0a 00 00 00 75 1e 83 fa 41 75 05 48 85 c0 75 29 8b 81 10 0d 00 00 <39> 06 7c 25 f6 81 14 0d 00 00 02 75 b7 c3 80 b9 0c 0d 00 00 00 75 Oct 09 14:17:46 lp-sasha kernel: RSP: 0018:c928f8c0 EFLAGS: 00010297 Oct 09 14:17:46 lp-sasha kernel: RAX: 00014c08 RBX: 8880369d4000 RCX: 8880369d3000 Oct 09 14:17:46 lp-sasha kernel: RDX: 0040 RSI: RDI: 8880369d4000 Oct 09 14:17:46 lp-sasha kernel: RBP: 88800601cc00 R08: 8880051da298 R09: 8226201a Oct 09 14:17:46 lp-sasha kernel: R10: 88800469aa80 R11: 888004c84ff8 R12: Oct 09 14:17:46 lp-sasha kernel: R13: 8880051da000 R14: 2000 R15: 0003 Oct 09 14:17:46 lp-sasha kernel: FS: 7fd0192b3440() GS:8880bc90() knlGS: Oct 09 14:17:46 lp-sasha kernel: CS: 0010 DS: ES: CR0: 80050033 Oct 09 14:17:46 lp-sasha kernel: CR2: CR3: 04976000 CR4: 06e0 Oct 09 14:17:46 lp-sasha kernel: Call Trace: Oct 09 14:17:46 lp-sasha kernel: nouveau_connector_get_modes+0x1e6/0x240 [nouveau] Oct 09 14:17:46 lp-sasha kernel: ? kfree+0xb9/0x240 Oct 09 14:17:46 lp-sasha kernel: ? drm_connector_list_iter_next+0x7c/0xa0 Oct 09 14:17:46 lp-sasha kernel: drm_helper_probe_single_connector_modes+0x1ba/0x7c0 Oct 09 14:17:46 lp-sasha kernel: drm_client_modeset_probe+0x27e/0x1360 Oct 09 14:17:46 lp-sasha kernel: ? nvif_object_sclass_put+0xc/0x20 [nouveau] Oct 09 14:17:46 lp-sasha kernel: ? nouveau_cli_init+0x3cc/0x440 [nouveau] Oct 09 14:17:46 lp-sasha kernel: ? ktime_get_mono_fast_ns+0x49/0xa0 Oct 09 14:17:46 lp-sasha kernel: ? nouveau_drm_open+0x4e/0x180 [nouveau] Oct 09 14:17:46 lp-sasha kernel: __drm_fb_helper_initial_config_and_unlock+0x3f/0x4a0 Oct 09 14:17:46 lp-sasha kernel: ? drm_file_alloc+0x18f/0x260 Oct 09 14:17:46 lp-sasha kernel: ? mutex_lock+0x9/0x40 Oct 09 14:17:46 lp-sasha kernel: ? drm_client_init+0x110/0x160 Oct 09 14:17:46 lp-sasha kernel: nouveau_fbcon_init+0x14d/0x1c0 [nouveau] Oct 09 14:17:46 lp-sasha kernel: nouveau_drm_device_init+0x1c0/0x880 [nouveau] Oct 09 14:17:46 lp-sasha kernel: nouveau_drm_probe+0x11a/0x1e0 [nouveau] Oct 09 14:17:46 lp-sasha kernel: pci_device_probe+0xcd/0x140 Oct 09 14:17:46 lp-sasha kernel: really_probe+0xd8/0x400 Oct 09 14:17:46 lp-sasha kernel: driver_probe_device+0x4a/0xa0 Oct 09 14:17:46 lp-sasha kernel: device_driver_attach+0x9c/0xc0 Oct 09 14:17:46 lp-sasha kernel: __driver_attach+0x6f/0x100 Oct 09 14:17:46 lp-sasha kernel: ? device_driver_attach+0xc0/0xc0 Oct 09 14:17:46 lp-sasha kernel: bus_for_each_dev+0x75/0xc0 Oct 09 14:17:46 lp-sasha kernel: bus_add_driver+0x106/0x1c0 Oct 09 14:17:46 lp-sasha kernel: driver_register+0x86/0xe0 Oct 09 14:17:46 lp-sasha kernel: ? 0xa044e000 Oct 09 14:17:46 lp-sasha kernel: do_one_initcall+0x48/0x1e0 Oct 09 14:17:46 lp-sasha kernel: ? _cond_resched+0x11/0x60 Oct 09 14:17:46 lp-sasha kernel: ? kmem_cache_alloc_trace+0x19c/0x1e0 Oct 09 14:17:46 lp-sasha kernel: do_init_module+0x57/0x220 Oct 09 14:17:46 lp-sasha kernel: __do_sys_finit_module+0xa0/0xe0 Oct 09 14:17:46 lp-sasha kernel: do_syscall_64+0x33/0x40 Oct 09 14:17:46 lp-sasha kernel: entry_SYSCALL_64_after_hwframe+0x44/0xa9 Oct 09 14:17:46 lp-sasha kernel: RIP: 0033:0x7fd01a060d5d Oct 09 14:17:46 lp-sasha kernel: Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d e3 70 0c 00 f7 d8 64 89 01 48 Oct 09 14:17:46 lp-sasha kernel: RSP: 002b:7ffc8ad38a98 EFLAGS: 0246 ORIG_RAX: 0139 Oct 09 14:17:46 lp-sasha kernel: RAX: ffda RBX: 563f6e7fd530 RCX: 7fd01a060d5d Oct 09 14:17:46 lp-sasha kernel: RDX: RSI: 7fd01a19f95d RDI: 000f Oct 09 14:17:46 lp-sasha kernel: RBP: 0002 R08: R09: 0007 Oct 09 14:17:46 lp-sasha kernel: R10: 000f R11: 0246 R12:
Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn
On Fri, Oct 09, 2020 at 02:37:23PM +0200, Mauro Carvalho Chehab wrote: > I'm not a mm/ expert, but, from what I understood from Daniel's patch > description is that this is unsafe *only if* __GFP_MOVABLE is used. No, it is unconditionally unsafe. The CMA movable mappings are specific VMAs that will have bad issues here, but there are other types too. The only way to do something at a VMA level is to have a list of OK VMAs, eg because they were creatd via a special mmap helper from the media subsystem. > Well, no drivers inside the media subsystem uses such flag, although > they may rely on some infrastructure that could be using it behind > the bars. It doesn't matter, nothing prevents the user from calling media APIs on mmaps it gets from other subsystems. > If this is the case, the proper fix seems to have a GFP_NOT_MOVABLE > flag that it would be denying the core mm code to set __GFP_MOVABLE. We can't tell from the VMA these kinds of details.. It has to go the other direction, evey mmap that might be used as a userptr here has to be found and the VMA specially created to allow its use. At least that is a kernel only change, but will need people with the HW to do this work. > Please let address the issue on this way, instead of broken an > userspace API that it is there since 1991. It has happened before :( It took 4 years for RDMA to undo the uAPI breakage caused by a security fix for something that was a 15 years old bug. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/6] mm: introduce vma_set_file function v3
On Fri, Oct 09, 2020 at 05:03:38PM +0200, Christian König wrote: > +/* > + * Change backing file, only valid to use during initial VMA setup. > + */ > +void vma_set_file(struct vm_area_struct *vma, struct file *file) > +{ > + if (file) > + get_file(file); > + > + swap(vma->vm_file, file); > + > + if (file) > + fput(file); > +} fput crashes when file is NULL so the error handling after unmap_and_free_vma: can't handle this case, similarly vm_file can't be NULL either. So just simply: swap(vma->vm_file, file); get_file(vma->vm_file); fput(file); Will do? Just let it crash if any of them are wrongly NULL. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] MAINTAINERS: Add myself as a maintainer for vc4
Eric isn't working on vc4 anymore and I've been working on it, as well as merging patches for it, recently so let's make it official so I don't miss patches. Signed-off-by: Maxime Ripard --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 91d46806e511..a248e3a2b537 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5957,6 +5957,7 @@ F:include/uapi/drm/v3d_drm.h DRM DRIVERS FOR VC4 M: Eric Anholt +M: Maxime Ripard S: Supported T: git git://github.com/anholt/linux T: git git://anongit.freedesktop.org/drm/drm-misc -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/6] mm: mmap: fix fput in error path
Jason Gunthorpe wrote: > On Fri, Oct 09, 2020 at 03:04:20PM -0700, Andrew Morton wrote: >> On Fri, 9 Oct 2020 17:03:37 +0200 "Christian König" >> wrote: >> >> > Patch "495c10cc1c0c CHROMIUM: dma-buf: restore args..." >> > adds a workaround for a bug in mmap_region. >> > >> > As the comment states ->mmap() callback can change >> > vma->vm_file and so we might call fput() on the wrong file. >> > >> > Revert the workaround and proper fix this in mmap_region. >> > >> >> Doesn't this patch series address the same thing as >> https://lkml.kernel.org/r/20200916090733.31427-1-linmia...@huawei.com? > >Same basic issue, looks like both of these patches should be combined to plug >it fully. > >Jason I think so too. Both of these patches fix the fput at possible wrong @file due to ->mmap() callback can change vma->vm_file. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
nouveau broken on Riva TNT2 in 5.9.0-rc8: GPU not supported on big-endian
Hello, I'm testing 5.9.0-rc8 and found that Riva TNT2 stopped working: [0.00] Linux version 5.9.0-rc8+ (zary@gsql) (gcc (Debian 8.3.0-6) 8.3.0, GNU ld (GNU Binutils for Debian) 2.31.1) #326 SMP Fri Oct 9 22:31:40 CEST 2020 ... [ 14.771464] nouveau :01:00.0: GPU not supported on big-endian [ 14.771782] nouveau: probe of :01:00.0 failed with error -38 big-endian? WTF? The machine is x86. It works fine with Debian 5.7 kernel (5.7.10-1~bpo10+1): [0.00] Linux version 5.7.0-0.bpo.2-686 (debian-ker...@lists.debian.org) (gcc version 8.3.0 (Debian 8.3.0-6), GNU ld (GNU Binutils for Debian) 2.31.1) #1 SMP Debian 5.7.10-1~bpo10+1 (2020-07-30) ... [ 23.266196] nouveau :01:00.0: NVIDIA NV05 (20154000) [ 23.288582] nouveau :01:00.0: bios: version 02.05.20.02.00 [ 23.288869] nouveau :01:00.0: bios: DCB table not found [ 23.289595] nouveau :01:00.0: bios: DCB table not found [ 23.289956] nouveau :01:00.0: bios: DCB table not found [ 23.290015] nouveau :01:00.0: bios: DCB table not found [ 23.290215] agpgart-intel :00:00.0: AGP 3.0 bridge [ 23.290287] agpgart-intel :00:00.0: bridge is in legacy mode, falling back to 2.x [ 23.290351] agpgart-intel :00:00.0: putting AGP V2 device into 4x mode [ 23.290430] nouveau :01:00.0: putting AGP V2 device into 4x mode [ 23.290565] agpgart-intel :00:00.0: AGP 3.0 bridge [ 23.290627] agpgart-intel :00:00.0: bridge is in legacy mode, falling back to 2.x [ 23.290690] agpgart-intel :00:00.0: putting AGP V2 device into 4x mode [ 23.290768] nouveau :01:00.0: putting AGP V2 device into 4x mode [ 23.290830] nouveau :01:00.0: tmr: unknown input clock freq [ 23.293026] nouveau :01:00.0: fb: 32 MiB SDRAM [ 23.301269] [TTM] Zone kernel: Available graphics memory: 382728 KiB [ 23.301327] [TTM] Initializing pool allocator [ 23.301414] nouveau :01:00.0: DRM: VRAM: 31 MiB [ 23.301465] nouveau :01:00.0: DRM: GART: 128 MiB [ 23.301518] nouveau :01:00.0: DRM: BMP version 5.6 [ 23.301570] nouveau :01:00.0: DRM: No DCB data found in VBIOS [ 23.303594] nouveau :01:00.0: DRM: MM: using M2MF for buffer copies [ 23.303719] nouveau :01:00.0: bios: DCB table not found [ 23.304904] nouveau :01:00.0: DRM: Saving VGA fonts [ 23.349089] nouveau :01:00.0: DRM: No DCB data found in VBIOS [ 23.349681] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). [ 23.383066] nouveau :01:00.0: DRM: allocated 1280x1024 fb: 0x4000, bo b10d2f17 [ 23.413903] fbcon: nouveaudrmfb (fb0) is primary device [ 23.569851] Console: switching to colour frame buffer device 160x64 [ 23.571050] nouveau :01:00.0: fb0: nouveaudrmfb frame buffer device -- Ondrej Zary ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm/disp: add error value record in for circle`s error index
In function dpu_core_irq_enable & dpu_core_irq_disable, when some index enable or disable failed, return value will be covered by next success index. Upper call function will not catch this error, this maybe does not meet the expectation. This change is to make the code a bit more readable. Signed-off-by: Bernard Zhao --- drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c index f1bc6a1af7a7..e6da0469b743 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c @@ -123,8 +123,8 @@ int dpu_core_irq_enable(struct dpu_kms *dpu_kms, int *irq_idxs, u32 irq_count) DRM_ERROR("irq_idx=%d enable_count=%d\n", irq_idxs[0], counts); for (i = 0; (i < irq_count) && !ret; i++) - ret = _dpu_core_irq_enable(dpu_kms, irq_idxs[i]); - + if (_dpu_core_irq_enable(dpu_kms, irq_idxs[i]) != 0) + ret = -EINVAL; return ret; } @@ -178,8 +178,8 @@ int dpu_core_irq_disable(struct dpu_kms *dpu_kms, int *irq_idxs, u32 irq_count) DRM_ERROR("irq_idx=%d enable_count=%d\n", irq_idxs[0], counts); for (i = 0; (i < irq_count) && !ret; i++) - ret = _dpu_core_irq_disable(dpu_kms, irq_idxs[i]); - + if (_dpu_core_irq_disable(dpu_kms, irq_idxs[i]) != 0) + ret = -EINVAL; return ret; } -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[tip: core/rcu] drm/i915: Cleanup PREEMPT_COUNT leftovers
The following commit has been merged into the core/rcu branch of tip: Commit-ID: 5d35c1c982ffaaccd1ec974e96e7a5244bbadaa1 Gitweb: https://git.kernel.org/tip/5d35c1c982ffaaccd1ec974e96e7a5244bbadaa1 Author:Thomas Gleixner AuthorDate:Mon, 14 Sep 2020 19:35:03 +02:00 Committer: Paul E. McKenney CommitterDate: Thu, 01 Oct 2020 09:02:13 -07:00 drm/i915: Cleanup PREEMPT_COUNT leftovers CONFIG_PREEMPT_COUNT is now unconditionally enabled and will be removed. Cleanup the leftovers before doing so. Signed-off-by: Thomas Gleixner Reviewed-by: Joonas Lahtinen Cc: Jani Nikula Cc: Rodrigo Vivi Cc: David Airlie Cc: Daniel Vetter Cc: intel-...@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org Signed-off-by: Paul E. McKenney --- drivers/gpu/drm/i915/Kconfig.debug | 1 - drivers/gpu/drm/i915/i915_utils.h | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug index 1cb28c2..17d9b00 100644 --- a/drivers/gpu/drm/i915/Kconfig.debug +++ b/drivers/gpu/drm/i915/Kconfig.debug @@ -20,7 +20,6 @@ config DRM_I915_DEBUG bool "Enable additional driver debugging" depends on DRM_I915 select DEBUG_FS - select PREEMPT_COUNT select I2C_CHARDEV select STACKDEPOT select DRM_DP_AUX_CHARDEV diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h index 5477337..ecfed86 100644 --- a/drivers/gpu/drm/i915/i915_utils.h +++ b/drivers/gpu/drm/i915/i915_utils.h @@ -337,8 +337,7 @@ wait_remaining_ms_from_jiffies(unsigned long timestamp_jiffies, int to_wait_ms) (Wmax)) #define wait_for(COND, MS) _wait_for((COND), (MS) * 1000, 10, 1000) -/* If CONFIG_PREEMPT_COUNT is disabled, in_atomic() always reports false. */ -#if defined(CONFIG_DRM_I915_DEBUG) && defined(CONFIG_PREEMPT_COUNT) +#ifdef CONFIG_DRM_I915_DEBUG # define _WAIT_FOR_ATOMIC_CHECK(ATOMIC) WARN_ON_ONCE((ATOMIC) && !in_atomic()) #else # define _WAIT_FOR_ATOMIC_CHECK(ATOMIC) do { } while (0) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn
On Fri, Oct 09, 2020 at 12:34:21PM +0200, Mauro Carvalho Chehab wrote: > Hi, > > Em Fri, 9 Oct 2020 09:59:26 +0200 > Daniel Vetter escreveu: > > > Way back it was a reasonable assumptions that iomem mappings never > > change the pfn range they point at. But this has changed: > > > > - gpu drivers dynamically manage their memory nowadays, invalidating > > ptes with unmap_mapping_range when buffers get moved > > > > - contiguous dma allocations have moved from dedicated carvetouts to > > cma regions. This means if we miss the unmap the pfn might contain > > pagecache or anon memory (well anything allocated with GFP_MOVEABLE) > > > > - even /dev/mem now invalidates mappings when the kernel requests that > > iomem region when CONFIG_IO_STRICT_DEVMEM is set, see 3234ac664a87 > > ("/dev/mem: Revoke mappings when a driver claims the region") > > > > Accessing pfns obtained from ptes without holding all the locks is > > therefore no longer a good idea. > > > > Unfortunately there's some users where this is not fixable (like v4l > > userptr of iomem mappings) or involves a pile of work (vfio type1 > > iommu). For now annotate these as unsafe and splat appropriately. > > > > This patch adds an unsafe_follow_pfn, which later patches will then > > roll out to all appropriate places. > > NACK, as this breaks an existing userspace API on media. It doesn't break it. You get a big warning the thing is broken and it keeps working. We can't leave such a huge security hole open - it impacts other subsystems, distros need to be able to run in a secure mode. > While I agree that using the userptr on media is something that > new drivers may not support, as DMABUF is a better way of > handling it, changing this for existing ones is a big no, > as it may break usersapace. media community needs to work to fix this, not pretend it is OK to keep going as-is. Dealing with security issues is the one case where an uABI break might be acceptable. If you want to NAK it then you need to come up with the work to do something here correctly that will support the old drivers without the kernel taint. Unfortunately making things uncomfortable for the subsystem is the big hammer the core kernel needs to use to actually get this security work done by those responsible. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC PKS/PMEM 48/58] drivers/md: Utilize new kmap_thread()
On 2020/10/10 03:50, ira.we...@intel.com wrote: > From: Ira Weiny > > These kmap() calls are localized to a single thread. To avoid the over > head of global PKRS updates use the new kmap_thread() call. > Hi Ira, There were a number of options considered. 1) Attempt to change all the thread local kmap() calls to kmap_atomic() 2) Introduce a flags parameter to kmap() to indicate if the mapping should be global or not 3) Change ~20-30 call sites to 'kmap_global()' to indicate that they require a global mapping of the pages 4) Change ~209 call sites to 'kmap_thread()' to indicate that the mapping is to be used within that thread of execution only I copied the above information from patch 00/58 to this message. The idea behind kmap_thread() is fine to me, but as you said the new api is very easy to be missed in new code (even for me). I would like to be supportive to option 2) introduce a flag to kmap(), then we won't forget the new thread-localized kmap method, and people won't ask why a _thread() function is called but no kthread created. Thanks. Coly Li > Cc: Coly Li (maintainer:BCACHE (BLOCK LAYER CACHE)) > Cc: Kent Overstreet (maintainer:BCACHE (BLOCK > LAYER CACHE)) > Signed-off-by: Ira Weiny > --- > drivers/md/bcache/request.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c > index c7cadaafa947..a4571f6d09dd 100644 > --- a/drivers/md/bcache/request.c > +++ b/drivers/md/bcache/request.c > @@ -44,10 +44,10 @@ static void bio_csum(struct bio *bio, struct bkey *k) > uint64_t csum = 0; > > bio_for_each_segment(bv, bio, iter) { > - void *d = kmap(bv.bv_page) + bv.bv_offset; > + void *d = kmap_thread(bv.bv_page) + bv.bv_offset; > > csum = bch_crc64_update(csum, d, bv.bv_len); > - kunmap(bv.bv_page); > + kunmap_thread(bv.bv_page); > } > > k->ptr[KEY_PTRS(k)] = csum & (~0ULL >> 1); > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 14/17] resource: Move devmem revoke code to resource framework
On Fri, Oct 09, 2020 at 09:59:31AM +0200, Daniel Vetter wrote: > +struct address_space *iomem_get_mapping(void) > +{ > + return iomem_inode->i_mapping; This should pair an acquire with the release below > + /* > + * Publish /dev/mem initialized. > + * Pairs with smp_load_acquire() in revoke_iomem(). > + */ > + smp_store_release(_inode, inode); However, this seems abnormal, initcalls rarely do this kind of stuff with global data.. The kernel crashes if this fs_initcall is raced with iomem_get_mapping() due to the unconditional dereference, so I think it can be safely switched to a simple assignment. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn
On Fri, Oct 09, 2020 at 07:52:05PM +0200, Daniel Vetter wrote: > > > If this is the case, the proper fix seems to have a GFP_NOT_MOVABLE > > > flag that it would be denying the core mm code to set __GFP_MOVABLE. > > > > We can't tell from the VMA these kinds of details.. > > > > It has to go the other direction, evey mmap that might be used as a > > userptr here has to be found and the VMA specially created to allow > > its use. At least that is a kernel only change, but will need people > > with the HW to do this work. > > I think the only reasonable way to keep this working is: > - add a struct dma_buf *vma_tryget_dma_buf(struct vm_area_struct *vma); > - add dma-buf export support to fbdev and v4l > - roll this out everywhere we still need it. It seems to me there is a technical way forward to restore user compat, so it is really no different than RDMA/DRM pain we both suffered before. Thus no justification to NAK it. If media wants things to keep working they have to do the technical path like you outline above. > Realistically this just isn't going to happen. If your series goes ahead it will get solved. Someone will take on the huge project to either add DMA buf to the drivers people still care about, or do the work above to transparently handle in kernel. If we allow things to keep working without consequence then nobody will do it. The only reason we did the 4 years of work in RDMA was because Linus went in and broke the uABI for a security fix. It was hundreds of patches to fix it, so I don't have much sympathy for "it is too hard" here. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC PKS/PMEM 22/58] fs/f2fs: Utilize new kmap_thread()
On Fri, Oct 09, 2020 at 02:34:34PM -0700, Eric Biggers wrote: > On Fri, Oct 09, 2020 at 12:49:57PM -0700, ira.we...@intel.com wrote: > > The kmap() calls in this FS are localized to a single thread. To avoid > > the over head of global PKRS updates use the new kmap_thread() call. > > > > @@ -2410,12 +2410,12 @@ static inline struct page *f2fs_pagecache_get_page( > > > > static inline void f2fs_copy_page(struct page *src, struct page *dst) > > { > > - char *src_kaddr = kmap(src); > > - char *dst_kaddr = kmap(dst); > > + char *src_kaddr = kmap_thread(src); > > + char *dst_kaddr = kmap_thread(dst); > > > > memcpy(dst_kaddr, src_kaddr, PAGE_SIZE); > > - kunmap(dst); > > - kunmap(src); > > + kunmap_thread(dst); > > + kunmap_thread(src); > > } > > Wouldn't it make more sense to switch cases like this to kmap_atomic()? > The pages are only mapped to do a memcpy(), then they're immediately unmapped. Maybe you missed the earlier thread from Thomas trying to do something similar for rather different reasons ... https://lore.kernel.org/lkml/20200919091751.06...@linutronix.de/ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/6] mm: mmap: fix fput in error path
On Fri, Oct 09, 2020 at 03:04:20PM -0700, Andrew Morton wrote: > On Fri, 9 Oct 2020 17:03:37 +0200 "Christian König" > wrote: > > > Patch "495c10cc1c0c CHROMIUM: dma-buf: restore args..." > > adds a workaround for a bug in mmap_region. > > > > As the comment states ->mmap() callback can change > > vma->vm_file and so we might call fput() on the wrong file. > > > > Revert the workaround and proper fix this in mmap_region. > > > > Doesn't this patch series address the same thing as > https://lkml.kernel.org/r/20200916090733.31427-1-linmia...@huawei.com? Same basic issue, looks like both of these patches should be combined to plug it fully. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/i915/dpcd_bl: uncheck PWM_PIN_CAP when detect eDP backlight capabilities
BOE panel with ID 2270 claims both PWM_PIN_CAP and AUX_SET_CAP backlight control bits, but default chip backlight failed to control brightness. Check AUX_SET_CAP and proceed to check quirks or VBT backlight type. DPCD can control the brightness of this pannel. Signed-off-by: Aaron Ma --- drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c index acbd7eb66cbe..308b14159b7c 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c @@ -334,8 +334,7 @@ intel_dp_aux_display_control_capable(struct intel_connector *connector) * the panel can support backlight control over the aux channel */ if (intel_dp->edp_dpcd[1] & DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP && - (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP) && - !(intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP)) { + (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP)) { drm_dbg_kms(>drm, "AUX Backlight Control Supported!\n"); return true; } -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 10/17] media/videbuf1|2: Mark follow_pfn usage as unsafe
Em Fri, 9 Oct 2020 09:59:27 +0200 Daniel Vetter escreveu: > The media model assumes that buffers are all preallocated, so that > when a media pipeline is running we never miss a deadline because the > buffers aren't allocated or available. > > This means we cannot fix the v4l follow_pfn usage through > mmu_notifier, without breaking how this all works. The only real fix > is to deprecate userptr support for VM_IO | VM_PFNMAP mappings and > tell everyone to cut over to dma-buf memory sharing for zerocopy. > > userptr for normal memory will keep working as-is. I won't repeat here the discussions for patch 09/17, but just to be clear about this one: NACK. We need a better alternative to avoid breaking existing media applications. > > Signed-off-by: Daniel Vetter > Cc: Jason Gunthorpe > Cc: Kees Cook > Cc: Dan Williams > Cc: Andrew Morton > Cc: John Hubbard > Cc: Jérôme Glisse > Cc: Jan Kara > Cc: Dan Williams > Cc: linux...@kvack.org > Cc: linux-arm-ker...@lists.infradead.org > Cc: linux-samsung-...@vger.kernel.org > Cc: linux-me...@vger.kernel.org > Cc: Pawel Osciak > Cc: Marek Szyprowski > Cc: Kyungmin Park > Cc: Tomasz Figa > Cc: Laurent Dufour > Cc: Vlastimil Babka > Cc: Daniel Jordan > Cc: Michel Lespinasse > --- > drivers/media/common/videobuf2/frame_vector.c | 2 +- > drivers/media/v4l2-core/videobuf-dma-contig.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/common/videobuf2/frame_vector.c > b/drivers/media/common/videobuf2/frame_vector.c > index 2b0b97761d15..a1b85fe9e7c1 100644 > --- a/drivers/media/common/videobuf2/frame_vector.c > +++ b/drivers/media/common/videobuf2/frame_vector.c > @@ -69,7 +69,7 @@ int get_vaddr_frames(unsigned long start, unsigned int > nr_frames, > break; > > while (ret < nr_frames && start + PAGE_SIZE <= vma->vm_end) { > - err = follow_pfn(vma, start, [ret]); > + err = unsafe_follow_pfn(vma, start, [ret]); > if (err) { > if (ret == 0) > ret = err; > diff --git a/drivers/media/v4l2-core/videobuf-dma-contig.c > b/drivers/media/v4l2-core/videobuf-dma-contig.c > index 52312ce2ba05..821c4a76ab96 100644 > --- a/drivers/media/v4l2-core/videobuf-dma-contig.c > +++ b/drivers/media/v4l2-core/videobuf-dma-contig.c > @@ -183,7 +183,7 @@ static int videobuf_dma_contig_user_get(struct > videobuf_dma_contig_memory *mem, > user_address = untagged_baddr; > > while (pages_done < (mem->size >> PAGE_SHIFT)) { > - ret = follow_pfn(vma, user_address, _pfn); > + ret = unsafe_follow_pfn(vma, user_address, _pfn); > if (ret) > break; > Thanks, Mauro ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn
Em Fri, 9 Oct 2020 19:52:05 +0200 Daniel Vetter escreveu: > On Fri, Oct 9, 2020 at 2:48 PM Jason Gunthorpe wrote: > > > > On Fri, Oct 09, 2020 at 02:37:23PM +0200, Mauro Carvalho Chehab wrote: > > > > > I'm not a mm/ expert, but, from what I understood from Daniel's patch > > > description is that this is unsafe *only if* __GFP_MOVABLE is used. > > > > No, it is unconditionally unsafe. The CMA movable mappings are > > specific VMAs that will have bad issues here, but there are other > > types too. I didn't check the mm dirty details, but I strongly suspect that the mm code has a way to prevent moving a mmapped page while it is still in usage. If not, then the entire movable pages concept sounds broken to me, and has to be fixed at mm subsystem. > > > > The only way to do something at a VMA level is to have a list of OK > > VMAs, eg because they were creatd via a special mmap helper from the > > media subsystem. I'm not sure if I'm following you on that. The media API can work with different ways of sending buffers to userspace: - read(); - using the overlay mode. This interface is deprecated in practice, being replaced by DMABUF. Only a few older hardware supports it, and it depends on an special X11 helper driver for it to work. - via DMABUF: https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/dmabuf.html - via mmap, using a mmap helper: https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/mmap.html - via mmap, using userspace-provided pointers: https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/userp.html The existing open-source programs usually chose one or more of the above modes. if the Kernel driver returns -EINVAL when an mmapped streaming I/O mode is not supported, userspace has to select a different method. Most userspace open source programs have fallback support: if one mmap I/O method fails, it selects another one, although this is not a mandatory requirement. I found (and fixed) a few ones that were relying exclusively on userptr support, but I didn't make a comprehensive check. Also there are a number of relevant closed-source apps that we have no idea about what methods they use, like Skype, and other similar videoconferencing programs. Breaking support for those, specially at a time where people are relying on it in order to participate on conferences and doing internal meetings is a **very bad** idea. So, whatever solution is taken, it should not be dumping warning messages at the system and tainting the Kernel, but, instead, checking if the userspace request is valid or not. If it is invalid, return the proper error code via the right V4L2 ioctl, in order to let userspace choose a different method. I the request is valid, refcount the pages for them to not be moved while they're still in usage. - Let me provide some background about how things work at the media subsytem. If you want to know more, the userspace-provided memory mapped pointers work is described here: https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/userp.html#userp Basically, userspace calls either one of those ioctls: VIDIOC_CREATE_BUFS: https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/vidioc-create-bufs.html Which is translated into a videobuf2 call to: vb2_ioctl_create_bufs() VIDIOC_REQBUFS https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/vidioc-reqbufs.html#vidioc-reqbufs Which is translated into a videobuf2 call to: vb2_ioctl_reqbufs() Both internally calls vb2_verify_memory_type(), which is responsible for checking if the provided pointers are OK for the usage and/or do all necessary page allocations, and taking care of any special requirements. This could easily have some additional checks to verify if the requested VMA address has pages that are movable or not, ensuring that ensure that the VMA is OK, and locking them in order to prevent the mm code to move such pages while they are in usage by the media subsystem. Now, as I said before, I don't know the dirty details about how to lock those pages at the mm subsystem in order to avoid it to move the used pages. Yet, when vb2_create_framevec() is called, the check if VMA is OK should already be happened at vb2_verify_memory_type(). - It should be noticed that the dirty hack added by patch 09/17 and 10/17 affects *all* types of memory allocations at V4L2, as this kAPI is called by the 3 different memory models supported at the media subsystem: drivers/media/common/videobuf2/videobuf2-vmalloc.c drivers/media/common/videobuf2/videobuf2-dma-contig.c drivers/media/common/videobuf2/videobuf2-dma-sg.c In other words, with this code: int unsafe_follow_pfn(struct vm_area_struct *vma, unsigned long address, unsigned long
Re: [PATCH v8 3/4] drm/kmb: Mipi DSI part of the display driver
On Fri, Oct 02, 2020 at 07:17:01PM -0700, Anitha Chrisanthus wrote: > Initializes Mipi DSI and sets up connects to ADV bridge > > v2: removed license text > upclassed dev_private, removed HAVE_IRQ. (Sam) > > v3: Squashed all 59 commits to one > > v4: review changes from Sam Ravnborg > renamed dev_p to kmb > > v5: corrected spellings > v6: corrected checkpatch warnings > v7: review changes Sam Ravnborg and Thomas Zimmerman > removed unnecessary logs and defines and ifdef codes (Sam) > split dphy_init_sequence smaller (Sam) > removed redundant checks in kmb_dsi (Sam) > changed kmb_dsi_init to drm_bridge_connector_init and > drm_connector_attach_encoder to bridge's connector (Sam) > v8: call drm_bridge_attach with DRM_BRIDGE_ATTACH_NO_CONNECTOR > > Cc: Sam Ravnborg > Signed-off-by: Anitha Chrisanthus > Reviewed-by: Bob Paauwe > --- > drivers/gpu/drm/kmb/kmb_dsi.c | 1524 > + > drivers/gpu/drm/kmb/kmb_dsi.h | 350 ++ > 2 files changed, 1874 insertions(+) > create mode 100644 drivers/gpu/drm/kmb/kmb_dsi.c > create mode 100644 drivers/gpu/drm/kmb/kmb_dsi.h > > diff --git a/drivers/gpu/drm/kmb/kmb_dsi.c b/drivers/gpu/drm/kmb/kmb_dsi.c > new file mode 100644 > index 000..849f1cc > --- /dev/null > +++ b/drivers/gpu/drm/kmb/kmb_dsi.c > @@ -0,0 +1,1524 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright © 2019-2020 Intel Corporation > + */ > + > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "kmb_drv.h" > +#include "kmb_dsi.h" > +#include "kmb_regs.h" > + > +static struct mipi_dsi_host *dsi_host; > +static struct mipi_dsi_device *dsi_device; > + > +/* Default setting is 1080p, 4 lanes */ > +#define IMG_HEIGHT_LINES 1080 > +#define IMG_WIDTH_PX 1920 > +#define MIPI_TX_ACTIVE_LANES 4 > + > +struct mipi_tx_frame_section_cfg mipi_tx_frame0_sect_cfg = { > + .width_pixels = IMG_WIDTH_PX, > + .height_lines = IMG_HEIGHT_LINES, > + .data_type = DSI_LP_DT_PPS_RGB888_24B, > + .data_mode = MIPI_DATA_MODE1, > + .dma_packed = 0 > +}; > + > +struct mipi_tx_frame_cfg mipitx_frame0_cfg = { > + .sections[0] = _tx_frame0_sect_cfg, > + .sections[1] = NULL, > + .sections[2] = NULL, > + .sections[3] = NULL, > + .vsync_width = 5, > + .v_backporch = 36, > + .v_frontporch = 4, > + .hsync_width = 44, > + .h_backporch = 148, > + .h_frontporch = 88 > +}; > + > +struct mipi_tx_dsi_cfg mipitx_dsi_cfg = { > + .hfp_blank_en = 0, > + .eotp_en = 0, > + .lpm_last_vfp_line = 0, > + .lpm_first_vsa_line = 0, > + .sync_pulse_eventn = DSI_VIDEO_MODE_NO_BURST_EVENT, > + .hfp_blanking = SEND_BLANK_PACKET, > + .hbp_blanking = SEND_BLANK_PACKET, > + .hsa_blanking = SEND_BLANK_PACKET, > + .v_blanking = SEND_BLANK_PACKET, > +}; > + > +struct mipi_ctrl_cfg mipi_tx_init_cfg = { > + .active_lanes = MIPI_TX_ACTIVE_LANES, > + .lane_rate_mbps = MIPI_TX_LANE_DATA_RATE_MBPS, > + .ref_clk_khz = MIPI_TX_REF_CLK_KHZ, > + .cfg_clk_khz = MIPI_TX_CFG_CLK_KHZ, > + .tx_ctrl_cfg = { > + .frames[0] = _frame0_cfg, > + .frames[1] = NULL, > + .frames[2] = NULL, > + .frames[3] = NULL, > + .tx_dsi_cfg = _dsi_cfg, > + .line_sync_pkt_en = 0, > + .line_counter_active = 0, > + .frame_counter_active = 0, > + .tx_always_use_hact = 1, > + .tx_hact_wait_stop = 1, > + } > +}; > + > +struct mipi_hs_freq_range_cfg { > + u16 default_bit_rate_mbps; > + u8 hsfreqrange_code; > +}; > + > +struct vco_params { > + u32 freq; > + u32 range; > + u32 divider; > +}; > + > +static struct vco_params vco_table[] = { > + {52, 0x3f, 8}, > + {80, 0x39, 8}, > + {105, 0x2f, 4}, > + {160, 0x29, 4}, > + {210, 0x1f, 2}, > + {320, 0x19, 2}, > + {420, 0x0f, 1}, > + {630, 0x09, 1}, > + {1100, 0x03, 1}, > + {0x, 0x01, 1}, > +}; > + > +static struct mipi_hs_freq_range_cfg > +mipi_hs_freq_range[MIPI_DPHY_DEFAULT_BIT_RATES] = { > + {.default_bit_rate_mbps = 80, .hsfreqrange_code = 0x00}, > + {.default_bit_rate_mbps = 90, .hsfreqrange_code = 0x10}, > + {.default_bit_rate_mbps = 100, .hsfreqrange_code = 0x20}, > + {.default_bit_rate_mbps = 110, .hsfreqrange_code = 0x30}, > + {.default_bit_rate_mbps = 120, .hsfreqrange_code = 0x01}, > + {.default_bit_rate_mbps = 130, .hsfreqrange_code = 0x11}, > + {.default_bit_rate_mbps = 140, .hsfreqrange_code = 0x21}, > + {.default_bit_rate_mbps = 150, .hsfreqrange_code = 0x31}, > + {.default_bit_rate_mbps = 160, .hsfreqrange_code = 0x02}, > + {.default_bit_rate_mbps = 170,
Re: [PATCH v1 1/4] drm: Add Keem Bay VPU codec DRM
Hello everyone, (Adding some Hantro developers) On Fri, 9 Oct 2020 at 19:15, Daniel Vetter wrote: > > On Fri, Oct 09, 2020 at 07:57:52PM +0800, kuhanh.murugasen.krish...@intel.com > wrote: > > From: "Murugasen Krishnan, Kuhanh" > > > > This is a new DRM media codec driver for Intel's Keem Bay SOC which > > integrates the Verisilicon's Hantro Video Processor Unit (VPU) IP. > > The SoC couples an ARM Cortex A53 CPU with an Intel Movidius VPU. > > > > Hantro VPU IP is a series of video decoder and encoder semiconductor IP > > cores, > > which can be flexibly configured for video surveillance, multimedia consumer > > products, Internet of Things, cloud service products, data centers, aerial > > photography and recorders, thereby providing video transcoding and > > multi-channel > > HD video encoding and decoding. > > > > Hantro VPU IP consists of Hantro VC8000D for decoder and Hantro VC8000E for > > encoder. > > Before you guys even start reviewing or discussing this: good news everyone! Verisilicon Hantro VPU support is in mainline since a few releases now. How about you run a quick "git grep hantro -- drivers/" and see for yourself :-) ? Spoiler alert: we currently support G1 core, supporting MPEG-2, H.264, VP8 and some post-processor features. We are working on G2 for HEVC and VP9, and we have patches ready for VC8000D for H264. Given the VPU is stateless, it requires quite a bit of work on the application side. There are implementations in GStreamer (see v4l2codecs plugin), Chromium, and Ffmpeg. Given all the stateless codec drivers depend on the stateless controls APIs, and given this API is still marked as experimental/unstable, the drivers are in staging. Other than this, these drivers are just as good as any, and have been shipped for quite some time now. I expect to move them out of staging soon, just as soon as we clean and stabilize this control API. I will be happy to review patches adding Keem Bay support, to be honest, unsure what that implies, but we'll see. Thanks, Ezequiel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 2/2] drm/mediatek: dsi: fix scrolling of panel with small hfp or hbp
Replace horizontal_backporch_byte with vm->hback_porch * bpp to aovid flowing judgement negative number. if ((vm->hfront_porch * dsi_tmp_buf_bpp + horizontal_backporch_byte) > data_phy_cycles * dsi->lanes + delta) Signed-off-by: Jitao Shi --- drivers/gpu/drm/mediatek/mtk_dsi.c | 54 ++ 1 file changed, 19 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 16fd99dcdacf..f69ebeaf 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -445,6 +445,7 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi) u32 horizontal_backporch_byte; u32 horizontal_frontporch_byte; u32 dsi_tmp_buf_bpp, data_phy_cycles; + u32 delta; struct mtk_phy_timing *timing = >phy_timing; struct videomode *vm = >vm; @@ -475,42 +476,25 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi) data_phy_cycles = timing->lpx + timing->da_hs_prepare + timing->da_hs_zero + timing->da_hs_exit + 3; - if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) { - if ((vm->hfront_porch + vm->hback_porch) * dsi_tmp_buf_bpp > - data_phy_cycles * dsi->lanes + 18) { - horizontal_frontporch_byte = - vm->hfront_porch * dsi_tmp_buf_bpp - - (data_phy_cycles * dsi->lanes + 18) * - vm->hfront_porch / - (vm->hfront_porch + vm->hback_porch); - - horizontal_backporch_byte = - horizontal_backporch_byte - - (data_phy_cycles * dsi->lanes + 18) * - vm->hback_porch / - (vm->hfront_porch + vm->hback_porch); - } else { - DRM_WARN("HFP less than d-phy, FPS will under 60Hz\n"); - horizontal_frontporch_byte = vm->hfront_porch * -dsi_tmp_buf_bpp; - } + delta = (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) ? 18 : 12; + + if ((vm->hfront_porch * dsi_tmp_buf_bpp + horizontal_backporch_byte) > + data_phy_cycles * dsi->lanes + delta) { + horizontal_frontporch_byte = + vm->hfront_porch * dsi_tmp_buf_bpp - + (data_phy_cycles * dsi->lanes + delta) * + vm->hfront_porch / + (vm->hfront_porch + vm->hback_porch); + + horizontal_backporch_byte = + horizontal_backporch_byte - + (data_phy_cycles * dsi->lanes + delta) * + vm->hback_porch / + (vm->hfront_porch + vm->hback_porch); } else { - if ((vm->hfront_porch + vm->hback_porch) * dsi_tmp_buf_bpp > - data_phy_cycles * dsi->lanes + 12) { - horizontal_frontporch_byte = - vm->hfront_porch * dsi_tmp_buf_bpp - - (data_phy_cycles * dsi->lanes + 12) * - vm->hfront_porch / - (vm->hfront_porch + vm->hback_porch); - horizontal_backporch_byte = horizontal_backporch_byte - - (data_phy_cycles * dsi->lanes + 12) * - vm->hback_porch / - (vm->hfront_porch + vm->hback_porch); - } else { - DRM_WARN("HFP less than d-phy, FPS will under 60Hz\n"); - horizontal_frontporch_byte = vm->hfront_porch * -dsi_tmp_buf_bpp; - } + DRM_WARN("HFP + HBP less than d-phy, FPS will under 60Hz\n"); + horizontal_frontporch_byte = vm->hfront_porch * +dsi_tmp_buf_bpp; } writel(horizontal_sync_active_byte, dsi->regs + DSI_HSA_WC); -- 2.12.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 1/2] Revert "drm/mediatek: dsi: Fix scrolling of panel with small hfp or hbp"
This reverts commit 35bf948f1edbf507f6e57e0879fa6ea36d2d2930. Signed-off-by: Jitao Shi --- drivers/gpu/drm/mediatek/mtk_dsi.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 80b7a082e874..16fd99dcdacf 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -466,13 +466,14 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi) horizontal_sync_active_byte = (vm->hsync_len * dsi_tmp_buf_bpp - 10); if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE) - horizontal_backporch_byte = vm->hback_porch * dsi_tmp_buf_bpp; + horizontal_backporch_byte = + (vm->hback_porch * dsi_tmp_buf_bpp - 10); else - horizontal_backporch_byte = (vm->hback_porch + vm->hsync_len) * - dsi_tmp_buf_bpp; + horizontal_backporch_byte = ((vm->hback_porch + vm->hsync_len) * + dsi_tmp_buf_bpp - 10); data_phy_cycles = timing->lpx + timing->da_hs_prepare + - timing->da_hs_zero + timing->da_hs_exit; + timing->da_hs_zero + timing->da_hs_exit + 3; if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) { if ((vm->hfront_porch + vm->hback_porch) * dsi_tmp_buf_bpp > -- 2.12.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[v4 PATCH 0/2] fix scrolling of panel with small hfp or hbp
Changes since v3: - Revert v2, for v2 will cause some bridge ic no output. the cause the video linetime doesn't match display mode from get mode. - Make sure the horizontal_frontporch_byte and horizontal_backporch_byte are > 0. Jitao Shi (2): Revert "drm/mediatek: dsi: Fix scrolling of panel with small hfp or hbp" drm/mediatek: dsi: fix scrolling of panel with small hfp or hbp drivers/gpu/drm/mediatek/mtk_dsi.c | 65 +++--- 1 file changed, 25 insertions(+), 40 deletions(-) -- 2.12.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel