Re: [PATCH v2 6/7] drm/panfrost: Add support for GPU heap allocations

2019-08-02 Thread Steven Price
On 30/07/2019 21:03, Rob Herring wrote: > On Thu, Jul 25, 2019 at 9:35 AM Steven Price wrote: >> >> On 25/07/2019 15:59, Steven Price wrote: >> [...] >>> It would appear that in the following call sgt==NULL: ret = sg_alloc_table_from_pages(sgt, pages + page_offset,

Re: [PATCH v2 6/7] drm/panfrost: Add support for GPU heap allocations

2019-08-02 Thread Steven Price
On 30/07/2019 20:08, Rob Herring wrote: > On Tue, Jul 30, 2019 at 12:55 PM Alyssa Rosenzweig > wrote: >> >>> In any case, per process AS is a prerequisite to all this. >> >> Oh, I hadn't realized that was still a todo. In the meantime, what's the >> implication of shipping without it? (I.e. in whi

Re: [PATCH v2 6/7] drm/panfrost: Add support for GPU heap allocations

2019-07-31 Thread Alyssa Rosenzweig
> In any case, per process AS is a prerequisite to all this. Oh, I hadn't realized that was still a todo. In the meantime, what's the implication of shipping without it? (I.e. in which threat model are users vulnerable without it?) Malicious userspace process snooping on other framebuffers (on X11

Re: [PATCH v2 6/7] drm/panfrost: Add support for GPU heap allocations

2019-07-30 Thread Rob Herring
On Thu, Jul 25, 2019 at 9:35 AM Steven Price wrote: > > On 25/07/2019 15:59, Steven Price wrote: > [...] > > It would appear that in the following call sgt==NULL: > >> ret = sg_alloc_table_from_pages(sgt, pages + page_offset, > >> NUM_FAULT_PAGES, 0, SZ_2M

Re: [PATCH v2 6/7] drm/panfrost: Add support for GPU heap allocations

2019-07-30 Thread Rob Herring
On Tue, Jul 30, 2019 at 12:55 PM Alyssa Rosenzweig wrote: > > > In any case, per process AS is a prerequisite to all this. > > Oh, I hadn't realized that was still a todo. In the meantime, what's the > implication of shipping without it? (I.e. in which threat model are > users vulnerable without i

Re: [PATCH v2 6/7] drm/panfrost: Add support for GPU heap allocations

2019-07-30 Thread Rob Herring
On Mon, Jul 29, 2019 at 1:18 AM Steven Price wrote: > > On 25/07/2019 18:40, Alyssa Rosenzweig wrote: > >> Sorry, I was being sloppy again![1] I meant CPU mmapped. > > > > No worries, just wanted to check :) > > > >> Apparently the blob in some cases creates a SAME_VA GROW_ON_GPF buffer - > >> sin

Re: [PATCH v2 6/7] drm/panfrost: Add support for GPU heap allocations

2019-07-29 Thread Steven Price
On 25/07/2019 18:40, Alyssa Rosenzweig wrote: Sorry, I was being sloppy again![1] I meant CPU mmapped. No worries, just wanted to check :) Apparently the blob in some cases creates a SAME_VA GROW_ON_GPF buffer - since SAME_VA means permanently mapped on the CPU this translated to mmapping a H

Re: [PATCH v2 6/7] drm/panfrost: Add support for GPU heap allocations

2019-07-29 Thread Alyssa Rosenzweig
> It looks like the driver might be allocated a depth or stencil buffer > without knowing whether it will be used. The buffer is then "grown" if > it is needed by the GPU. The problem is it is possible for the > application to access it later. Hmm. I "think" you should be able to use a dummy (unba

Re: [PATCH v2 6/7] drm/panfrost: Add support for GPU heap allocations

2019-07-26 Thread Rob Herring
On Fri, Jul 26, 2019 at 3:15 AM Steven Price wrote: > > On 25/07/2019 22:11, Rob Herring wrote: > > On Thu, Jul 25, 2019 at 7:08 AM Robin Murphy wrote: > >> > >> Hi Rob, > >> > >> On 25/07/2019 02:10, Rob Herring wrote: > >> [...] > >>> @@ -328,6 +427,18 @@ static irqreturn_t panfrost_mmu_irq_han

Re: [PATCH v2 6/7] drm/panfrost: Add support for GPU heap allocations

2019-07-26 Thread Steven Price
On 25/07/2019 22:11, Rob Herring wrote: On Thu, Jul 25, 2019 at 7:08 AM Robin Murphy wrote: Hi Rob, On 25/07/2019 02:10, Rob Herring wrote: [...] @@ -328,6 +427,18 @@ static irqreturn_t panfrost_mmu_irq_handler(int irq, void *data) access_type = (fault_status >> 8) & 0x3;

Re: [PATCH v2 6/7] drm/panfrost: Add support for GPU heap allocations

2019-07-26 Thread Steven Price
On 25/07/2019 15:59, Steven Price wrote: [...] > It would appear that in the following call sgt==NULL: >> ret = sg_alloc_table_from_pages(sgt, pages + page_offset, >> NUM_FAULT_PAGES, 0, SZ_2M, GFP_KERNEL); > > Which means we've ended up with a BO with bo-

Re: Re: [PATCH v2 6/7] drm/panfrost: Add support for GPU heap allocations

2019-07-26 Thread Alyssa Rosenzweig
> Sorry, I was being sloppy again![1] I meant CPU mmapped. No worries, just wanted to check :) > Apparently the blob in some cases creates a SAME_VA GROW_ON_GPF buffer - > since SAME_VA means permanently mapped on the CPU this translated to > mmapping a HEAP object. Why it does this I've no idea

Re: [PATCH v2 6/7] drm/panfrost: Add support for GPU heap allocations

2019-07-26 Thread Alyssa Rosenzweig
> Either we should prevent mapping of HEAP objects I'm good with that. AFAIK, HEAP objects shouldn't be (CPU) mmapped anyway in normal use; if you need them mapped (for debugging etc), it's no big deal to just.. not set the HEAP flag in debug builds. Or do you mean GPU mapping? signature.asc De

Re: [PATCH v2 6/7] drm/panfrost: Add support for GPU heap allocations

2019-07-26 Thread Steven Price
On 25/07/2019 02:10, Rob Herring wrote: > The midgard/bifrost GPUs need to allocate GPU heap memory which is > allocated on GPU page faults and not pinned in memory. The vendor driver > calls this functionality GROW_ON_GPF. > > This implementation assumes that BOs allocated with the > PANFROST_BO_

Re: Re: [PATCH v2 6/7] drm/panfrost: Add support for GPU heap allocations

2019-07-26 Thread Steven Price
On 25/07/2019 17:13, Alyssa Rosenzweig wrote: >> Either we should prevent mapping of HEAP objects > > I'm good with that. AFAIK, HEAP objects shouldn't be (CPU) mmapped > anyway in normal use; if you need them mapped (for debugging etc), it's > no big deal to just.. not set the HEAP flag in debug

Re: [PATCH v2 6/7] drm/panfrost: Add support for GPU heap allocations

2019-07-26 Thread Steven Price
On 25/07/2019 22:28, Rob Herring wrote: On Thu, Jul 25, 2019 at 9:35 AM Steven Price wrote: On 25/07/2019 15:59, Steven Price wrote: [...] It would appear that in the following call sgt==NULL: ret = sg_alloc_table_from_pages(sgt, pages + page_offset,

Re: [PATCH v2 6/7] drm/panfrost: Add support for GPU heap allocations

2019-07-26 Thread Robin Murphy
On 26/07/2019 10:15, Steven Price wrote: On 25/07/2019 22:11, Rob Herring wrote: On Thu, Jul 25, 2019 at 7:08 AM Robin Murphy wrote: Hi Rob, On 25/07/2019 02:10, Rob Herring wrote: [...] @@ -328,6 +427,18 @@ static irqreturn_t panfrost_mmu_irq_handler(int irq, void *data)    ac

Re: [PATCH v2 6/7] drm/panfrost: Add support for GPU heap allocations

2019-07-25 Thread Rob Herring
On Thu, Jul 25, 2019 at 9:35 AM Steven Price wrote: > > On 25/07/2019 15:59, Steven Price wrote: > [...] > > It would appear that in the following call sgt==NULL: > >> ret = sg_alloc_table_from_pages(sgt, pages + page_offset, > >> NUM_FAULT_PAGES, 0, SZ_2M

Re: [PATCH v2 6/7] drm/panfrost: Add support for GPU heap allocations

2019-07-25 Thread Rob Herring
On Thu, Jul 25, 2019 at 7:08 AM Robin Murphy wrote: > > Hi Rob, > > On 25/07/2019 02:10, Rob Herring wrote: > [...] > > @@ -328,6 +427,18 @@ static irqreturn_t panfrost_mmu_irq_handler(int irq, > > void *data) > > access_type = (fault_status >> 8) & 0x3; > > source_id

Re: [PATCH v2 6/7] drm/panfrost: Add support for GPU heap allocations

2019-07-25 Thread Robin Murphy
Hi Rob, On 25/07/2019 02:10, Rob Herring wrote: [...] @@ -328,6 +427,18 @@ static irqreturn_t panfrost_mmu_irq_handler(int irq, void *data) access_type = (fault_status >> 8) & 0x3; source_id = (fault_status >> 16); + /* Page fault only */ +