Re: [RFC PATCH] drm:- Add a modifier to denote 'protected' framebuffer
On Mon, Sep 30, 2019 at 8:57 AM Ayan Halder wrote: > > On Mon, Sep 30, 2019 at 09:51:35AM +, Brian Starkey wrote: > > Hi, > > > > On Tue, Sep 17, 2019 at 07:36:45PM +0200, Daniel Vetter wrote: > > > On Tue, Sep 17, 2019 at 6:15 PM Neil Armstrong > > > wrote: > > > > > > > > Hi, > > > > > > > > On 17/09/2019 18:07, Liviu Dudau wrote: > > > > > On Tue, Sep 17, 2019 at 02:53:01PM +0200, Daniel Vetter wrote: > > > > >> On Mon, Sep 09, 2019 at 01:42:53PM +, Ayan Halder wrote: > > > > >>> Add a modifier 'DRM_FORMAT_MOD_ARM_PROTECTED' which denotes that > > > > >>> the framebuffer > > > > >>> is allocated in a protected system memory. > > > > >>> Essentially, we want to support EGL_EXT_protected_content in our > > > > >>> komeda driver. > > > > >>> > > > > >>> Signed-off-by: Ayan Kumar Halder > > > > >>> > > > > >>> /-- Note to reviewer > > > > >>> Komeda driver is capable of rendering DRM (Digital Rights > > > > >>> Management) protected > > > > >>> content. The DRM content is stored in a framebuffer allocated in > > > > >>> system memory > > > > >>> (which needs some special hardware signals for access). > > > > >>> > > > > >>> Let us ignore how the protected system memory is allocated and for > > > > >>> the scope of > > > > >>> this discussion, we want to figure out the best way possible for > > > > >>> the userspace > > > > >>> to communicate to the drm driver to turn the protected mode on (for > > > > >>> accessing the > > > > >>> framebuffer with the DRM content) or off. > > > > >>> > > > > >>> The possible ways by which the userspace could achieve this is via:- > > > > >>> > > > > >>> 1. Modifiers :- This looks to me the best way by which the > > > > >>> userspace can > > > > >>> communicate to the kernel to turn the protected mode on for the > > > > >>> komeda driver > > > > >>> as it is going to access one of the protected framebuffers. The > > > > >>> only problem is > > > > >>> that the current modifiers describe the tiling/compression format. > > > > >>> However, it > > > > >>> does not hurt to extend the meaning of modifiers to denote other > > > > >>> attributes of > > > > >>> the framebuffer as well. > > > > >>> > > > > >>> The other reason is that on Android, we get an info from Gralloc > > > > >>> (GRALLOC_USAGE_PROTECTED) which tells us that the buffer is > > > > >>> protected. This can > > > > >>> be used to set up the modifier/s (AddFB2) during framebuffer > > > > >>> creation. > > > > >> > > > > >> How does this mesh with other modifiers, like AFBC? That's where I > > > > >> see the > > > > >> issue here. > > > > > > > > > > AFBC modifiers are currently under Arm's namespace, the thought > > > > > behind the DRM > > > > > modifiers would be to have it as a "generic" modifier. > > > > > > But if it's a generic flag, how do you combine that with other > > > modifiers? Like if you have a tiled buffer, but also encrypted? Or > > > afbc compressed, or whatever else. I'd expect for your hw encryption > > > is orthogonal to the buffer/tiling/compression format used? > > > > This bit doesn't overlap with any of the other AFBC modifiers, so as > > you say it'd be orthogonal, and could be set on AFBC buffers (if we > > went that route). > > > > > > > > > >>> 2. Framebuffer flags :- As of today, this can be one of the two > > > > >>> values > > > > >>> ie (DRM_MODE_FB_INTERLACED/DRM_MODE_FB_MODIFIERS). Unlike > > > > >>> modifiers, the drm > > > > >>> framebuffer flags are generic to the drm subsystem and ideally we > > > > >>> should not > > > > >>> introduce any driver specific constraint/feature. > > > > >>> > > > > >>> 3. Connector property:- I could see the following properties used > > > > >>> for DRM > > > > >>> protected content:- > > > > >>> DRM_MODE_CONTENT_PROTECTION_DESIRED / ENABLED :- "This property is > > > > >>> used by > > > > >>> userspace to request the kernel protect future content communicated > > > > >>> over > > > > >>> the link". Clearly, we are not concerned with the protection > > > > >>> attributes of the > > > > >>> transmitter. So, we cannot use this property for our case. > > > > >>> > > > > >>> 4. DRM plane property:- Again, we want to communicate that the > > > > >>> framebuffer(which > > > > >>> can be attached to any plane) is protected. So introducing a new > > > > >>> plane property > > > > >>> does not help. > > > > >>> > > > > >>> 5. DRM crtc property:- For the same reason as above, introducing a > > > > >>> new crtc > > > > >>> property does not help. > > > > >> > > > > >> 6. Just track this as part of buffer allocation, i.e. I think it does > > > > >> matter how you allocate these protected buffers. We could add a "is > > > > >> protected buffer" flag at the dma_buf level for this. > > > > I also like this approach. The protected-ness is a property of the > > allocation, so makes sense to store it with the allocation IMO. > > > > > > >> > > > > >> So yeah for this stuff here I think we do want the full userspace > > > > >>
Re: [RFC PATCH] drm:- Add a modifier to denote 'protected' framebuffer
On Mon, Sep 30, 2019 at 09:51:35AM +, Brian Starkey wrote: > Hi, > > On Tue, Sep 17, 2019 at 07:36:45PM +0200, Daniel Vetter wrote: > > On Tue, Sep 17, 2019 at 6:15 PM Neil Armstrong > > wrote: > > > > > > Hi, > > > > > > On 17/09/2019 18:07, Liviu Dudau wrote: > > > > On Tue, Sep 17, 2019 at 02:53:01PM +0200, Daniel Vetter wrote: > > > >> On Mon, Sep 09, 2019 at 01:42:53PM +, Ayan Halder wrote: > > > >>> Add a modifier 'DRM_FORMAT_MOD_ARM_PROTECTED' which denotes that the > > > >>> framebuffer > > > >>> is allocated in a protected system memory. > > > >>> Essentially, we want to support EGL_EXT_protected_content in our > > > >>> komeda driver. > > > >>> > > > >>> Signed-off-by: Ayan Kumar Halder > > > >>> > > > >>> /-- Note to reviewer > > > >>> Komeda driver is capable of rendering DRM (Digital Rights Management) > > > >>> protected > > > >>> content. The DRM content is stored in a framebuffer allocated in > > > >>> system memory > > > >>> (which needs some special hardware signals for access). > > > >>> > > > >>> Let us ignore how the protected system memory is allocated and for > > > >>> the scope of > > > >>> this discussion, we want to figure out the best way possible for the > > > >>> userspace > > > >>> to communicate to the drm driver to turn the protected mode on (for > > > >>> accessing the > > > >>> framebuffer with the DRM content) or off. > > > >>> > > > >>> The possible ways by which the userspace could achieve this is via:- > > > >>> > > > >>> 1. Modifiers :- This looks to me the best way by which the userspace > > > >>> can > > > >>> communicate to the kernel to turn the protected mode on for the > > > >>> komeda driver > > > >>> as it is going to access one of the protected framebuffers. The only > > > >>> problem is > > > >>> that the current modifiers describe the tiling/compression format. > > > >>> However, it > > > >>> does not hurt to extend the meaning of modifiers to denote other > > > >>> attributes of > > > >>> the framebuffer as well. > > > >>> > > > >>> The other reason is that on Android, we get an info from Gralloc > > > >>> (GRALLOC_USAGE_PROTECTED) which tells us that the buffer is > > > >>> protected. This can > > > >>> be used to set up the modifier/s (AddFB2) during framebuffer creation. > > > >> > > > >> How does this mesh with other modifiers, like AFBC? That's where I see > > > >> the > > > >> issue here. > > > > > > > > AFBC modifiers are currently under Arm's namespace, the thought behind > > > > the DRM > > > > modifiers would be to have it as a "generic" modifier. > > > > But if it's a generic flag, how do you combine that with other > > modifiers? Like if you have a tiled buffer, but also encrypted? Or > > afbc compressed, or whatever else. I'd expect for your hw encryption > > is orthogonal to the buffer/tiling/compression format used? > > This bit doesn't overlap with any of the other AFBC modifiers, so as > you say it'd be orthogonal, and could be set on AFBC buffers (if we > went that route). > > > > > > >>> 2. Framebuffer flags :- As of today, this can be one of the two values > > > >>> ie (DRM_MODE_FB_INTERLACED/DRM_MODE_FB_MODIFIERS). Unlike modifiers, > > > >>> the drm > > > >>> framebuffer flags are generic to the drm subsystem and ideally we > > > >>> should not > > > >>> introduce any driver specific constraint/feature. > > > >>> > > > >>> 3. Connector property:- I could see the following properties used for > > > >>> DRM > > > >>> protected content:- > > > >>> DRM_MODE_CONTENT_PROTECTION_DESIRED / ENABLED :- "This property is > > > >>> used by > > > >>> userspace to request the kernel protect future content communicated > > > >>> over > > > >>> the link". Clearly, we are not concerned with the protection > > > >>> attributes of the > > > >>> transmitter. So, we cannot use this property for our case. > > > >>> > > > >>> 4. DRM plane property:- Again, we want to communicate that the > > > >>> framebuffer(which > > > >>> can be attached to any plane) is protected. So introducing a new > > > >>> plane property > > > >>> does not help. > > > >>> > > > >>> 5. DRM crtc property:- For the same reason as above, introducing a > > > >>> new crtc > > > >>> property does not help. > > > >> > > > >> 6. Just track this as part of buffer allocation, i.e. I think it does > > > >> matter how you allocate these protected buffers. We could add a "is > > > >> protected buffer" flag at the dma_buf level for this. > > I also like this approach. The protected-ness is a property of the > allocation, so makes sense to store it with the allocation IMO. > > > > >> > > > >> So yeah for this stuff here I think we do want the full userspace side, > > > >> from allocator to rendering something into this protected buffers (no > > > >> need > > > >> to also have the entire "decode a protected bitstream part" imo, since > > > >> that will freak people out). Unfortunately, in my experience, that > > > >> kills > > > >> it for upstream :-/
Re: [RFC PATCH] drm:- Add a modifier to denote 'protected' framebuffer
Hi, On Tue, Sep 17, 2019 at 07:36:45PM +0200, Daniel Vetter wrote: > On Tue, Sep 17, 2019 at 6:15 PM Neil Armstrong > wrote: > > > > Hi, > > > > On 17/09/2019 18:07, Liviu Dudau wrote: > > > On Tue, Sep 17, 2019 at 02:53:01PM +0200, Daniel Vetter wrote: > > >> On Mon, Sep 09, 2019 at 01:42:53PM +, Ayan Halder wrote: > > >>> Add a modifier 'DRM_FORMAT_MOD_ARM_PROTECTED' which denotes that the > > >>> framebuffer > > >>> is allocated in a protected system memory. > > >>> Essentially, we want to support EGL_EXT_protected_content in our komeda > > >>> driver. > > >>> > > >>> Signed-off-by: Ayan Kumar Halder > > >>> > > >>> /-- Note to reviewer > > >>> Komeda driver is capable of rendering DRM (Digital Rights Management) > > >>> protected > > >>> content. The DRM content is stored in a framebuffer allocated in system > > >>> memory > > >>> (which needs some special hardware signals for access). > > >>> > > >>> Let us ignore how the protected system memory is allocated and for the > > >>> scope of > > >>> this discussion, we want to figure out the best way possible for the > > >>> userspace > > >>> to communicate to the drm driver to turn the protected mode on (for > > >>> accessing the > > >>> framebuffer with the DRM content) or off. > > >>> > > >>> The possible ways by which the userspace could achieve this is via:- > > >>> > > >>> 1. Modifiers :- This looks to me the best way by which the userspace can > > >>> communicate to the kernel to turn the protected mode on for the komeda > > >>> driver > > >>> as it is going to access one of the protected framebuffers. The only > > >>> problem is > > >>> that the current modifiers describe the tiling/compression format. > > >>> However, it > > >>> does not hurt to extend the meaning of modifiers to denote other > > >>> attributes of > > >>> the framebuffer as well. > > >>> > > >>> The other reason is that on Android, we get an info from Gralloc > > >>> (GRALLOC_USAGE_PROTECTED) which tells us that the buffer is protected. > > >>> This can > > >>> be used to set up the modifier/s (AddFB2) during framebuffer creation. > > >> > > >> How does this mesh with other modifiers, like AFBC? That's where I see > > >> the > > >> issue here. > > > > > > AFBC modifiers are currently under Arm's namespace, the thought behind > > > the DRM > > > modifiers would be to have it as a "generic" modifier. > > But if it's a generic flag, how do you combine that with other > modifiers? Like if you have a tiled buffer, but also encrypted? Or > afbc compressed, or whatever else. I'd expect for your hw encryption > is orthogonal to the buffer/tiling/compression format used? This bit doesn't overlap with any of the other AFBC modifiers, so as you say it'd be orthogonal, and could be set on AFBC buffers (if we went that route). > > > >>> 2. Framebuffer flags :- As of today, this can be one of the two values > > >>> ie (DRM_MODE_FB_INTERLACED/DRM_MODE_FB_MODIFIERS). Unlike modifiers, > > >>> the drm > > >>> framebuffer flags are generic to the drm subsystem and ideally we > > >>> should not > > >>> introduce any driver specific constraint/feature. > > >>> > > >>> 3. Connector property:- I could see the following properties used for > > >>> DRM > > >>> protected content:- > > >>> DRM_MODE_CONTENT_PROTECTION_DESIRED / ENABLED :- "This property is used > > >>> by > > >>> userspace to request the kernel protect future content communicated over > > >>> the link". Clearly, we are not concerned with the protection attributes > > >>> of the > > >>> transmitter. So, we cannot use this property for our case. > > >>> > > >>> 4. DRM plane property:- Again, we want to communicate that the > > >>> framebuffer(which > > >>> can be attached to any plane) is protected. So introducing a new plane > > >>> property > > >>> does not help. > > >>> > > >>> 5. DRM crtc property:- For the same reason as above, introducing a new > > >>> crtc > > >>> property does not help. > > >> > > >> 6. Just track this as part of buffer allocation, i.e. I think it does > > >> matter how you allocate these protected buffers. We could add a "is > > >> protected buffer" flag at the dma_buf level for this. I also like this approach. The protected-ness is a property of the allocation, so makes sense to store it with the allocation IMO. > > >> > > >> So yeah for this stuff here I think we do want the full userspace side, > > >> from allocator to rendering something into this protected buffers (no > > >> need > > >> to also have the entire "decode a protected bitstream part" imo, since > > >> that will freak people out). Unfortunately, in my experience, that kills > > >> it for upstream :-/ But also in my experience of looking into this for > > >> other gpu's, we really need to have the full picture here to make sure > > >> we're not screwing this up. > > > > > > Maybe Ayan could've been a bit clearer in his message, but the ask here > > > is for ideas > > > on how userspace "communicates" (stores?) the
Re: [RFC PATCH] drm:- Add a modifier to denote 'protected' framebuffer
On Thu, Sep 19, 2019 at 5:13 PM Ayan Halder wrote:
>
> On Thu, Sep 19, 2019 at 04:10:42PM +0200, Daniel Vetter wrote:
> > On Thu, Sep 19, 2019 at 4:03 PM Ayan Halder wrote:
> > >
> > > On Wed, Sep 18, 2019 at 10:30:12PM +0100, Daniel Stone wrote:
> > >
> > > Hi All,
> > > Thanks for your suggestions.
> > >
> > > > Hi Liviu,
> > > >
> > > > On Wed, 18 Sep 2019 at 13:04, Liviu Dudau wrote:
> > > > > On Wed, Sep 18, 2019 at 09:49:40AM +0100, Daniel Stone wrote:
> > > > > > I totally agree. Framebuffers aren't about the underlying memory
> > > > > > they
> > > > > > point to, but about how to _interpret_ that memory: it decorates a
> > > > > > pointer with width, height, stride, format, etc, to allow you to
> > > > > > make
> > > > > > sense of that memory. I see content protection as being the same as
> > > > > > physical contiguity, which is a property of the underlying memory
> > > > > > itself. Regardless of the width, height, or format, you just cannot
> > > > > > access that memory unless it's under the appropriate ('secure
> > > > > > enough')
> > > > > > conditions.
> > > > > >
> > > > > > So I think a dmabuf attribute would be most appropriate, since
> > > > > > that's
> > > > > > where you have to do all your MMU handling, and everything else you
> > > > > > need to do to allow access to that buffer, anyway.
> > > > >
> > > > > Isn't it how AMD currently implements protected buffers as well?
> > > >
> > > > No idea to be honest, I haven't seen anything upstream.
> > > >
> > > > > > There's a lot of complexity beyond just 'it's protected'; for
> > > > > > instance, some CP providers mandate that their content can only be
> > > > > > streamed over HDCP 2.2 Type-1 when going through an external
> > > > > > connection. One way you could do that is to use a secure world
> > > > > > (external controller like Intel's ME, or CPU-internal enclave like
> > > > > > SGX
> > > > > > or TEE) to examine the display pipe configuration, and only then
> > > > > > allow
> > > > > > access to the buffers and/or keys. Stuff like that is always going
> > > > > > to
> > > > > > be out in the realm of vendor & product-policy-specific add-ons, but
> > > > > > it should be possible to agree on at least the basic mechanics and
> > > > > > expectations of a secure path without things like that.
> > > > >
> > > > > I also expect that going through the secure world will be pretty much
> > > > > transparent for
> > > > > the kernel driver, as the most likely hardware implementations would
> > > > > enable
> > > > > additional signaling that will get trapped and handled by the secure
> > > > > OS. I'm not
> > > > > trying to simplify things, just taking the stance that it is
> > > > > userspace that is
> > > > > coordinating all this, we're trying only to find a common ground on
> > > > > how to handle
> > > > > this in the kernel.
> > > >
> > > > Yeah, makes sense.
> > > >
> > > > As a strawman, how about a new flag to drmPrimeHandleToFD() which sets
> > > > the 'protected' flag on the resulting dmabuf?
> > >
> > > To be honest, during our internal discussion james.qian.w...@arm.com had a
> > > similar suggestion of adding a new flag to dma_buf but I decided
> > > against it.
> > >
> > > As per my understanding, adding custom dma buf flags (like
> > > AMDGPU_GEM_CREATE_XXX, etc) is possible if we(Arm) had our own allocator.
> > > We
> > > rely on the dumb allocator and ion allocator for framebuffer creation.
> > >
> > > I was looking at an allocator independent way of userspace
> > > communicating to the drm framework that the framebuffer is protected.
> > >
> > > Thus, it looked to me that framebuffer modifier is the best (or the least
> > > corrupt) way of going forth.
> > >
> > > We use ion and dumb allocator for framebuffer object creation. The way
> > > I see it is as follows :-
> > >
> > > 1. For ion allocator :-
> > > Userspace can specify that it wants the buffer from a secure heap or any
> > > other
> > > special region of heap. The ion driver will either fault into the secure
> > > os to
> > > create the buffers or it will do some other magic. Honestly, I have still
> > > not
> > > figured that out. But it will be agnostic to the drm core.
> >
> > Allocating buffers from a special heap is what I expected the
> > interface to be. The issue is that if we specify the secure mode any
> > time later on, then it could be changed. E.g. with Daniel Stone's idea
> > of a handle2fd flag, you could export the buffer twice, once secure,
> > once non-secure. That sounds like a silly thing to me, and better to
> > prevent that - or is this actually possible/wanted, i.e. do we want to
> > change the secure mode for a buffer later on?
> >
> > > The userspace gets a handle to the buffer and then it calls addFB2 with
> > > DRM_FORMAT_MOD_ARM_PROTECTED, so that the driver can configure the
> > > dpu's protection bits (to access the memory with special signals).
> >
> > If we allocate a secure buffer there's no need for
Re: [RFC PATCH] drm:- Add a modifier to denote 'protected' framebuffer
On Wed, Sep 18, 2019 at 8:04 AM Liviu Dudau wrote:
>
> On Wed, Sep 18, 2019 at 09:49:40AM +0100, Daniel Stone wrote:
> > Hi all,
>
> Hi,
>
> >
> > On Tue, 17 Sep 2019 at 13:53, Daniel Vetter wrote:
> > > On Mon, Sep 09, 2019 at 01:42:53PM +, Ayan Halder wrote:
> > > > Let us ignore how the protected system memory is allocated and for the
> > > > scope of
> > > > this discussion, we want to figure out the best way possible for the
> > > > userspace
> > > > to communicate to the drm driver to turn the protected mode on (for
> > > > accessing the
> > > > framebuffer with the DRM content) or off.
> > > >
> > > > The possible ways by which the userspace could achieve this is via:-
> > > >
> > > > 1. Modifiers :- This looks to me the best way by which the userspace can
> > > > communicate to the kernel to turn the protected mode on for the komeda
> > > > driver
> > > > as it is going to access one of the protected framebuffers. The only
> > > > problem is
> > > > that the current modifiers describe the tiling/compression format.
> > > > However, it
> > > > does not hurt to extend the meaning of modifiers to denote other
> > > > attributes of
> > > > the framebuffer as well.
> > > >
> > > > The other reason is that on Android, we get an info from Gralloc
> > > > (GRALLOC_USAGE_PROTECTED) which tells us that the buffer is protected.
> > > > This can
> > > > be used to set up the modifier/s (AddFB2) during framebuffer creation.
> > >
> > > How does this mesh with other modifiers, like AFBC? That's where I see the
> > > issue here.
> >
> > Yeah. On other SoCs, we certainly have usecases for protected content
> > with different buffer layouts. The i.MX8M can protect particular
> > memory areas and partition them to protect access from particular
> > devices (e.g. display controller and video decoder only, not CPU or
> > GPU). Those memory areas can contain linear buffers, or tiled buffers,
> > or supertiled buffers, or ...
> >
> > Stealing a modifier isn't appropriate.
>
> I tend to agree with you here. Given that the modifiers were introduced
> mostly to
> help vendors add their ideosyncratic bits, having a generic flag as a
> modifier is
> not a good idea.
>
> >
> > > 6. Just track this as part of buffer allocation, i.e. I think it does
> > > matter how you allocate these protected buffers. We could add a "is
> > > protected buffer" flag at the dma_buf level for this.
> >
> > I totally agree. Framebuffers aren't about the underlying memory they
> > point to, but about how to _interpret_ that memory: it decorates a
> > pointer with width, height, stride, format, etc, to allow you to make
> > sense of that memory. I see content protection as being the same as
> > physical contiguity, which is a property of the underlying memory
> > itself. Regardless of the width, height, or format, you just cannot
> > access that memory unless it's under the appropriate ('secure enough')
> > conditions.
> >
> > So I think a dmabuf attribute would be most appropriate, since that's
> > where you have to do all your MMU handling, and everything else you
> > need to do to allow access to that buffer, anyway.
>
> Isn't it how AMD currently implements protected buffers as well?
Our buffer security is implemented as part of our GPU's virtual memory
interface. Our current proposed API is the set a "secure" flag when
memory is allocated and then when a process maps the buffer into its
GPU virtual address space, we set the appropriate flags in the PTEs.
See this series for more info:
https://patchwork.freedesktop.org/series/66531/
You can have the buffers in whatever tiled or linear format makes
sense. The encryption happens on top of that. In order for the
various blocks on the GPU to be able to access the encrypted memory,
they need to be switched into "secure" mode. So for example, if you
submit work to the GPU that included secure memory, you need to flag
that work as secure. I don't think our solution is really shareable
outside of our asics so in our case, we don't really have to worry
about passing secure buffers between drivers.
Alex
>
> >
> > > So yeah for this stuff here I think we do want the full userspace side,
> > > from allocator to rendering something into this protected buffers (no need
> > > to also have the entire "decode a protected bitstream part" imo, since
> > > that will freak people out). Unfortunately, in my experience, that kills
> > > it for upstream :-/ But also in my experience of looking into this for
> > > other gpu's, we really need to have the full picture here to make sure
> > > we're not screwing this up.
> >
> > Yeah. I know there are a few people looking at this at the moment, so
> > hopefully we are able to get something up and out in the open as a
> > strawman.
> >
> > There's a lot of complexity beyond just 'it's protected'; for
> > instance, some CP providers mandate that their content can only be
> > streamed over HDCP 2.2 Type-1 when going through an external
> > connection.
Re: [RFC PATCH] drm:- Add a modifier to denote 'protected' framebuffer
On Thu, Sep 19, 2019 at 04:10:42PM +0200, Daniel Vetter wrote:
> On Thu, Sep 19, 2019 at 4:03 PM Ayan Halder wrote:
> >
> > On Wed, Sep 18, 2019 at 10:30:12PM +0100, Daniel Stone wrote:
> >
> > Hi All,
> > Thanks for your suggestions.
> >
> > > Hi Liviu,
> > >
> > > On Wed, 18 Sep 2019 at 13:04, Liviu Dudau wrote:
> > > > On Wed, Sep 18, 2019 at 09:49:40AM +0100, Daniel Stone wrote:
> > > > > I totally agree. Framebuffers aren't about the underlying memory they
> > > > > point to, but about how to _interpret_ that memory: it decorates a
> > > > > pointer with width, height, stride, format, etc, to allow you to make
> > > > > sense of that memory. I see content protection as being the same as
> > > > > physical contiguity, which is a property of the underlying memory
> > > > > itself. Regardless of the width, height, or format, you just cannot
> > > > > access that memory unless it's under the appropriate ('secure enough')
> > > > > conditions.
> > > > >
> > > > > So I think a dmabuf attribute would be most appropriate, since that's
> > > > > where you have to do all your MMU handling, and everything else you
> > > > > need to do to allow access to that buffer, anyway.
> > > >
> > > > Isn't it how AMD currently implements protected buffers as well?
> > >
> > > No idea to be honest, I haven't seen anything upstream.
> > >
> > > > > There's a lot of complexity beyond just 'it's protected'; for
> > > > > instance, some CP providers mandate that their content can only be
> > > > > streamed over HDCP 2.2 Type-1 when going through an external
> > > > > connection. One way you could do that is to use a secure world
> > > > > (external controller like Intel's ME, or CPU-internal enclave like SGX
> > > > > or TEE) to examine the display pipe configuration, and only then allow
> > > > > access to the buffers and/or keys. Stuff like that is always going to
> > > > > be out in the realm of vendor & product-policy-specific add-ons, but
> > > > > it should be possible to agree on at least the basic mechanics and
> > > > > expectations of a secure path without things like that.
> > > >
> > > > I also expect that going through the secure world will be pretty much
> > > > transparent for
> > > > the kernel driver, as the most likely hardware implementations would
> > > > enable
> > > > additional signaling that will get trapped and handled by the secure
> > > > OS. I'm not
> > > > trying to simplify things, just taking the stance that it is userspace
> > > > that is
> > > > coordinating all this, we're trying only to find a common ground on how
> > > > to handle
> > > > this in the kernel.
> > >
> > > Yeah, makes sense.
> > >
> > > As a strawman, how about a new flag to drmPrimeHandleToFD() which sets
> > > the 'protected' flag on the resulting dmabuf?
> >
> > To be honest, during our internal discussion james.qian.w...@arm.com had a
> > similar suggestion of adding a new flag to dma_buf but I decided
> > against it.
> >
> > As per my understanding, adding custom dma buf flags (like
> > AMDGPU_GEM_CREATE_XXX, etc) is possible if we(Arm) had our own allocator. We
> > rely on the dumb allocator and ion allocator for framebuffer creation.
> >
> > I was looking at an allocator independent way of userspace
> > communicating to the drm framework that the framebuffer is protected.
> >
> > Thus, it looked to me that framebuffer modifier is the best (or the least
> > corrupt) way of going forth.
> >
> > We use ion and dumb allocator for framebuffer object creation. The way
> > I see it is as follows :-
> >
> > 1. For ion allocator :-
> > Userspace can specify that it wants the buffer from a secure heap or any
> > other
> > special region of heap. The ion driver will either fault into the secure os
> > to
> > create the buffers or it will do some other magic. Honestly, I have still
> > not
> > figured that out. But it will be agnostic to the drm core.
>
> Allocating buffers from a special heap is what I expected the
> interface to be. The issue is that if we specify the secure mode any
> time later on, then it could be changed. E.g. with Daniel Stone's idea
> of a handle2fd flag, you could export the buffer twice, once secure,
> once non-secure. That sounds like a silly thing to me, and better to
> prevent that - or is this actually possible/wanted, i.e. do we want to
> change the secure mode for a buffer later on?
>
> > The userspace gets a handle to the buffer and then it calls addFB2 with
> > DRM_FORMAT_MOD_ARM_PROTECTED, so that the driver can configure the
> > dpu's protection bits (to access the memory with special signals).
>
> If we allocate a secure buffer there's no need for flags anymore I
> think - it would be a property of the underlying buffer (like a
> contiguous buffer). All we need are two things:
> - make sure secure buffers can only be imported by secure-buffer aware drivers
> - some way for such drivers to figure out whether they deal with a
> secure buffer or not.
I am with you on this. Yes, we
Re: [RFC PATCH] drm:- Add a modifier to denote 'protected' framebuffer
On Thu, Sep 19, 2019 at 4:03 PM Ayan Halder wrote:
>
> On Wed, Sep 18, 2019 at 10:30:12PM +0100, Daniel Stone wrote:
>
> Hi All,
> Thanks for your suggestions.
>
> > Hi Liviu,
> >
> > On Wed, 18 Sep 2019 at 13:04, Liviu Dudau wrote:
> > > On Wed, Sep 18, 2019 at 09:49:40AM +0100, Daniel Stone wrote:
> > > > I totally agree. Framebuffers aren't about the underlying memory they
> > > > point to, but about how to _interpret_ that memory: it decorates a
> > > > pointer with width, height, stride, format, etc, to allow you to make
> > > > sense of that memory. I see content protection as being the same as
> > > > physical contiguity, which is a property of the underlying memory
> > > > itself. Regardless of the width, height, or format, you just cannot
> > > > access that memory unless it's under the appropriate ('secure enough')
> > > > conditions.
> > > >
> > > > So I think a dmabuf attribute would be most appropriate, since that's
> > > > where you have to do all your MMU handling, and everything else you
> > > > need to do to allow access to that buffer, anyway.
> > >
> > > Isn't it how AMD currently implements protected buffers as well?
> >
> > No idea to be honest, I haven't seen anything upstream.
> >
> > > > There's a lot of complexity beyond just 'it's protected'; for
> > > > instance, some CP providers mandate that their content can only be
> > > > streamed over HDCP 2.2 Type-1 when going through an external
> > > > connection. One way you could do that is to use a secure world
> > > > (external controller like Intel's ME, or CPU-internal enclave like SGX
> > > > or TEE) to examine the display pipe configuration, and only then allow
> > > > access to the buffers and/or keys. Stuff like that is always going to
> > > > be out in the realm of vendor & product-policy-specific add-ons, but
> > > > it should be possible to agree on at least the basic mechanics and
> > > > expectations of a secure path without things like that.
> > >
> > > I also expect that going through the secure world will be pretty much
> > > transparent for
> > > the kernel driver, as the most likely hardware implementations would
> > > enable
> > > additional signaling that will get trapped and handled by the secure OS.
> > > I'm not
> > > trying to simplify things, just taking the stance that it is userspace
> > > that is
> > > coordinating all this, we're trying only to find a common ground on how
> > > to handle
> > > this in the kernel.
> >
> > Yeah, makes sense.
> >
> > As a strawman, how about a new flag to drmPrimeHandleToFD() which sets
> > the 'protected' flag on the resulting dmabuf?
>
> To be honest, during our internal discussion james.qian.w...@arm.com had a
> similar suggestion of adding a new flag to dma_buf but I decided
> against it.
>
> As per my understanding, adding custom dma buf flags (like
> AMDGPU_GEM_CREATE_XXX, etc) is possible if we(Arm) had our own allocator. We
> rely on the dumb allocator and ion allocator for framebuffer creation.
>
> I was looking at an allocator independent way of userspace
> communicating to the drm framework that the framebuffer is protected.
>
> Thus, it looked to me that framebuffer modifier is the best (or the least
> corrupt) way of going forth.
>
> We use ion and dumb allocator for framebuffer object creation. The way
> I see it is as follows :-
>
> 1. For ion allocator :-
> Userspace can specify that it wants the buffer from a secure heap or any other
> special region of heap. The ion driver will either fault into the secure os to
> create the buffers or it will do some other magic. Honestly, I have still not
> figured that out. But it will be agnostic to the drm core.
Allocating buffers from a special heap is what I expected the
interface to be. The issue is that if we specify the secure mode any
time later on, then it could be changed. E.g. with Daniel Stone's idea
of a handle2fd flag, you could export the buffer twice, once secure,
once non-secure. That sounds like a silly thing to me, and better to
prevent that - or is this actually possible/wanted, i.e. do we want to
change the secure mode for a buffer later on?
> The userspace gets a handle to the buffer and then it calls addFB2 with
> DRM_FORMAT_MOD_ARM_PROTECTED, so that the driver can configure the
> dpu's protection bits (to access the memory with special signals).
If we allocate a secure buffer there's no need for flags anymore I
think - it would be a property of the underlying buffer (like a
contiguous buffer). All we need are two things:
- make sure secure buffers can only be imported by secure-buffer aware drivers
- some way for such drivers to figure out whether they deal with a
secure buffer or not.
There's no need for any flags anywhere else with the ion/secure
dma-buf heap solution. E.g. for contig buffer we also dont pass around
a DRM_FORMAT_MOD_PHYSICALLY_CONTIG for addfb2.
> 2. For dumb allocator :-
> I am curious to know if we can add 'IS_PROTECTED' flag to
> drm_mode_create_dumb.flags. This
Re: [RFC PATCH] drm:- Add a modifier to denote 'protected' framebuffer
On Wed, Sep 18, 2019 at 10:30:12PM +0100, Daniel Stone wrote:
Hi All,
Thanks for your suggestions.
> Hi Liviu,
>
> On Wed, 18 Sep 2019 at 13:04, Liviu Dudau wrote:
> > On Wed, Sep 18, 2019 at 09:49:40AM +0100, Daniel Stone wrote:
> > > I totally agree. Framebuffers aren't about the underlying memory they
> > > point to, but about how to _interpret_ that memory: it decorates a
> > > pointer with width, height, stride, format, etc, to allow you to make
> > > sense of that memory. I see content protection as being the same as
> > > physical contiguity, which is a property of the underlying memory
> > > itself. Regardless of the width, height, or format, you just cannot
> > > access that memory unless it's under the appropriate ('secure enough')
> > > conditions.
> > >
> > > So I think a dmabuf attribute would be most appropriate, since that's
> > > where you have to do all your MMU handling, and everything else you
> > > need to do to allow access to that buffer, anyway.
> >
> > Isn't it how AMD currently implements protected buffers as well?
>
> No idea to be honest, I haven't seen anything upstream.
>
> > > There's a lot of complexity beyond just 'it's protected'; for
> > > instance, some CP providers mandate that their content can only be
> > > streamed over HDCP 2.2 Type-1 when going through an external
> > > connection. One way you could do that is to use a secure world
> > > (external controller like Intel's ME, or CPU-internal enclave like SGX
> > > or TEE) to examine the display pipe configuration, and only then allow
> > > access to the buffers and/or keys. Stuff like that is always going to
> > > be out in the realm of vendor & product-policy-specific add-ons, but
> > > it should be possible to agree on at least the basic mechanics and
> > > expectations of a secure path without things like that.
> >
> > I also expect that going through the secure world will be pretty much
> > transparent for
> > the kernel driver, as the most likely hardware implementations would enable
> > additional signaling that will get trapped and handled by the secure OS.
> > I'm not
> > trying to simplify things, just taking the stance that it is userspace that
> > is
> > coordinating all this, we're trying only to find a common ground on how to
> > handle
> > this in the kernel.
>
> Yeah, makes sense.
>
> As a strawman, how about a new flag to drmPrimeHandleToFD() which sets
> the 'protected' flag on the resulting dmabuf?
To be honest, during our internal discussion james.qian.w...@arm.com had a
similar suggestion of adding a new flag to dma_buf but I decided
against it.
As per my understanding, adding custom dma buf flags (like
AMDGPU_GEM_CREATE_XXX, etc) is possible if we(Arm) had our own allocator. We
rely on the dumb allocator and ion allocator for framebuffer creation.
I was looking at an allocator independent way of userspace
communicating to the drm framework that the framebuffer is protected.
Thus, it looked to me that framebuffer modifier is the best (or the least
corrupt) way of going forth.
We use ion and dumb allocator for framebuffer object creation. The way
I see it is as follows :-
1. For ion allocator :-
Userspace can specify that it wants the buffer from a secure heap or any other
special region of heap. The ion driver will either fault into the secure os to
create the buffers or it will do some other magic. Honestly, I have still not
figured that out. But it will be agnostic to the drm core.
The userspace gets a handle to the buffer and then it calls addFB2 with
DRM_FORMAT_MOD_ARM_PROTECTED, so that the driver can configure the
dpu's protection bits (to access the memory with special signals).
2. For dumb allocator :-
I am curious to know if we can add 'IS_PROTECTED' flag to
drm_mode_create_dumb.flags. This can/might be used to set dma_buf
flags. Let me know if this is an incorrect/forbidden path.
In a nutshell, my objective is to figure out if the userspace is able
to communicate to the drm core about the 'protection' status of the
buffer without introducing Arm specific buffer allocator.
Thanks,
Ayan
> Cheers,
> Daniel
Re: [RFC PATCH] drm:- Add a modifier to denote 'protected' framebuffer
Hi Liviu,
On Wed, 18 Sep 2019 at 13:04, Liviu Dudau wrote:
> On Wed, Sep 18, 2019 at 09:49:40AM +0100, Daniel Stone wrote:
> > I totally agree. Framebuffers aren't about the underlying memory they
> > point to, but about how to _interpret_ that memory: it decorates a
> > pointer with width, height, stride, format, etc, to allow you to make
> > sense of that memory. I see content protection as being the same as
> > physical contiguity, which is a property of the underlying memory
> > itself. Regardless of the width, height, or format, you just cannot
> > access that memory unless it's under the appropriate ('secure enough')
> > conditions.
> >
> > So I think a dmabuf attribute would be most appropriate, since that's
> > where you have to do all your MMU handling, and everything else you
> > need to do to allow access to that buffer, anyway.
>
> Isn't it how AMD currently implements protected buffers as well?
No idea to be honest, I haven't seen anything upstream.
> > There's a lot of complexity beyond just 'it's protected'; for
> > instance, some CP providers mandate that their content can only be
> > streamed over HDCP 2.2 Type-1 when going through an external
> > connection. One way you could do that is to use a secure world
> > (external controller like Intel's ME, or CPU-internal enclave like SGX
> > or TEE) to examine the display pipe configuration, and only then allow
> > access to the buffers and/or keys. Stuff like that is always going to
> > be out in the realm of vendor & product-policy-specific add-ons, but
> > it should be possible to agree on at least the basic mechanics and
> > expectations of a secure path without things like that.
>
> I also expect that going through the secure world will be pretty much
> transparent for
> the kernel driver, as the most likely hardware implementations would enable
> additional signaling that will get trapped and handled by the secure OS. I'm
> not
> trying to simplify things, just taking the stance that it is userspace that is
> coordinating all this, we're trying only to find a common ground on how to
> handle
> this in the kernel.
Yeah, makes sense.
As a strawman, how about a new flag to drmPrimeHandleToFD() which sets
the 'protected' flag on the resulting dmabuf?
Cheers,
Daniel
Re: [RFC PATCH] drm:- Add a modifier to denote 'protected' framebuffer
On Wed, Sep 18, 2019 at 09:49:40AM +0100, Daniel Stone wrote:
> Hi all,
Hi,
>
> On Tue, 17 Sep 2019 at 13:53, Daniel Vetter wrote:
> > On Mon, Sep 09, 2019 at 01:42:53PM +, Ayan Halder wrote:
> > > Let us ignore how the protected system memory is allocated and for the
> > > scope of
> > > this discussion, we want to figure out the best way possible for the
> > > userspace
> > > to communicate to the drm driver to turn the protected mode on (for
> > > accessing the
> > > framebuffer with the DRM content) or off.
> > >
> > > The possible ways by which the userspace could achieve this is via:-
> > >
> > > 1. Modifiers :- This looks to me the best way by which the userspace can
> > > communicate to the kernel to turn the protected mode on for the komeda
> > > driver
> > > as it is going to access one of the protected framebuffers. The only
> > > problem is
> > > that the current modifiers describe the tiling/compression format.
> > > However, it
> > > does not hurt to extend the meaning of modifiers to denote other
> > > attributes of
> > > the framebuffer as well.
> > >
> > > The other reason is that on Android, we get an info from Gralloc
> > > (GRALLOC_USAGE_PROTECTED) which tells us that the buffer is protected.
> > > This can
> > > be used to set up the modifier/s (AddFB2) during framebuffer creation.
> >
> > How does this mesh with other modifiers, like AFBC? That's where I see the
> > issue here.
>
> Yeah. On other SoCs, we certainly have usecases for protected content
> with different buffer layouts. The i.MX8M can protect particular
> memory areas and partition them to protect access from particular
> devices (e.g. display controller and video decoder only, not CPU or
> GPU). Those memory areas can contain linear buffers, or tiled buffers,
> or supertiled buffers, or ...
>
> Stealing a modifier isn't appropriate.
I tend to agree with you here. Given that the modifiers were introduced mostly
to
help vendors add their ideosyncratic bits, having a generic flag as a modifier
is
not a good idea.
>
> > 6. Just track this as part of buffer allocation, i.e. I think it does
> > matter how you allocate these protected buffers. We could add a "is
> > protected buffer" flag at the dma_buf level for this.
>
> I totally agree. Framebuffers aren't about the underlying memory they
> point to, but about how to _interpret_ that memory: it decorates a
> pointer with width, height, stride, format, etc, to allow you to make
> sense of that memory. I see content protection as being the same as
> physical contiguity, which is a property of the underlying memory
> itself. Regardless of the width, height, or format, you just cannot
> access that memory unless it's under the appropriate ('secure enough')
> conditions.
>
> So I think a dmabuf attribute would be most appropriate, since that's
> where you have to do all your MMU handling, and everything else you
> need to do to allow access to that buffer, anyway.
Isn't it how AMD currently implements protected buffers as well?
>
> > So yeah for this stuff here I think we do want the full userspace side,
> > from allocator to rendering something into this protected buffers (no need
> > to also have the entire "decode a protected bitstream part" imo, since
> > that will freak people out). Unfortunately, in my experience, that kills
> > it for upstream :-/ But also in my experience of looking into this for
> > other gpu's, we really need to have the full picture here to make sure
> > we're not screwing this up.
>
> Yeah. I know there are a few people looking at this at the moment, so
> hopefully we are able to get something up and out in the open as a
> strawman.
>
> There's a lot of complexity beyond just 'it's protected'; for
> instance, some CP providers mandate that their content can only be
> streamed over HDCP 2.2 Type-1 when going through an external
> connection. One way you could do that is to use a secure world
> (external controller like Intel's ME, or CPU-internal enclave like SGX
> or TEE) to examine the display pipe configuration, and only then allow
> access to the buffers and/or keys. Stuff like that is always going to
> be out in the realm of vendor & product-policy-specific add-ons, but
> it should be possible to agree on at least the basic mechanics and
> expectations of a secure path without things like that.
I also expect that going through the secure world will be pretty much
transparent for
the kernel driver, as the most likely hardware implementations would enable
additional signaling that will get trapped and handled by the secure OS. I'm not
trying to simplify things, just taking the stance that it is userspace that is
coordinating all this, we're trying only to find a common ground on how to
handle
this in the kernel.
Best regards,
Liviu
>
> Cheers,
> Daniel
--
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---
¯\_(ツ)_/¯
Re: [RFC PATCH] drm:- Add a modifier to denote 'protected' framebuffer
Hi all,
On Tue, 17 Sep 2019 at 13:53, Daniel Vetter wrote:
> On Mon, Sep 09, 2019 at 01:42:53PM +, Ayan Halder wrote:
> > Let us ignore how the protected system memory is allocated and for the
> > scope of
> > this discussion, we want to figure out the best way possible for the
> > userspace
> > to communicate to the drm driver to turn the protected mode on (for
> > accessing the
> > framebuffer with the DRM content) or off.
> >
> > The possible ways by which the userspace could achieve this is via:-
> >
> > 1. Modifiers :- This looks to me the best way by which the userspace can
> > communicate to the kernel to turn the protected mode on for the komeda
> > driver
> > as it is going to access one of the protected framebuffers. The only
> > problem is
> > that the current modifiers describe the tiling/compression format. However,
> > it
> > does not hurt to extend the meaning of modifiers to denote other attributes
> > of
> > the framebuffer as well.
> >
> > The other reason is that on Android, we get an info from Gralloc
> > (GRALLOC_USAGE_PROTECTED) which tells us that the buffer is protected. This
> > can
> > be used to set up the modifier/s (AddFB2) during framebuffer creation.
>
> How does this mesh with other modifiers, like AFBC? That's where I see the
> issue here.
Yeah. On other SoCs, we certainly have usecases for protected content
with different buffer layouts. The i.MX8M can protect particular
memory areas and partition them to protect access from particular
devices (e.g. display controller and video decoder only, not CPU or
GPU). Those memory areas can contain linear buffers, or tiled buffers,
or supertiled buffers, or ...
Stealing a modifier isn't appropriate.
> 6. Just track this as part of buffer allocation, i.e. I think it does
> matter how you allocate these protected buffers. We could add a "is
> protected buffer" flag at the dma_buf level for this.
I totally agree. Framebuffers aren't about the underlying memory they
point to, but about how to _interpret_ that memory: it decorates a
pointer with width, height, stride, format, etc, to allow you to make
sense of that memory. I see content protection as being the same as
physical contiguity, which is a property of the underlying memory
itself. Regardless of the width, height, or format, you just cannot
access that memory unless it's under the appropriate ('secure enough')
conditions.
So I think a dmabuf attribute would be most appropriate, since that's
where you have to do all your MMU handling, and everything else you
need to do to allow access to that buffer, anyway.
> So yeah for this stuff here I think we do want the full userspace side,
> from allocator to rendering something into this protected buffers (no need
> to also have the entire "decode a protected bitstream part" imo, since
> that will freak people out). Unfortunately, in my experience, that kills
> it for upstream :-/ But also in my experience of looking into this for
> other gpu's, we really need to have the full picture here to make sure
> we're not screwing this up.
Yeah. I know there are a few people looking at this at the moment, so
hopefully we are able to get something up and out in the open as a
strawman.
There's a lot of complexity beyond just 'it's protected'; for
instance, some CP providers mandate that their content can only be
streamed over HDCP 2.2 Type-1 when going through an external
connection. One way you could do that is to use a secure world
(external controller like Intel's ME, or CPU-internal enclave like SGX
or TEE) to examine the display pipe configuration, and only then allow
access to the buffers and/or keys. Stuff like that is always going to
be out in the realm of vendor & product-policy-specific add-ons, but
it should be possible to agree on at least the basic mechanics and
expectations of a secure path without things like that.
Cheers,
Daniel
Re: [RFC PATCH] drm:- Add a modifier to denote 'protected' framebuffer
On Tue, Sep 17, 2019 at 6:15 PM Neil Armstrong wrote: > > Hi, > > On 17/09/2019 18:07, Liviu Dudau wrote: > > On Tue, Sep 17, 2019 at 02:53:01PM +0200, Daniel Vetter wrote: > >> On Mon, Sep 09, 2019 at 01:42:53PM +, Ayan Halder wrote: > >>> Add a modifier 'DRM_FORMAT_MOD_ARM_PROTECTED' which denotes that the > >>> framebuffer > >>> is allocated in a protected system memory. > >>> Essentially, we want to support EGL_EXT_protected_content in our komeda > >>> driver. > >>> > >>> Signed-off-by: Ayan Kumar Halder > >>> > >>> /-- Note to reviewer > >>> Komeda driver is capable of rendering DRM (Digital Rights Management) > >>> protected > >>> content. The DRM content is stored in a framebuffer allocated in system > >>> memory > >>> (which needs some special hardware signals for access). > >>> > >>> Let us ignore how the protected system memory is allocated and for the > >>> scope of > >>> this discussion, we want to figure out the best way possible for the > >>> userspace > >>> to communicate to the drm driver to turn the protected mode on (for > >>> accessing the > >>> framebuffer with the DRM content) or off. > >>> > >>> The possible ways by which the userspace could achieve this is via:- > >>> > >>> 1. Modifiers :- This looks to me the best way by which the userspace can > >>> communicate to the kernel to turn the protected mode on for the komeda > >>> driver > >>> as it is going to access one of the protected framebuffers. The only > >>> problem is > >>> that the current modifiers describe the tiling/compression format. > >>> However, it > >>> does not hurt to extend the meaning of modifiers to denote other > >>> attributes of > >>> the framebuffer as well. > >>> > >>> The other reason is that on Android, we get an info from Gralloc > >>> (GRALLOC_USAGE_PROTECTED) which tells us that the buffer is protected. > >>> This can > >>> be used to set up the modifier/s (AddFB2) during framebuffer creation. > >> > >> How does this mesh with other modifiers, like AFBC? That's where I see the > >> issue here. > > > > AFBC modifiers are currently under Arm's namespace, the thought behind the > > DRM > > modifiers would be to have it as a "generic" modifier. But if it's a generic flag, how do you combine that with other modifiers? Like if you have a tiled buffer, but also encrypted? Or afbc compressed, or whatever else. I'd expect for your hw encryption is orthogonal to the buffer/tiling/compression format used? > >>> 2. Framebuffer flags :- As of today, this can be one of the two values > >>> ie (DRM_MODE_FB_INTERLACED/DRM_MODE_FB_MODIFIERS). Unlike modifiers, the > >>> drm > >>> framebuffer flags are generic to the drm subsystem and ideally we should > >>> not > >>> introduce any driver specific constraint/feature. > >>> > >>> 3. Connector property:- I could see the following properties used for DRM > >>> protected content:- > >>> DRM_MODE_CONTENT_PROTECTION_DESIRED / ENABLED :- "This property is used by > >>> userspace to request the kernel protect future content communicated over > >>> the link". Clearly, we are not concerned with the protection attributes > >>> of the > >>> transmitter. So, we cannot use this property for our case. > >>> > >>> 4. DRM plane property:- Again, we want to communicate that the > >>> framebuffer(which > >>> can be attached to any plane) is protected. So introducing a new plane > >>> property > >>> does not help. > >>> > >>> 5. DRM crtc property:- For the same reason as above, introducing a new > >>> crtc > >>> property does not help. > >> > >> 6. Just track this as part of buffer allocation, i.e. I think it does > >> matter how you allocate these protected buffers. We could add a "is > >> protected buffer" flag at the dma_buf level for this. > >> > >> So yeah for this stuff here I think we do want the full userspace side, > >> from allocator to rendering something into this protected buffers (no need > >> to also have the entire "decode a protected bitstream part" imo, since > >> that will freak people out). Unfortunately, in my experience, that kills > >> it for upstream :-/ But also in my experience of looking into this for > >> other gpu's, we really need to have the full picture here to make sure > >> we're not screwing this up. > > > > Maybe Ayan could've been a bit clearer in his message, but the ask here is > > for ideas > > on how userspace "communicates" (stores?) the fact that the buffers are > > protected to > > the kernel driver. In our display processor we need to the the hardware > > that the > > buffers are protected before it tries to fetch them so that it can 1) > > enable the > > additional hardware signaling that sets the protection around the stream; > > and 2) read > > the protected buffers in a special mode where there the magic happens. That was clear, but for the full picture we also need to know how these buffers are produced and where they are allocated. One approach would be to have a dma-buf heap that gives you encrypted
Re: [RFC PATCH] drm:- Add a modifier to denote 'protected' framebuffer
Hi, On 17/09/2019 18:07, Liviu Dudau wrote: > On Tue, Sep 17, 2019 at 02:53:01PM +0200, Daniel Vetter wrote: >> On Mon, Sep 09, 2019 at 01:42:53PM +, Ayan Halder wrote: >>> Add a modifier 'DRM_FORMAT_MOD_ARM_PROTECTED' which denotes that the >>> framebuffer >>> is allocated in a protected system memory. >>> Essentially, we want to support EGL_EXT_protected_content in our komeda >>> driver. >>> >>> Signed-off-by: Ayan Kumar Halder >>> >>> /-- Note to reviewer >>> Komeda driver is capable of rendering DRM (Digital Rights Management) >>> protected >>> content. The DRM content is stored in a framebuffer allocated in system >>> memory >>> (which needs some special hardware signals for access). >>> >>> Let us ignore how the protected system memory is allocated and for the >>> scope of >>> this discussion, we want to figure out the best way possible for the >>> userspace >>> to communicate to the drm driver to turn the protected mode on (for >>> accessing the >>> framebuffer with the DRM content) or off. >>> >>> The possible ways by which the userspace could achieve this is via:- >>> >>> 1. Modifiers :- This looks to me the best way by which the userspace can >>> communicate to the kernel to turn the protected mode on for the komeda >>> driver >>> as it is going to access one of the protected framebuffers. The only >>> problem is >>> that the current modifiers describe the tiling/compression format. However, >>> it >>> does not hurt to extend the meaning of modifiers to denote other attributes >>> of >>> the framebuffer as well. >>> >>> The other reason is that on Android, we get an info from Gralloc >>> (GRALLOC_USAGE_PROTECTED) which tells us that the buffer is protected. This >>> can >>> be used to set up the modifier/s (AddFB2) during framebuffer creation. >> >> How does this mesh with other modifiers, like AFBC? That's where I see the >> issue here. > > AFBC modifiers are currently under Arm's namespace, the thought behind the DRM > modifiers would be to have it as a "generic" modifier. > >>> >>> 2. Framebuffer flags :- As of today, this can be one of the two values >>> ie (DRM_MODE_FB_INTERLACED/DRM_MODE_FB_MODIFIERS). Unlike modifiers, the drm >>> framebuffer flags are generic to the drm subsystem and ideally we should not >>> introduce any driver specific constraint/feature. >>> >>> 3. Connector property:- I could see the following properties used for DRM >>> protected content:- >>> DRM_MODE_CONTENT_PROTECTION_DESIRED / ENABLED :- "This property is used by >>> userspace to request the kernel protect future content communicated over >>> the link". Clearly, we are not concerned with the protection attributes of >>> the >>> transmitter. So, we cannot use this property for our case. >>> >>> 4. DRM plane property:- Again, we want to communicate that the >>> framebuffer(which >>> can be attached to any plane) is protected. So introducing a new plane >>> property >>> does not help. >>> >>> 5. DRM crtc property:- For the same reason as above, introducing a new crtc >>> property does not help. >> >> 6. Just track this as part of buffer allocation, i.e. I think it does >> matter how you allocate these protected buffers. We could add a "is >> protected buffer" flag at the dma_buf level for this. >> >> So yeah for this stuff here I think we do want the full userspace side, >> from allocator to rendering something into this protected buffers (no need >> to also have the entire "decode a protected bitstream part" imo, since >> that will freak people out). Unfortunately, in my experience, that kills >> it for upstream :-/ But also in my experience of looking into this for >> other gpu's, we really need to have the full picture here to make sure >> we're not screwing this up. > > Maybe Ayan could've been a bit clearer in his message, but the ask here is > for ideas > on how userspace "communicates" (stores?) the fact that the buffers are > protected to > the kernel driver. In our display processor we need to the the hardware that > the > buffers are protected before it tries to fetch them so that it can 1) enable > the > additional hardware signaling that sets the protection around the stream; and > 2) read > the protected buffers in a special mode where there the magic happens. > > So yeah, we know we do want full userspace support, we're prodding the > community on > answers on how to best let the kernel side know what userspace has done. Actually this is interesting for other multimedia SoCs implementing secure video decode paths where video buffers are allocated and managed by a trusted app. Neil > > Best regards, > Liviu > > >> -Daniel >> >>> >>> --/ >>> >>> --- >>> include/uapi/drm/drm_fourcc.h | 9 + >>> 1 file changed, 9 insertions(+) >>> >>> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h >>> index 3feeaa3f987a..38e5e81d11fe 100644 >>> --- a/include/uapi/drm/drm_fourcc.h >>> +++ b/include/uapi/drm/drm_fourcc.h >>> @@ -742,6 +742,15 @@
Re: [RFC PATCH] drm:- Add a modifier to denote 'protected' framebuffer
On Tue, Sep 17, 2019 at 02:53:01PM +0200, Daniel Vetter wrote:
> On Mon, Sep 09, 2019 at 01:42:53PM +, Ayan Halder wrote:
> > Add a modifier 'DRM_FORMAT_MOD_ARM_PROTECTED' which denotes that the
> > framebuffer
> > is allocated in a protected system memory.
> > Essentially, we want to support EGL_EXT_protected_content in our komeda
> > driver.
> >
> > Signed-off-by: Ayan Kumar Halder
> >
> > /-- Note to reviewer
> > Komeda driver is capable of rendering DRM (Digital Rights Management)
> > protected
> > content. The DRM content is stored in a framebuffer allocated in system
> > memory
> > (which needs some special hardware signals for access).
> >
> > Let us ignore how the protected system memory is allocated and for the
> > scope of
> > this discussion, we want to figure out the best way possible for the
> > userspace
> > to communicate to the drm driver to turn the protected mode on (for
> > accessing the
> > framebuffer with the DRM content) or off.
> >
> > The possible ways by which the userspace could achieve this is via:-
> >
> > 1. Modifiers :- This looks to me the best way by which the userspace can
> > communicate to the kernel to turn the protected mode on for the komeda
> > driver
> > as it is going to access one of the protected framebuffers. The only
> > problem is
> > that the current modifiers describe the tiling/compression format. However,
> > it
> > does not hurt to extend the meaning of modifiers to denote other attributes
> > of
> > the framebuffer as well.
> >
> > The other reason is that on Android, we get an info from Gralloc
> > (GRALLOC_USAGE_PROTECTED) which tells us that the buffer is protected. This
> > can
> > be used to set up the modifier/s (AddFB2) during framebuffer creation.
>
> How does this mesh with other modifiers, like AFBC? That's where I see the
> issue here.
AFBC modifiers are currently under Arm's namespace, the thought behind the DRM
modifiers would be to have it as a "generic" modifier.
> >
> > 2. Framebuffer flags :- As of today, this can be one of the two values
> > ie (DRM_MODE_FB_INTERLACED/DRM_MODE_FB_MODIFIERS). Unlike modifiers, the drm
> > framebuffer flags are generic to the drm subsystem and ideally we should not
> > introduce any driver specific constraint/feature.
> >
> > 3. Connector property:- I could see the following properties used for DRM
> > protected content:-
> > DRM_MODE_CONTENT_PROTECTION_DESIRED / ENABLED :- "This property is used by
> > userspace to request the kernel protect future content communicated over
> > the link". Clearly, we are not concerned with the protection attributes of
> > the
> > transmitter. So, we cannot use this property for our case.
> >
> > 4. DRM plane property:- Again, we want to communicate that the
> > framebuffer(which
> > can be attached to any plane) is protected. So introducing a new plane
> > property
> > does not help.
> >
> > 5. DRM crtc property:- For the same reason as above, introducing a new crtc
> > property does not help.
>
> 6. Just track this as part of buffer allocation, i.e. I think it does
> matter how you allocate these protected buffers. We could add a "is
> protected buffer" flag at the dma_buf level for this.
>
> So yeah for this stuff here I think we do want the full userspace side,
> from allocator to rendering something into this protected buffers (no need
> to also have the entire "decode a protected bitstream part" imo, since
> that will freak people out). Unfortunately, in my experience, that kills
> it for upstream :-/ But also in my experience of looking into this for
> other gpu's, we really need to have the full picture here to make sure
> we're not screwing this up.
Maybe Ayan could've been a bit clearer in his message, but the ask here is for
ideas
on how userspace "communicates" (stores?) the fact that the buffers are
protected to
the kernel driver. In our display processor we need to the the hardware that the
buffers are protected before it tries to fetch them so that it can 1) enable the
additional hardware signaling that sets the protection around the stream; and
2) read
the protected buffers in a special mode where there the magic happens.
So yeah, we know we do want full userspace support, we're prodding the
community on
answers on how to best let the kernel side know what userspace has done.
Best regards,
Liviu
> -Daniel
>
> >
> > --/
> >
> > ---
> > include/uapi/drm/drm_fourcc.h | 9 +
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> > index 3feeaa3f987a..38e5e81d11fe 100644
> > --- a/include/uapi/drm/drm_fourcc.h
> > +++ b/include/uapi/drm/drm_fourcc.h
> > @@ -742,6 +742,15 @@ extern "C" {
> > */
> > #define AFBC_FORMAT_MOD_BCH (1ULL << 11)
> >
> > +/*
> > + * Protected framebuffer
> > + *
> > + * The framebuffer is allocated in a protected system memory which can be
> > accessed
> > + * via some special hardware signals from the
Re: [RFC PATCH] drm:- Add a modifier to denote 'protected' framebuffer
On Mon, Sep 09, 2019 at 01:42:53PM +, Ayan Halder wrote:
> Add a modifier 'DRM_FORMAT_MOD_ARM_PROTECTED' which denotes that the
> framebuffer
> is allocated in a protected system memory.
> Essentially, we want to support EGL_EXT_protected_content in our komeda
> driver.
>
> Signed-off-by: Ayan Kumar Halder
>
> /-- Note to reviewer
> Komeda driver is capable of rendering DRM (Digital Rights Management)
> protected
> content. The DRM content is stored in a framebuffer allocated in system memory
> (which needs some special hardware signals for access).
>
> Let us ignore how the protected system memory is allocated and for the scope
> of
> this discussion, we want to figure out the best way possible for the userspace
> to communicate to the drm driver to turn the protected mode on (for accessing
> the
> framebuffer with the DRM content) or off.
>
> The possible ways by which the userspace could achieve this is via:-
>
> 1. Modifiers :- This looks to me the best way by which the userspace can
> communicate to the kernel to turn the protected mode on for the komeda driver
> as it is going to access one of the protected framebuffers. The only problem
> is
> that the current modifiers describe the tiling/compression format. However, it
> does not hurt to extend the meaning of modifiers to denote other attributes of
> the framebuffer as well.
>
> The other reason is that on Android, we get an info from Gralloc
> (GRALLOC_USAGE_PROTECTED) which tells us that the buffer is protected. This
> can
> be used to set up the modifier/s (AddFB2) during framebuffer creation.
How does this mesh with other modifiers, like AFBC? That's where I see the
issue here.
>
> 2. Framebuffer flags :- As of today, this can be one of the two values
> ie (DRM_MODE_FB_INTERLACED/DRM_MODE_FB_MODIFIERS). Unlike modifiers, the drm
> framebuffer flags are generic to the drm subsystem and ideally we should not
> introduce any driver specific constraint/feature.
>
> 3. Connector property:- I could see the following properties used for DRM
> protected content:-
> DRM_MODE_CONTENT_PROTECTION_DESIRED / ENABLED :- "This property is used by
> userspace to request the kernel protect future content communicated over
> the link". Clearly, we are not concerned with the protection attributes of the
> transmitter. So, we cannot use this property for our case.
>
> 4. DRM plane property:- Again, we want to communicate that the
> framebuffer(which
> can be attached to any plane) is protected. So introducing a new plane
> property
> does not help.
>
> 5. DRM crtc property:- For the same reason as above, introducing a new crtc
> property does not help.
6. Just track this as part of buffer allocation, i.e. I think it does
matter how you allocate these protected buffers. We could add a "is
protected buffer" flag at the dma_buf level for this.
So yeah for this stuff here I think we do want the full userspace side,
from allocator to rendering something into this protected buffers (no need
to also have the entire "decode a protected bitstream part" imo, since
that will freak people out). Unfortunately, in my experience, that kills
it for upstream :-/ But also in my experience of looking into this for
other gpu's, we really need to have the full picture here to make sure
we're not screwing this up.
-Daniel
>
> --/
>
> ---
> include/uapi/drm/drm_fourcc.h | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 3feeaa3f987a..38e5e81d11fe 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -742,6 +742,15 @@ extern "C" {
> */
> #define AFBC_FORMAT_MOD_BCH (1ULL << 11)
>
> +/*
> + * Protected framebuffer
> + *
> + * The framebuffer is allocated in a protected system memory which can be
> accessed
> + * via some special hardware signals from the dpu. This is used to support
> + * 'GRALLOC_USAGE_PROTECTED' in our framebuffer for
> EGL_EXT_protected_content.
> + */
> +#define DRM_FORMAT_MOD_ARM_PROTECTED fourcc_mod_code(ARM, (1ULL << 55))
> +
> /*
> * Allwinner tiled modifier
> *
> --
> 2.23.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
