On Wed, 2009-08-05 at 11:18 +0200, Thomas Hellström wrote:
> Michel Dänzer wrote:
> > On Wed, 2009-08-05 at 11:01 +0200, Thomas Hellström wrote:
> >
> >> Michel Dänzer wrote:
> >>
> >>> On Wed, 2009-08-05 at 10:50 +0200, Thomas Hellström wrote:
> >>>
> >>>
> >>>> Michel Dänzer wrote:
> >>>>
> >>>>
> >>>>> From: Michel Dänzer <daen...@vmware.com>
> >>>>>
> >>>>> Make sure bo->vm_node is valid, to prevent crashes in the fault
> >>>>> handler, and
> >>>>> adjust vma->vm_pgoff according to it, so userspace attempts to access
> >>>>> /dev/fb*
> >>>>> mappings don't always result in SIGBUS.
> >>>>>
> >>>>> Signed-off-by: Michel Dänzer <daen...@vmware.com>
> >>>>> ---
> >>>>> drivers/gpu/drm/ttm/ttm_bo_vm.c | 6 ++++++
> >>>>> 1 files changed, 6 insertions(+), 0 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c
> >>>>> b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> >>>>> index 27b146c..57d0c94 100644
> >>>>> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
> >>>>> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> >>>>> @@ -282,7 +282,13 @@ int ttm_fbdev_mmap(struct vm_area_struct *vma,
> >>>>> struct ttm_buffer_object *bo)
> >>>>> if (vma->vm_pgoff != 0)
> >>>>> return -EACCES;
> >>>>>
> >>>>> + if (!bo->vm_node) {
> >>>>> + printk(KERN_ERR TTM_PFX "bo->vm_node == NULL\n");
> >>>>> + return -EACCES;
> >>>>> + }
> >>>>>
> >>>>>
> >>>>>
> >>>> Michel,
> >>>> Did you actually hit a bo with an invalid vm_node?
> >>>>
> >>>>
> >>> See patch 3 - the radeon driver used to create the fbcon BO as
> >>> kernel-only, so when I tried passing that to ttm_fbdev_mmap()
> >>> all /dev/fb* userspace mappings mysteriously caught SIGBUS on access.
> >>>
> >
> > Actually, it was even worse - the TTM fault handler crashed because it
> > unconditionally dereferences bo->vm_node.
> >
> >
> >>> It took me a while to track that down, this is basically a debugging aid
> >>> for bugs like that.
> >>>
> >>>
> >> Ah, OK. But since that is really a kernel bug, we should trap it with
> >> BUG_ON().
> >>
> >
> > Good point, will do.
> >
> >
> >
> Aargh. Wait. I remember now.
>
> The fbcon bo is exported through the fbdev address space at offset 0.
> The vm_node is for the drm device address space only. So it is perfectly
> legal and actually correct for it not to have a vm_node, unless it's
> going to be accessible from the drm device. Does it need to for KMS?
I don't think so.
> I'm a bit unsure whether it's OK to export a bo through two different
> address spaces. In particular, unmap_mapping_range() on the drm device
> will not kill the fbdev user-space mappings.
Hmm, so that would mean that if an fbdev mapping is created while the BO
is in VRAM, it would still access VRAM after the BO has been evicted? Is
there a solution for this?
> The bug is in the ttm fbdev fault handler which is the same as the drm
> device fault handler that uses the vm_node->offset when it should
> actually use 0. We should probably add an is_fbdev argument to the fault
> handler and wrap it for its two uses.
Or maybe it could just check if vm_node is non-NULL and use 0 otherwise.
I'll defer to you which is better.
--
Earthling Michel Dänzer | http://www.vmware.com
Libre software enthusiast | Debian, X and DRI developer
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
