Re: [PATCH 02/10] drm/radeon: UVD bringup v7

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

 



Am 03.04.2013 16:53, schrieb Jerome Glisse:
On Wed, Apr 03, 2013 at 01:18:31AM +0200, Christian König wrote:
[SNIP]

  /* hardcode those limit for now */
  #define RADEON_VA_IB_OFFSET			(1 << 20)
  #define RADEON_VA_RESERVED_SIZE			(8 << 20)
@@ -357,8 +360,9 @@ struct radeon_bo_list {
  	struct ttm_validate_buffer tv;
  	struct radeon_bo	*bo;
  	uint64_t		gpu_offset;
-	unsigned		rdomain;
-	unsigned		wdomain;
+	bool			written;
+	unsigned		domain;
+	unsigned		alt_domain;
  	u32			tiling_flags;
  };
I think that the change to the rdomain/wdomain should be in a patch
of its own. I think the change is fine but we had issue with change
that touched that part previously, would make bisecting and
understanding the change implication easier.

Agree, I actually planed to do so, but for the whole IP review stuff we needed to maintain a more or less stable patch base. Long story, but I'm going to change it.

@@ -826,7 +830,6 @@ struct radeon_cs_reloc {
  	struct radeon_bo		*robj;
  	struct radeon_bo_list		lobj;
  	uint32_t			handle;
-	uint32_t			flags;
  };
Why removing the flags ? iirc it's not really use right now but i
remember plan to use it.

Ups, just a rebasing artifact. But when it's unused we should remove it, probably just not in this patch.

[SNIP]

+static int radeon_uvd_cs_reloc(struct radeon_cs_parser *p, int data0, int data1)
+{
+	struct radeon_cs_chunk *relocs_chunk;
+	struct radeon_cs_reloc *reloc;
+	unsigned idx, cmd;
+	uint64_t start, end;
+
+	relocs_chunk = &p->chunks[p->chunk_relocs_idx];
+	idx = radeon_get_ib_value(p, data1);
+	if (idx >= relocs_chunk->length_dw) {
+		DRM_ERROR("Relocs at %d after relocations chunk end %d !\n",
+			  idx, relocs_chunk->length_dw);
+		return -EINVAL;
+	}
+
+	reloc = p->relocs_ptr[(idx / 4)];
+	start = reloc->lobj.gpu_offset;
+	end = start + radeon_bo_size(reloc->robj);
+	start += radeon_get_ib_value(p, data0);
I am assuming there is no way for you to know the size that the uvd engine will write to ?
You are not checking anything on uvd possibly overwritting after the bo end.

Yeah that gave me headache for a quite long time, too. The problem is to figure out how much is actually written you need to keep track of the whole lot of informations including the UVD session, create/decode/destroy messages and allot of fiddling with the codec specific parameters.

And if I understand the UVD internals correctly even if we check everything there is no guarantee that a special crafted bitstream could not let UVD to write over the end of the buffer....

Is it ok if we but a big TODO on it for the initial patch?

Cheers,
Christian.
_______________________________________________
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