[Mesa-dev] [PATCH 2/4] i965: Fix intel_miptree_map() signature to be more 64-bit safe

[Ben Widawsky]

On Wed, Nov 19, 2014 at 09:18:54PM -0800, Kenneth Graunke wrote:
> On Wednesday, November 19, 2014 02:13:03 PM Ian Romanick wrote:
> > On 11/18/2014 09:11 PM, Chad Versace wrote:
> > > This patch should diminish the likelihood of pointer arithmetic overflow
> > > bugs, like the one fixed by b69c7c5dac.
> > > 
> > > Change the type of parameter 'out_stride' from int to ptrdiff_t. The
> > > logic is that if you call intel_miptree_map() and use the value of
> > > 'out_stride', then you must be doing pointer arithmetic on 'out_ptr'.
> > > Using ptrdiff_t instead of int should make a little bit harder to hit
> > > overflow bugs.
> > 
> > So... we talked about this a little bit on Monday, and I don't think we
> > ever had a conclusion.  What happens if you have a 32-bit application
> > running on a platform with 48-bit GPU address space?
> 
> CC'ing Ben, who knows all the gory details.
> 
> I don't really understand the problem - the GPU uses 48-bit addressing, and
> can access more than 4G...but we're talking about map, which makes a buffer
> available in an application's virtual address space...which is 32-bit in your
> example.  It should always be placed at a < 4GB virtual address and work out
> fine.

That's correct. Behind the scenes you very well may be getting 48b addresses,
but like Ken said, that should present no issue assuming you can still reprsent
a 64b data type. One would hope that whoever actually lands the patches provides
a way on context create to only get a 4GB address space, since you'll save some
memory, and make correctness a lot easier to "prove".

For the SVM case, there are two models.
1. Create a buffer on the CPU, and just use it at the same offset in the GPU
- no problem.
2. Create a buffer with GEM on the GPU, and just use it on the CPU
- This case, which doesn't make a whole lot of sense IMO, would take some work.

> 
> That said, I don't think the kernel ever uses >= 4GB address spaces today.
> Ben wrote 4GGGTT support and had both kernel and userspace patches to make
> it work, but I don't think it ever actually landed.  I'm pretty sure
> shipping userspace is not quite 48-bit safe - there are a few buffers that
> have to be placed < 4GB (some hardware packets only take 32-bit addresses
> still), and I don't think any software is in place to make that happen.

4GGGTT landed (I haven't checked if it's been reverted). WRT to unsafe
userspace, it's probably mostly safe after your patch to fix libdrm. Unless you
start getting into the prelocation stuff, where I'd bet there are at least a few
unsafe assumptions (unless my patches or equivalent were merged, the kernel
isn't even 48b safe). I also think we determined the 0 state base thing won't
work, which means the odds of actually needing more than 32b are quite small.

> 
> --Ken



> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


-- 
Ben Widawsky, Intel Open Source Technology Center