On Wed, Sep 18, 2013 at 04:59:01PM +0100, Chris Wilson wrote: > On Wed, Sep 18, 2013 at 08:48:45AM -0700, Ben Widawsky wrote: > > On Wed, Sep 18, 2013 at 03:53:37PM +0100, Chris Wilson wrote: > > > On Wed, Sep 18, 2013 at 07:47:45AM -0700, Ben Widawsky wrote: > > > > On Wed, Sep 18, 2013 at 09:30:17AM +0100, Chris Wilson wrote: > > > > > On Tue, Sep 17, 2013 at 05:02:03PM -0700, Ben Widawsky wrote: > > > > > > > The code does > > > > > > > > > > > > > > exec_start = i915_gem_obj_offset(batch_obj, vm) + > > > > > > > args->batch_start_offset; > > > > > > > exec_len = args->batch_len; > > > > > > > ... > > > > > > > ret = ring->dispatch_execbuffer(ring, > > > > > > > exec_start, exec_len, > > > > > > > flags); > > > > > > > if (ret) > > > > > > > goto err; > > > > > > > > > > > > > > So we lookup the address of the batch buffer in the wrong vm for > > > > > > > I915_DISPATCH_SECURE. > > > > > > > -Chris > > > > > > > > > > > > > > > > > > > But this is very easily solved, no? > > > > > > > > > > > > http://cgit.freedesktop.org/~bwidawsk/drm-intel/tree/drivers/gpu/drm/i915/i915_gem_execbuffer.c?h=ppgtt#n1083 > > > > > > > > > > No, just because the batch once had a ggtt entry doesn't mean the CS is > > > > > going to use the ggtt for this execution... > > > > > -Chris > > > > > > > > > > -- > > > > > Chris Wilson, Intel Open Source Technology Centre > > > > > > > > I guess your use of the term, "CS" here is a bit confusing to me, since > > > > in my head the CS better do whatever we tell it to do. > > > > > > Exactly, we tell the CS to use the ggtt for the SECURE batch (at least > > > on some generations). > > > > > > > If you're trying to say, "just because batch_obj has a ggtt binding; > > > > that doesn't necessarily mean it's the one to use at dispatch." I think > > > > that statement is true, but it's still pretty simple to solve, just use > > > > the I915_DISPATCH_SECURE flag to check instead of > > > > obj->has_global_gtt_mapping. Right? > > > > > > Yes. With the same caveat that it may change. > > > > What may change? Dispatch secure always means use the GGTT offset, does > > it not? Or do you think we'll want privileged batches running from the > > PPGTT? If the latter is true, why ever use GGTT? > > The security bit is already independent from the use-ppgtt bit. With > Haswell it should be possible to execute a privileged batch buffer from > a ppgtt address, right? In which case we would not need to allocate a > GGTT entry. > Right, that was my point. But I *still* fail to see how your earlier request does anything to help this along. The decision can still easily be made at any given time with the I915_DISPATCH_SECURE flag, and per platform driver policy. Say, if you wanted HSW to always run privileged batches out of PPGTT. OTOH, if we need to pass a flag down to specify which address space to execute the batch out of, maybe some more hoops need to be jumped through. I don't see a reason to do this however, and if we want to support IVB, we have to support GGTT execution anyway - so I'm not really sure of a benefit to building in support for PPGTT privileged execution. > > > > I'm really sorry about being so dense here. > > > > > > > > As a side note, I tried really hard to think of how we could end up with > > > > a ggtt mapping for batch_obj, and not want to use that one. I'm not > > > > actually sure it's possible, but I can't prove it as such, so I'm > > > > willing to assume it is possible. Excluding SNB, so few objects actually > > > > will get a ggtt mapping, I don't believe any of them should be reused > > > > for a batch BO - however, IGT can probably make it happen. > > > > > > It's trivial for a batch to end up with a ggtt entry - userspace can > > > just access it through the GTT. Or any bo prior to reusing it as a > > > batch. > > > -Chris > > > > Trivial, perhaps on the gtt mapping. It's not really relevant, but is > > any userspace currently doing that? As for BO reuse, that's a separate > > problem - are we handing back BOs with their mappings intact? That seems > > like a security problem. > > *Userspace* caches its bo with the mappings intact. > -Chris Yes, this seems like a potential (albeit small) problem to me if we (the kernel) arbitrarily upgrade BOs to a GGTT mapping. I guess everything running privileged is trusted though, so we don't need to worry about the unintentional global BOs being snooped. It does sort of seem to circumvent real PPGTT to some extent though if the global mappings linger. Let me state that at this point of the thread, I am lost. Do you still want the original change you asked for? I still don't understand, or see a reason for not just quirking away with a quick check (which I'll state again doesn't even matter until patches which haven't yet been written are posted) - but I clearly haven't been able to convince you; and nobody else is stepping in. -- Ben Widawsky, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
