brucech...@via.com.tw wrote:
Bruce, Others.
The v4l interfaces look ok to me.
However, the double-buffer flush mechanism does not.
If I understand the code correctly, the user-space application prepares
command buffers directly in AGP, and asks the
drm module to submit them. We can't allow this for security reasons.
The user-space application could for example fill the buffer with
commands to texture from arbitrary system memory, getting hold of other
user's private data.
The whole ring-buffer stuff and the command verifier was once
implemented to fix that security problem.
I mentioned this in the last email I posted as a comment to the big
patch, and detailed the unichrome drm module security policy.
So this patch is a NAK from my side.
/Thomas
> Hello All:
> This is patch 2 of the 5 patches. It's based on 2.6.32-rc3+patch1 and for
> the support of
> video command flush(using double buffer to improve performance) and
> interfaces for
> communication with v4l kernel module.
>
> 5 files has been modified for this patch.
>
> Sign-off by Bruce C. Chang <brucech...@via.com.tw>
> diff -Nur linux-2.6.32-rc3-old/drivers/gpu/drm/via/via_dma.c
> linux-2.6.32-rc3-new/drivers/gpu/drm/via/via_dma.c
> --- linux-2.6.32-rc3-old/drivers/gpu/drm/via/via_dma.c 2009-10-08
> 10:14:16.000000000 +0800
> +++ linux-2.6.32-rc3-new/drivers/gpu/drm/via/via_dma.c 2009-10-08
> 10:14:29.000000000 +0800
> @@ -68,6 +68,13 @@
> *vb++ = (w2); \
> dev_priv->dma_low += 8;
>
> +#define VIA_OUT_VIDEO_AGP_BUFFER(cmd1, cmd2) \
> + do { \
> + *cur_virtual++ = cmd1; \
> + *cur_virtual++ = cmd2; \
> + cmdbuf_info.cmd_size += 8; \
> + } while (0);
> +
> static void via_cmdbuf_flush(struct drm_via_private *dev_priv,
> uint32_t cmd_type);
> static void via_cmdbuf_start(drm_via_private_t * dev_priv);
> @@ -158,6 +165,7 @@
> int via_dma_cleanup(struct drm_device * dev)
> {
> struct drm_via_video_save_head *pnode;
> + int i;
>
> for (pnode = via_video_save_head; pnode; pnode =
> (struct drm_via_video_save_head *)pnode->next)
> @@ -167,12 +175,23 @@
> (drm_via_private_t *) dev->dev_private;
>
> if (dev_priv->ring.virtual_start) {
> - via_cmdbuf_reset(dev_priv);
> + if (dev_priv->cr_status == CR_FOR_RINGBUFFER)
> + via_cmdbuf_flush(dev_priv, HC_HAGPBpID_STOP);
> +
> + via_wait_idle(dev_priv);
>
> drm_core_ioremapfree(&dev_priv->ring.map, dev);
> dev_priv->ring.virtual_start = NULL;
> }
>
> + for (i = 0; i < 3; i++) {
> + if (dev_priv->video_agp_address_map[i].handle &&
> + dev_priv->video_agp_address_map[i].size)
> + drm_core_ioremapfree(dev_priv->
> + video_agp_address_map+i, dev);
> + /*Fix for suspend reuse video buf*/
> + dev_priv->video_agp_address_map[i].handle = NULL;
> + }
> }
>
> return 0;
> @@ -235,6 +254,7 @@
>
> via_cmdbuf_start(dev_priv);
>
> + dev_priv->cr_status = CR_FOR_RINGBUFFER;
> return 0;
> }
>
> @@ -343,12 +363,42 @@
> static int via_cmdbuffer(struct drm_device *dev, void *data, struct drm_file
> *file_priv)
> {
> drm_via_cmdbuffer_t *cmdbuf = data;
> - int ret;
> + drm_via_private_t *dev_priv = dev->dev_private;
> + int ret = 0, count;
>
> LOCK_TEST_WITH_RETURN(dev, file_priv);
>
> DRM_DEBUG("buf %p size %lu\n", cmdbuf->buf, cmdbuf->size);
>
> + if (dev_priv->cr_status == CR_FOR_VIDEO) {
> + /* Because our driver will hook CR stop cmd behind video cmd,
> + * all we need to do here is to wait for CR idle,
> + * and initialize ring buffer.
> + */
> + count = 10000000;
> + while (count-- && (VIA_READ(VIA_REG_STATUS) &
> + VIA_CMD_RGTR_BUSY))
> + cpu_relax();
> + /* Seldom happen */
> + if (count < 0) {
> + DRM_INFO("The CR can't be idle from video agp cmd \
> + dispatch when it is needed by ring buffer \n");
> + return -1;
> + }
> + /* CR has been idle so that we need to initialize ring buffer */
> + dev_priv->dma_ptr = dev_priv->ring.virtual_start;
> + dev_priv->dma_low = 0;
> + dev_priv->dma_high = 0x1000000;
> + dev_priv->dma_wrap = 0x1000000;
> + dev_priv->dma_offset = 0x0;
> + dev_priv->last_pause_ptr = NULL;
> + dev_priv->hw_addr_ptr = dev_priv->mmio->handle + 0x418;
> +
> + via_cmdbuf_start(dev_priv);
> +
> + dev_priv->cr_status = CR_FOR_RINGBUFFER;
> +
> + }
> ret = via_dispatch_cmdbuffer(dev, cmdbuf);
> if (ret) {
> return ret;
> @@ -357,6 +407,135 @@
> return 0;
> }
>
> +/* Dispatch command of video : using double buffer mechanism */
> +int via_cmdbuffer_video_agp(struct drm_device *dev, void *data,
> + struct drm_file *file_priv)
> +{
> + drm_via_private_t *dev_priv = dev->dev_private;
> + struct drm_via_video_agp_cmd cmdbuf_info;
> + int count;
> + u32 start_addr, start_addr_lo;
> + u32 end_addr, end_addr_lo;
> + u32 pause_addr, pause_addr_hi, pause_addr_lo;
> + u32 *cur_virtual;
> + u32 command;
> + int i = 0;
> + struct drm_local_map map;
> +
> + LOCK_TEST_WITH_RETURN(dev, file_priv);
> +
> + /* Check whether CR services for ring buffer or for video engine. */
> + if (dev_priv->cr_status == CR_FOR_RINGBUFFER) {
> + /* Here we need to hook stop cmd in tail of ringbuffer
> + * in order to stop CR, for we will reset start/end/pause
> + * address for fetch cmd from video AGP buffer
> + */
> + via_cmdbuf_flush(dev_priv, HC_HAGPBpID_STOP);
> + }
> +
> + /* Set CR status here to avoid ring buffer crush in case we
> + * can't initialize CR for video properly
> + */
> + dev_priv->cr_status = CR_FOR_VIDEO;
> +
> + /* Wait idle since we will reset CR relevant registers. */
> + count = 10000000;
> + while (count-- && (VIA_READ(VIA_REG_STATUS) & VIA_CMD_RGTR_BUSY))
> + cpu_relax();
> +
> + /* Seldom happen */
> + if (count < 0) {
> + DRM_INFO("The CR can't be idle from video agp cmd dispatch \
> + when it is needed by ring buffer \n");
> + return -1;
> + }
> +
> + /* Till here, the CR has been idle, all need to here is to initialize
> + * CR START/END/PAUSE address registers according to video AGP buffer
> + * location and size. BE LUCKY!!!
> + */
> + cmdbuf_info = *(struct drm_via_video_agp_cmd *)data;
> +
> + start_addr = cmdbuf_info.offset + dev->agp->base;
> + end_addr = cmdbuf_info.buffer_size + start_addr;
> +
> + if ((cmdbuf_info.buffer_size & 0xFF) ||
> + (start_addr + 2 * 0xFF > end_addr) ||
> + start_addr & 0xFF) {
> + DRM_INFO("The video cmd is too large or you didn't set the \
> + video cmd 2 DWORD alignment. \n");
> + return -1;
> + }
> +
> + map.offset = start_addr;
> + map.size = cmdbuf_info.buffer_size;
> + map.type = map.flags = map.mtrr = 0;
> + map.handle = 0;
> +
> + for (i = 0; i < 3; i++) {
> + if ((dev_priv->video_agp_address_map[i].offset == map.offset) &&
> + (dev_priv->video_agp_address_map[i].size == map.size) &&
> + dev_priv->video_agp_address_map[i].handle) {
> + map.handle = dev_priv->video_agp_address_map[i].handle;
> + break;
> + }
> + if (!dev_priv->video_agp_address_map[i].handle)
> + break;
> + }
> +
> + /* Check whether this agp cmd buffer has already been remaped before */
> + /* case: Never be remaped before */
> + if (!map.handle) {
> + drm_core_ioremap(&map, dev);
> + if (!map.handle)
> + return -1;
> + /* there is a free hole for filling in this address map */
> + if (i < 3)
> + dev_priv->video_agp_address_map[i] = map;
> + else {
> + drm_core_ioremapfree(dev_priv->video_agp_address_map,
> + dev);
> + dev_priv->video_agp_address_map[0] = map;
> + }
> + }
> +
> + cur_virtual = map.handle + cmdbuf_info.cmd_size;
> +
> + VIA_OUT_VIDEO_AGP_BUFFER(HC_HEADER2 | ((VIA_REG_TRANSET >> 2) << 12) |
> + (VIA_REG_TRANSPACE >> 2), HC_ParaType_PreCR << 16);
> +
> + /* pause register need 0xFF alignment */
> + do {
> + VIA_OUT_VIDEO_AGP_BUFFER(HC_DUMMY, HC_DUMMY);
> + } while (cmdbuf_info.cmd_size & 0xFF);
> + pause_addr = cmdbuf_info.cmd_size + start_addr - 8;
> +
> + pause_addr_lo = ((HC_SubA_HAGPBpL << 24) | HC_HAGPBpID_STOP |
> + (pause_addr & HC_HAGPBpL_MASK));
> + pause_addr_hi = ((HC_SubA_HAGPBpH << 24) | (pause_addr >> 24));
> + start_addr_lo = ((HC_SubA_HAGPBstL << 24) | (start_addr & 0xFFFFFF));
> + end_addr_lo = ((HC_SubA_HAGPBendL << 24) | (end_addr & 0xFFFFFF));
> + command = ((HC_SubA_HAGPCMNT << 24) | (start_addr >> 24) |
> + ((end_addr & 0xff000000) >> 16));
> + *(cur_virtual-2) = pause_addr_hi;
> + *(cur_virtual-1) = pause_addr_lo;
> +
> + via_flush_write_combine();
> +
> + VIA_WRITE(VIA_REG_TRANSET, (HC_ParaType_PreCR << 16));
> + VIA_WRITE(VIA_REG_TRANSPACE, command);
> + VIA_WRITE(VIA_REG_TRANSPACE, start_addr_lo);
> + VIA_WRITE(VIA_REG_TRANSPACE, end_addr_lo);
> +
> + VIA_WRITE(VIA_REG_TRANSPACE, pause_addr_hi);
> + VIA_WRITE(VIA_REG_TRANSPACE, pause_addr_lo);
> + DRM_WRITEMEMORYBARRIER();
> + /* fire */
> + VIA_WRITE(VIA_REG_TRANSPACE, command | HC_HAGPCMNT_MASK);
> +
> + return 0;
> +}
> +
> static int via_dispatch_pci_cmdbuffer(struct drm_device * dev,
> drm_via_cmdbuffer_t * cmd)
> {
> @@ -904,6 +1083,7 @@
> DRM_IOCTL_DEF(DRM_VIA_FB_INIT, via_fb_init, DRM_AUTH|DRM_MASTER),
> DRM_IOCTL_DEF(DRM_VIA_MAP_INIT, via_map_init, DRM_AUTH|DRM_MASTER),
> DRM_IOCTL_DEF(DRM_VIA_DEC_FUTEX, via_decoder_futex, DRM_AUTH),
> + DRM_IOCTL_DEF(DRM_VIA_GET_INFO, via_get_drm_info, DRM_AUTH),
> DRM_IOCTL_DEF(DRM_VIA_DMA_INIT, via_dma_init, DRM_AUTH),
> DRM_IOCTL_DEF(DRM_VIA_CMDBUFFER, via_cmdbuffer, DRM_AUTH),
> DRM_IOCTL_DEF(DRM_VIA_FLUSH, via_flush_ioctl, DRM_AUTH),
> @@ -912,6 +1092,7 @@
> DRM_IOCTL_DEF(DRM_VIA_WAIT_IRQ, via_wait_irq, DRM_AUTH),
> DRM_IOCTL_DEF(DRM_VIA_DMA_BLIT, via_dma_blit, DRM_AUTH),
> DRM_IOCTL_DEF(DRM_VIA_BLIT_SYNC, via_dma_blit_sync, DRM_AUTH),
> + DRM_IOCTL_DEF(DRM_VIA_FLUSH_VIDEO, via_cmdbuffer_video_agp, 0),
> DRM_IOCTL_DEF(DRM_VIA_INIT_JUDGE, via_drm_init_judge, 0)
> };
>
> diff -Nur linux-2.6.32-rc3-old/drivers/gpu/drm/via/via_drv.h
> linux-2.6.32-rc3-new/drivers/gpu/drm/via/via_drv.h
> --- linux-2.6.32-rc3-old/drivers/gpu/drm/via/via_drv.h 2009-10-08
> 10:14:16.000000000 +0800
> +++ linux-2.6.32-rc3-new/drivers/gpu/drm/via/via_drv.h 2009-10-08
> 10:14:29.000000000 +0800
> @@ -94,6 +94,11 @@
> unsigned long vram_offset;
> unsigned long agp_offset;
> drm_via_blitq_t blit_queues[VIA_NUM_BLIT_ENGINES];
> + struct drm_local_map video_agp_address_map[3];
> + enum {
> + CR_FOR_RINGBUFFER,
> + CR_FOR_VIDEO
> + } cr_status;
> uint32_t dma_diff;
> int initialize;
> } drm_via_private_t;
> @@ -121,6 +126,8 @@
> extern int via_agp_init(struct drm_device *dev, void *data, struct drm_file
> *file_priv);
> extern int via_map_init(struct drm_device *dev, void *data, struct drm_file
> *file_priv);
> extern int via_decoder_futex(struct drm_device *dev, void *data, struct
> drm_file *file_priv);
> +extern int via_get_drm_info(struct drm_device *dev, void *data,
> + struct drm_file *file_priv);
> extern int via_wait_irq(struct drm_device *dev, void *data, struct drm_file
> *file_priv);
> extern int via_dma_blit_sync( struct drm_device *dev, void *data, struct
> drm_file *file_priv );
> extern int via_dma_blit( struct drm_device *dev, void *data, struct drm_file
> *file_priv );
> diff -Nur linux-2.6.32-rc3-old/drivers/gpu/drm/via/via_map.c
> linux-2.6.32-rc3-new/drivers/gpu/drm/via/via_map.c
> --- linux-2.6.32-rc3-old/drivers/gpu/drm/via/via_map.c 2009-10-08
> 10:14:16.000000000 +0800
> +++ linux-2.6.32-rc3-new/drivers/gpu/drm/via/via_map.c 2009-10-08
> 10:14:29.000000000 +0800
> @@ -142,3 +142,21 @@
>
> return 0;
> }
> +
> +/* This function is for via another video decoder driver to get information
> */
> +int via_get_drm_info(struct drm_device *dev, void *data,
> + struct drm_file *file_priv)
> +{
> + drm_via_private_t *dev_priv = (drm_via_private_t *)dev->dev_private;
> + struct drm_via_info *info = data;
> +
> + if (!dev_priv->initialize || !dev->agp_buffer_map)
> + return -EINVAL;
> +
> + info->RegSize = dev_priv->mmio->size;
> + info->AgpSize = dev->agp_buffer_map->size;
> + info->RegHandle = dev_priv->mmio->offset;
> + info->AgpHandle = dev->agp_buffer_map->offset;
> +
> + return 0;
> +}
> diff -Nur linux-2.6.32-rc3-old/drivers/gpu/drm/via/via_mm.c
> linux-2.6.32-rc3-new/drivers/gpu/drm/via/via_mm.c
> --- linux-2.6.32-rc3-old/drivers/gpu/drm/via/via_mm.c 2009-10-08
> 10:14:16.000000000 +0800
> +++ linux-2.6.32-rc3-new/drivers/gpu/drm/via/via_mm.c 2009-10-08
> 10:14:29.000000000 +0800
> @@ -57,6 +57,8 @@
> DRM_DEBUG("offset = %u, size = %u\n", agp->offset, agp->size);
> return 0;
> }
> +static void *global_dev;
> +static void *global_file_priv;
>
> int via_fb_init(struct drm_device *dev, void *data, struct drm_file
> *file_priv)
> {
> @@ -79,6 +81,8 @@
>
> mutex_unlock(&dev->struct_mutex);
> DRM_DEBUG("offset = %u, size = %u\n", fb->offset, fb->size);
> + global_dev = dev;
> + global_file_priv = file_priv;
>
> return 0;
>
> @@ -291,3 +295,28 @@
> mutex_unlock(&dev->struct_mutex);
> return;
> }
> +
> +/* These two function is exported for via V4L module communication */
> +static int via_fb_alloc(drm_via_mem_t *mem)
> +{
> + struct drm_device *dev = global_dev;
> + struct drm_file *file_priv = global_file_priv;
> +
> + if (dev && file_priv)
> + return via_mem_alloc(dev, mem, file_priv);
> + else
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL(via_fb_alloc);
> +
> +static int via_fb_free(drm_via_mem_t *mem)
> +{
> + struct drm_device *dev = global_dev;
> + struct drm_file *file_priv = global_file_priv;
> +
> + if (dev && file_priv)
> + return via_mem_free(dev, mem, file_priv);
> + else
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL(via_fb_free);
> diff -Nur linux-2.6.32-rc3-old/include/drm/via_drm.h
> linux-2.6.32-rc3-new/include/drm/via_drm.h
> --- linux-2.6.32-rc3-old/include/drm/via_drm.h 2009-10-08
> 10:14:16.000000000 +0800
> +++ linux-2.6.32-rc3-new/include/drm/via_drm.h 2009-10-08
> 10:14:29.000000000 +0800
> @@ -53,6 +53,12 @@
> #define VIA_LOG_MIN_TEX_REGION_SIZE 16
> #endif
>
> +struct drm_via_info {
> + unsigned long AgpHandle;
> + unsigned long AgpSize;
> + unsigned long RegHandle;
> + unsigned long RegSize;
> +} ;
> #define VIA_UPLOAD_TEX0IMAGE 0x1 /* handled clientside */
> #define VIA_UPLOAD_TEX1IMAGE 0x2 /* handled clientside */
> #define VIA_UPLOAD_CTX 0x4
> @@ -69,7 +75,7 @@
> #define DRM_VIA_FB_INIT 0x03
> #define DRM_VIA_MAP_INIT 0x04
> #define DRM_VIA_DEC_FUTEX 0x05
> -#define NOT_USED
> +#define DRM_VIA_GET_INFO 0x06
> #define DRM_VIA_DMA_INIT 0x07
> #define DRM_VIA_CMDBUFFER 0x08
> #define DRM_VIA_FLUSH 0x09
> @@ -79,6 +85,7 @@
> #define DRM_VIA_WAIT_IRQ 0x0d
> #define DRM_VIA_DMA_BLIT 0x0e
> #define DRM_VIA_BLIT_SYNC 0x0f
> +#define DRM_VIA_FLUSH_VIDEO 0x12
> #define DRM_VIA_INIT_JUDGE 0x16
>
> #define DRM_IOCTL_VIA_ALLOCMEM DRM_IOWR(DRM_COMMAND_BASE +
> DRM_VIA_ALLOCMEM, drm_via_mem_t)
> @@ -87,6 +94,8 @@
> #define DRM_IOCTL_VIA_FB_INIT DRM_IOWR(DRM_COMMAND_BASE +
> DRM_VIA_FB_INIT, drm_via_fb_t)
> #define DRM_IOCTL_VIA_MAP_INIT DRM_IOWR(DRM_COMMAND_BASE +
> DRM_VIA_MAP_INIT, drm_via_init_t)
> #define DRM_IOCTL_VIA_DEC_FUTEX DRM_IOW( DRM_COMMAND_BASE +
> DRM_VIA_DEC_FUTEX, drm_via_futex_t)
> +#define DRM_IOCTL_VIA_GET_INFO DRM_IOR(DRM_COMMAND_BASE + \
> + DRM_VIA_GET_INFO, struct drm_via_info)
> #define DRM_IOCTL_VIA_DMA_INIT DRM_IOWR(DRM_COMMAND_BASE +
> DRM_VIA_DMA_INIT, drm_via_dma_init_t)
> #define DRM_IOCTL_VIA_CMDBUFFER DRM_IOW( DRM_COMMAND_BASE +
> DRM_VIA_CMDBUFFER, drm_via_cmdbuffer_t)
> #define DRM_IOCTL_VIA_FLUSH DRM_IO( DRM_COMMAND_BASE + DRM_VIA_FLUSH)
> @@ -94,6 +103,8 @@
> #define DRM_IOCTL_VIA_CMDBUF_SIZE DRM_IOWR( DRM_COMMAND_BASE +
> DRM_VIA_CMDBUF_SIZE, \
> drm_via_cmdbuf_size_t)
> #define DRM_IOCTL_VIA_WAIT_IRQ DRM_IOWR( DRM_COMMAND_BASE +
> DRM_VIA_WAIT_IRQ, drm_via_irqwait_t)
> +#define DRM_IOCTL_VIA_FLUSH_VIDEO DRM_IOW(DRM_COMMAND_BASE + \
> + DRM_VIA_FLUSH_VIDEO, struct drm_via_video_agp_cmd)
> #define DRM_IOCTL_VIA_DMA_BLIT DRM_IOW(DRM_COMMAND_BASE +
> DRM_VIA_DMA_BLIT, drm_via_dmablit_t)
> #define DRM_IOCTL_VIA_BLIT_SYNC DRM_IOW(DRM_COMMAND_BASE +
> DRM_VIA_BLIT_SYNC, drm_via_blitsync_t)
> #define DRM_IOCTL_VIA_INIT_JUDGE DRM_IOR(DRM_COMMAND_BASE + \
> @@ -119,6 +130,12 @@
> #define VIA_MEM_UNKNOWN 4
> #define VIA_MEM_VIDEO_SAVE 2 /*For video memory need to be saved in
> ACPI */
>
> +enum drm_agp_type {
> + AGP_RING_BUFFER,
> + AGP_DOUBLE_BUFFER,
> + DISABLED
> +};
> +
> typedef struct {
> __u32 offset;
> __u32 size;
> @@ -251,6 +268,12 @@
> struct drm_wait_vblank_reply reply;
> } drm_via_irqwait_t;
>
> +struct drm_via_video_agp_cmd {
> + u32 offset;
> + u32 cmd_size;
> + u32 buffer_size;
> +} ;
> +
> typedef struct drm_via_blitsync {
> __u32 sync_handle;
> unsigned engine;
>
> Thanks and Best regards
> =================================================
> Bruce C. Chang(張祖明)
> VIA Technologies, Inc.
> Address: 1F, 531, Chung-Cheng Road, Hsin-Tien, 231 Taipei
> Tel: +886-2-22185452 Ext 7323
> Mobile: +886-968343824
> Fax: +886-2-22186282
> Skype: Bruce.C.Chang
> Email: brucech...@via.com.tw
>
------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
