Re: [PATCH v4] DRM: add DRM Driver for Samsung SoC EXYNOS4210.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 09/10/2011 07:31 PM, Rob Clark wrote:
On Sat, Sep 10, 2011 at 9:04 AM, Thomas Hellstrom<thomas@xxxxxxxxxxxx>  wrote:
On 09/09/2011 01:38 PM, Inki Dae wrote:
This patch is a DRM Driver for Samsung SoC Exynos4210 and now enables
only FIMD yet but we will add HDMI support also in the future.

from now on, I will remove RFC prefix because I think we have got comments
enough.

this patch is based on git repository below:
git://git.kernel.org/pub/scm/linux/kernel/git/airlied/drm-2.6.git,
branch name: drm-next
commit-id: bcc65fd8e929a9d9d34d814d6efc1d2793546922

you can refer to our working repository below:
http://git.infradead.org/users/kmpark/linux-2.6-samsung
branch name: samsung-drm

We tried to re-use lowlevel codes of the FIMD driver(s3c-fb.c
based on Linux framebuffer) but couldn't so because lowlevel codes
of s3c-fb.c are included internally and so FIMD module of this driver has
its own lowlevel codes.

We used GEM framework for buffer management and DMA APIs(dma_alloc_*)
for buffer allocation. by using DMA API, we could use CMA later.

Refer to this link for CMA(Continuous Memory Allocator):
http://lkml.org/lkml/2011/7/20/45

this driver supports only physically continuous memory(non-iommu).

Links to previous versions of the patchset:
v1:<    https://lwn.net/Articles/454380/>
v2:<    http://www.spinics.net/lists/kernel/msg1224275.html>
v3:<    http://www.gossamer-threads.com/lists/linux/kernel/1423684>

Changelog v2:
DRM: add DRM_IOCTL_SAMSUNG_GEM_MMAP ioctl command.

     this feature maps user address space to physical memory region
     once user application requests DRM_IOCTL_SAMSUNG_GEM_MMAP ioctl.

DRM: code clean and add exception codes.

Changelog v3:
DRM: Support multiple irq.

     FIMD and HDMI have their own irq handler but DRM Framework can regiter
     only one irq handler this patch supports mutiple irq for Samsung SoC.

DRM: Consider modularization.

     each DRM, FIMD could be built as a module.

DRM: Have indenpendent crtc object.

     crtc isn't specific to SoC Platform so this patch gets a crtc
     to be used as common object.
     created crtc could be attached to any encoder object.

DRM: code clean and add exception codes.

Changelog v4:
DRM: remove is_defult from samsung_fb.

     is_default isn't used for default framebuffer.

DRM: code refactoring to fimd module.
     this patch is be considered with multiple display objects and
     would use its own request_irq() to register a irq handler instead of
     drm framework's one.

DRM: remove find_samsung_drm_gem_object()

DRM: move kernel private data structures and definitions to driver folder.

     samsung_drm.h would contain only public information for userspace
     ioctl interface.

DRM: code refactoring to gem modules.
     buffer module isn't dependent of gem module anymore.

DRM: fixed security issue.

DRM: remove encoder porinter from specific connector.

     samsung connector doesn't need to have generic encoder.

DRM: code clean and add exception codes.

Signed-off-by: Inki Dae<inki.dae@xxxxxxxxxxx>
Signed-off-by: Joonyoung Shim<jy0922.shim@xxxxxxxxxxx>
Signed-off-by: SeungWoo Kim<sw0312.kim@xxxxxxxxxxx>
Signed-off-by: kyungmin.park<kyungmin.park@xxxxxxxxxxx>
---

+static struct drm_ioctl_desc samsung_ioctls[] = {
+       DRM_IOCTL_DEF_DRV(SAMSUNG_GEM_CREATE,
samsung_drm_gem_create_ioctl,
+                       DRM_UNLOCKED | DRM_AUTH),

Hi!

With reference my previous security comment.

Let's say you have a compromised video player running as a DRM client, that
tries to repeatedly allocate huge GEM buffers...

What will happen when all DMA memory is exhausted? Will this cause other
device drivers to see an OOM, or only DRM?

The old DRI model basically allowed any authorized DRI client to exhaust
video ram or AGP memory, but never system memory. Newer DRI drivers
typically only allow DRI masters to do that.
as

I don't think an authorized DRI client should be able to easily exhaust
resources (DMA memory) used by other device drivers causing them to fail.
I'm not entirely sure what else can be done, other than have a
threshold on max MB allocatable of buffer memory..

Yes, I think that's what needs to be done, and that threshold should be low enough to keep other device drivers running in the worst allocation case.

In the samsung driver case, he is only allocating scanout memory from
CMA, so the limit will be the CMA region size.. beyond that you can't
get physically contiguous memory.  So I think this driver is safe.

It's not really what well-behaved user-space drivers do that should be a concern, but what compromized application *might* do that is a concern.

In the omap driver case, I'm also allowing allocation of shmem backed
buffers.  So perhaps I need a threshold of how many buffers can have
pages attached.  Offhand, I'd have to think about how to handle that..
  have any of the other UMA gfx drivers dealt with this yet?

I think shmem pages are more well behaved since they are allocated out of the page cache. When the page cache is exhausted, the kernel resorts to shrinkers to claim more memory, but the structures backing the shmem buffers use pinned memory and may exhaust available kmalloc memory, unless, as you say, their number is somehow limited. I'm not sure whether the shmem subsystem actually does this already...

I don't know how / if the Intel driver implements this. The ttm-aware drivers does some simple accounting of system memory used for graphics purposes, and limits it to a preset percentage of available system memory, and that percentage is adjustable using a sysfs entry. This requires, however, that the drivers register all long-lived data structures created as a result of a user-space call with the accounting system.

/Thomas


BR,
-R





/Thomas


+       DRM_IOCTL_DEF_DRV(SAMSUNG_GEM_MAP_OFFSET,
+                       samsung_drm_gem_map_offset_ioctl, DRM_UNLOCKED |
+                               DRM_AUTH),
+       DRM_IOCTL_DEF_DRV(SAMSUNG_GEM_MMAP,
+                       samsung_drm_gem_mmap_ioctl, DRM_UNLOCKED |
DRM_AUTH),
+};




_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel




_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux