Re: [PATCH v4 7/8] drm/fourcc: amlogic: Add modifier definitions for the Scatter layout

2020-03-30 Thread Pekka Paalanen
On Fri, 27 Mar 2020 15:14:46 +0100
Neil Armstrong  wrote:

> Hi,
> 
> On 26/03/2020 10:36, Pekka Paalanen wrote:
> > On Wed, 25 Mar 2020 17:18:15 +0100
> > Neil Armstrong  wrote:
> >   
> >> Hi,
> >>
> >> On 25/03/2020 14:49, Pekka Paalanen wrote:  
> >>> On Wed, 25 Mar 2020 11:24:15 +0100
> >>> Neil Armstrong  wrote:
> >>> 
>  Hi,
> 
>  On 25/03/2020 10:04, Simon Ser wrote:
> > On Wednesday, March 25, 2020 9:50 AM, Neil Armstrong 
> >  wrote:
> >   
> >> Amlogic uses a proprietary lossless image compression protocol and 
> >> format
> >> for their hardware video codec accelerators, either video decoders or
> >> video input encoders.
> >>
> >> This introduces the Scatter Memory layout, means the header contains 
> >> IOMMU
> >> references to the compressed frames content to optimize memory access
> >> and layout.
> >>
> >> In this mode, only the header memory address is needed, thus the 
> >> content
> >> memory organization is tied to the current producer execution and 
> >> cannot
> >> be saved/dumped neither transferrable between Amlogic SoCs supporting 
> >> this
> >> modifier.  
> >
> > I don't think this is suitable for modifiers. User-space relies on
> > being able to copy a buffer from one machine to another over the
> > network. It would be pretty annoying for user-space to have a blacklist
> > of modifiers that don't work this way.
> >
> > Example of such user-space:
> > https://gitlab.freedesktop.org/mstoeckl/waypipe/
> >   
> 
>  I really understand your point, but this is one of the use-cases we need 
>  solve.
>  This is why I split the fourcc patch and added an explicit comment.
> 
>  Please point me a way to display such buffer, the HW exists, works like 
>  that and
>  it's a fact and can't change.
> 
>  It will be the same for secure zero-copy buffers we can't map from 
>  userspace, but
>  only the HW decoder can read/write and HW display can read.
> >>>
> >>> The comparison to secure buffers is a good one.
> >>>
> >>> Are buffers with the DRM_FORMAT_MOD_AMLOGIC_FBC_LAYOUT_SCATTER modifier
> >>> meaningfully mmappable to CPU always / sometimes / never /
> >>> varies-and-cannot-know?
> >>
> >> mmappable, yes in our WIP V4L2 driver in non-secure path, meaningful, 
> >> absolutely never.
> >>
> >> So yeah, these should not be mmappable since not meaningful.  
> > 
> > Ok. So we have a modifier that means there is no point in even trying to
> > mmap the buffer.
> > 
> > Not being able to mmap automatically makes things like waypipe not be
> > able to work on the buffer, so the buffer cannot be replicated over a
> > network, hence there is no compatibility issue. However, it still
> > leaves the problem that, since waypipe is "just" a message relay that
> > does not participate in the protocol really, the two end points might
> > still negotiate to use a modifier that waypipe cannot handle.  
> 
> Not mmapable won't be limited to this kind of buffer, or secure, any DMA-BUF
> provider can decide to disable mmaping, so waypipe should work with this
> whatever this discussion goes to.
> 
> > 
> > Secure buffers have the same problem: by definition, one must not be
> > able to replicate the buffer elsewhere.
> > 
> > To me it seems there needs to be a way to identify buffers that cannot
> > be mmapped. mmap() failing is obvious, but in waypipe's case it is too
> > late - the end points have already negotiated the formats and modifiers
> > and they cannot handle failures afterwards.  
> 
> The AFAIK last open question was on this thread:
> https://lore.kernel.org/dri-devel/d6f8092d-9f90-d5ff-2ab3-b1867f8f5...@ti.com/
> But it was more like, how the consumer driver knows the buffer is secure.
> 
> Daniel, is there something new ?
> 
> >   
> >>>
> >>> Maybe this type should be handled similar to secure buffers, with the
> >>> exception that they are not actually secured but only mostly
> >>> inaccessible. Then again, I haven't looked at any of the secure buffer
> >>> proposals.
> >>
> >> Actually, the Amlogic platforms offers secure video path using these exact
> >> modifiers, AFAIK it doesn't support the NV12 dual-write output in secure.
> >>
> >> AFAIK last submission is from AMD, and it doesn't talk at all about 
> >> mmapability
> >> of the secure BOs.  
> > 
> > To me, a secure buffer concept automatically implies that there cannot
> > be CPU access to it. The CPU is not trusted, right? Not even the kernel.
> > I would assume secure implies no mmap. So I wonder, how does the secure
> > buffers proposal manage userspace like waypipe?  
> 
> None, as I said, waypipe whould handle non mmapable buffers, by asking
> for a different modifier set, or sending a gray buffer with a llama
> instead.

Hi,

the only thing waypipe can do, is not forward some of the modifiers
during negotiation, before a

Re: [PATCH v4 7/8] drm/fourcc: amlogic: Add modifier definitions for the Scatter layout

2020-03-27 Thread Neil Armstrong
Hi,

On 26/03/2020 10:36, Pekka Paalanen wrote:
> On Wed, 25 Mar 2020 17:18:15 +0100
> Neil Armstrong  wrote:
> 
>> Hi,
>>
>> On 25/03/2020 14:49, Pekka Paalanen wrote:
>>> On Wed, 25 Mar 2020 11:24:15 +0100
>>> Neil Armstrong  wrote:
>>>   
 Hi,

 On 25/03/2020 10:04, Simon Ser wrote:  
> On Wednesday, March 25, 2020 9:50 AM, Neil Armstrong 
>  wrote:
> 
>> Amlogic uses a proprietary lossless image compression protocol and format
>> for their hardware video codec accelerators, either video decoders or
>> video input encoders.
>>
>> This introduces the Scatter Memory layout, means the header contains 
>> IOMMU
>> references to the compressed frames content to optimize memory access
>> and layout.
>>
>> In this mode, only the header memory address is needed, thus the content
>> memory organization is tied to the current producer execution and cannot
>> be saved/dumped neither transferrable between Amlogic SoCs supporting 
>> this
>> modifier.
>
> I don't think this is suitable for modifiers. User-space relies on
> being able to copy a buffer from one machine to another over the
> network. It would be pretty annoying for user-space to have a blacklist
> of modifiers that don't work this way.
>
> Example of such user-space:
> https://gitlab.freedesktop.org/mstoeckl/waypipe/
> 

 I really understand your point, but this is one of the use-cases we need 
 solve.
 This is why I split the fourcc patch and added an explicit comment.

 Please point me a way to display such buffer, the HW exists, works like 
 that and
 it's a fact and can't change.

 It will be the same for secure zero-copy buffers we can't map from 
 userspace, but
 only the HW decoder can read/write and HW display can read.  
>>>
>>> The comparison to secure buffers is a good one.
>>>
>>> Are buffers with the DRM_FORMAT_MOD_AMLOGIC_FBC_LAYOUT_SCATTER modifier
>>> meaningfully mmappable to CPU always / sometimes / never /
>>> varies-and-cannot-know?  
>>
>> mmappable, yes in our WIP V4L2 driver in non-secure path, meaningful, 
>> absolutely never.
>>
>> So yeah, these should not be mmappable since not meaningful.
> 
> Ok. So we have a modifier that means there is no point in even trying to
> mmap the buffer.
> 
> Not being able to mmap automatically makes things like waypipe not be
> able to work on the buffer, so the buffer cannot be replicated over a
> network, hence there is no compatibility issue. However, it still
> leaves the problem that, since waypipe is "just" a message relay that
> does not participate in the protocol really, the two end points might
> still negotiate to use a modifier that waypipe cannot handle.

Not mmapable won't be limited to this kind of buffer, or secure, any DMA-BUF
provider can decide to disable mmaping, so waypipe should work with this
whatever this discussion goes to.

> 
> Secure buffers have the same problem: by definition, one must not be
> able to replicate the buffer elsewhere.
> 
> To me it seems there needs to be a way to identify buffers that cannot
> be mmapped. mmap() failing is obvious, but in waypipe's case it is too
> late - the end points have already negotiated the formats and modifiers
> and they cannot handle failures afterwards.

The AFAIK last open question was on this thread:
https://lore.kernel.org/dri-devel/d6f8092d-9f90-d5ff-2ab3-b1867f8f5...@ti.com/
But it was more like, how the consumer driver knows the buffer is secure.

Daniel, is there something new ?

> 
>>>
>>> Maybe this type should be handled similar to secure buffers, with the
>>> exception that they are not actually secured but only mostly
>>> inaccessible. Then again, I haven't looked at any of the secure buffer
>>> proposals.  
>>
>> Actually, the Amlogic platforms offers secure video path using these exact
>> modifiers, AFAIK it doesn't support the NV12 dual-write output in secure.
>>
>> AFAIK last submission is from AMD, and it doesn't talk at all about 
>> mmapability
>> of the secure BOs.
> 
> To me, a secure buffer concept automatically implies that there cannot
> be CPU access to it. The CPU is not trusted, right? Not even the kernel.
> I would assume secure implies no mmap. So I wonder, how does the secure
> buffers proposal manage userspace like waypipe?

None, as I said, waypipe whould handle non mmapable buffers, by asking
for a different modifier set, or sending a gray buffer with a llama
instead.

> 
> Or, is the secure buffer proposal allowing mmap, but the content is
> indecipherable? Maybe they shouldn't allow mmap?

Definitely, you'll have an HW bus error if you access a secure buffer,
otherwise the security is weak. A bus firewall is the common way to handle
such secure buffers.

> 
> I think much of the criticism against this modifier should also be
> presented to a secure buffers proposal and see how that turns out. If
> they have the s

Re: [PATCH v4 7/8] drm/fourcc: amlogic: Add modifier definitions for the Scatter layout

2020-03-26 Thread Pekka Paalanen
On Wed, 25 Mar 2020 17:18:15 +0100
Neil Armstrong  wrote:

> Hi,
> 
> On 25/03/2020 14:49, Pekka Paalanen wrote:
> > On Wed, 25 Mar 2020 11:24:15 +0100
> > Neil Armstrong  wrote:
> >   
> >> Hi,
> >>
> >> On 25/03/2020 10:04, Simon Ser wrote:  
> >>> On Wednesday, March 25, 2020 9:50 AM, Neil Armstrong 
> >>>  wrote:
> >>> 
>  Amlogic uses a proprietary lossless image compression protocol and format
>  for their hardware video codec accelerators, either video decoders or
>  video input encoders.
> 
>  This introduces the Scatter Memory layout, means the header contains 
>  IOMMU
>  references to the compressed frames content to optimize memory access
>  and layout.
> 
>  In this mode, only the header memory address is needed, thus the content
>  memory organization is tied to the current producer execution and cannot
>  be saved/dumped neither transferrable between Amlogic SoCs supporting 
>  this
>  modifier.
> >>>
> >>> I don't think this is suitable for modifiers. User-space relies on
> >>> being able to copy a buffer from one machine to another over the
> >>> network. It would be pretty annoying for user-space to have a blacklist
> >>> of modifiers that don't work this way.
> >>>
> >>> Example of such user-space:
> >>> https://gitlab.freedesktop.org/mstoeckl/waypipe/
> >>> 
> >>
> >> I really understand your point, but this is one of the use-cases we need 
> >> solve.
> >> This is why I split the fourcc patch and added an explicit comment.
> >>
> >> Please point me a way to display such buffer, the HW exists, works like 
> >> that and
> >> it's a fact and can't change.
> >>
> >> It will be the same for secure zero-copy buffers we can't map from 
> >> userspace, but
> >> only the HW decoder can read/write and HW display can read.  
> > 
> > The comparison to secure buffers is a good one.
> > 
> > Are buffers with the DRM_FORMAT_MOD_AMLOGIC_FBC_LAYOUT_SCATTER modifier
> > meaningfully mmappable to CPU always / sometimes / never /
> > varies-and-cannot-know?  
> 
> mmappable, yes in our WIP V4L2 driver in non-secure path, meaningful, 
> absolutely never.
> 
> So yeah, these should not be mmappable since not meaningful.

Ok. So we have a modifier that means there is no point in even trying to
mmap the buffer.

Not being able to mmap automatically makes things like waypipe not be
able to work on the buffer, so the buffer cannot be replicated over a
network, hence there is no compatibility issue. However, it still
leaves the problem that, since waypipe is "just" a message relay that
does not participate in the protocol really, the two end points might
still negotiate to use a modifier that waypipe cannot handle.

Secure buffers have the same problem: by definition, one must not be
able to replicate the buffer elsewhere.

To me it seems there needs to be a way to identify buffers that cannot
be mmapped. mmap() failing is obvious, but in waypipe's case it is too
late - the end points have already negotiated the formats and modifiers
and they cannot handle failures afterwards.

> > 
> > Maybe this type should be handled similar to secure buffers, with the
> > exception that they are not actually secured but only mostly
> > inaccessible. Then again, I haven't looked at any of the secure buffer
> > proposals.  
> 
> Actually, the Amlogic platforms offers secure video path using these exact
> modifiers, AFAIK it doesn't support the NV12 dual-write output in secure.
> 
> AFAIK last submission is from AMD, and it doesn't talk at all about 
> mmapability
> of the secure BOs.

To me, a secure buffer concept automatically implies that there cannot
be CPU access to it. The CPU is not trusted, right? Not even the kernel.
I would assume secure implies no mmap. So I wonder, how does the secure
buffers proposal manage userspace like waypipe?

Or, is the secure buffer proposal allowing mmap, but the content is
indecipherable? Maybe they shouldn't allow mmap?

I think much of the criticism against this modifier should also be
presented to a secure buffers proposal and see how that turns out. If
they have the same problem, maybe you could use their solution?


Thanks,
pq


pgpa3w4_QFMqi.pgp
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 7/8] drm/fourcc: amlogic: Add modifier definitions for the Scatter layout

2020-03-25 Thread Neil Armstrong
Hi,

On 25/03/2020 14:49, Pekka Paalanen wrote:
> On Wed, 25 Mar 2020 11:24:15 +0100
> Neil Armstrong  wrote:
> 
>> Hi,
>>
>> On 25/03/2020 10:04, Simon Ser wrote:
>>> On Wednesday, March 25, 2020 9:50 AM, Neil Armstrong 
>>>  wrote:
>>>   
 Amlogic uses a proprietary lossless image compression protocol and format
 for their hardware video codec accelerators, either video decoders or
 video input encoders.

 This introduces the Scatter Memory layout, means the header contains IOMMU
 references to the compressed frames content to optimize memory access
 and layout.

 In this mode, only the header memory address is needed, thus the content
 memory organization is tied to the current producer execution and cannot
 be saved/dumped neither transferrable between Amlogic SoCs supporting this
 modifier.  
>>>
>>> I don't think this is suitable for modifiers. User-space relies on
>>> being able to copy a buffer from one machine to another over the
>>> network. It would be pretty annoying for user-space to have a blacklist
>>> of modifiers that don't work this way.
>>>
>>> Example of such user-space:
>>> https://gitlab.freedesktop.org/mstoeckl/waypipe/
>>>   
>>
>> I really understand your point, but this is one of the use-cases we need 
>> solve.
>> This is why I split the fourcc patch and added an explicit comment.
>>
>> Please point me a way to display such buffer, the HW exists, works like that 
>> and
>> it's a fact and can't change.
>>
>> It will be the same for secure zero-copy buffers we can't map from 
>> userspace, but
>> only the HW decoder can read/write and HW display can read.
> 
> The comparison to secure buffers is a good one.
> 
> Are buffers with the DRM_FORMAT_MOD_AMLOGIC_FBC_LAYOUT_SCATTER modifier
> meaningfully mmappable to CPU always / sometimes / never /
> varies-and-cannot-know?

mmappable, yes in our WIP V4L2 driver in non-secure path, meaningful, 
absolutely never.

So yeah, these should not be mmappable since not meaningful.

> 
> Maybe this type should be handled similar to secure buffers, with the
> exception that they are not actually secured but only mostly
> inaccessible. Then again, I haven't looked at any of the secure buffer
> proposals.

Actually, the Amlogic platforms offers secure video path using these exact
modifiers, AFAIK it doesn't support the NV12 dual-write output in secure.

AFAIK last submission is from AMD, and it doesn't talk at all about mmapability
of the secure BOs.

Neil

> 
> 
> Thanks,
> pq
> 




signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 7/8] drm/fourcc: amlogic: Add modifier definitions for the Scatter layout

2020-03-25 Thread Pekka Paalanen
On Wed, 25 Mar 2020 11:24:15 +0100
Neil Armstrong  wrote:

> Hi,
> 
> On 25/03/2020 10:04, Simon Ser wrote:
> > On Wednesday, March 25, 2020 9:50 AM, Neil Armstrong 
> >  wrote:
> >   
> >> Amlogic uses a proprietary lossless image compression protocol and format
> >> for their hardware video codec accelerators, either video decoders or
> >> video input encoders.
> >>
> >> This introduces the Scatter Memory layout, means the header contains IOMMU
> >> references to the compressed frames content to optimize memory access
> >> and layout.
> >>
> >> In this mode, only the header memory address is needed, thus the content
> >> memory organization is tied to the current producer execution and cannot
> >> be saved/dumped neither transferrable between Amlogic SoCs supporting this
> >> modifier.  
> > 
> > I don't think this is suitable for modifiers. User-space relies on
> > being able to copy a buffer from one machine to another over the
> > network. It would be pretty annoying for user-space to have a blacklist
> > of modifiers that don't work this way.
> > 
> > Example of such user-space:
> > https://gitlab.freedesktop.org/mstoeckl/waypipe/
> >   
> 
> I really understand your point, but this is one of the use-cases we need 
> solve.
> This is why I split the fourcc patch and added an explicit comment.
> 
> Please point me a way to display such buffer, the HW exists, works like that 
> and
> it's a fact and can't change.
> 
> It will be the same for secure zero-copy buffers we can't map from userspace, 
> but
> only the HW decoder can read/write and HW display can read.

The comparison to secure buffers is a good one.

Are buffers with the DRM_FORMAT_MOD_AMLOGIC_FBC_LAYOUT_SCATTER modifier
meaningfully mmappable to CPU always / sometimes / never /
varies-and-cannot-know?

Maybe this type should be handled similar to secure buffers, with the
exception that they are not actually secured but only mostly
inaccessible. Then again, I haven't looked at any of the secure buffer
proposals.


Thanks,
pq


pgpV70bI_Pfsv.pgp
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 7/8] drm/fourcc: amlogic: Add modifier definitions for the Scatter layout

2020-03-25 Thread Neil Armstrong
Hi,

On 25/03/2020 10:04, Simon Ser wrote:
> On Wednesday, March 25, 2020 9:50 AM, Neil Armstrong 
>  wrote:
> 
>> Amlogic uses a proprietary lossless image compression protocol and format
>> for their hardware video codec accelerators, either video decoders or
>> video input encoders.
>>
>> This introduces the Scatter Memory layout, means the header contains IOMMU
>> references to the compressed frames content to optimize memory access
>> and layout.
>>
>> In this mode, only the header memory address is needed, thus the content
>> memory organization is tied to the current producer execution and cannot
>> be saved/dumped neither transferrable between Amlogic SoCs supporting this
>> modifier.
> 
> I don't think this is suitable for modifiers. User-space relies on
> being able to copy a buffer from one machine to another over the
> network. It would be pretty annoying for user-space to have a blacklist
> of modifiers that don't work this way.
> 
> Example of such user-space:
> https://gitlab.freedesktop.org/mstoeckl/waypipe/
> 

I really understand your point, but this is one of the use-cases we need solve.
This is why I split the fourcc patch and added an explicit comment.

Please point me a way to display such buffer, the HW exists, works like that and
it's a fact and can't change.

It will be the same for secure zero-copy buffers we can't map from userspace, 
but
only the HW decoder can read/write and HW display can read.

We need a solution for those if we want embedded and secure products to be 
supported
upstream, otherwise they will stay in an obscure off-tree linux tree and for 
example
AOSP support (which will support these secure video buffers) will use these 
vendor
specific hacks.

Neil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 7/8] drm/fourcc: amlogic: Add modifier definitions for the Scatter layout

2020-03-25 Thread Simon Ser
On Wednesday, March 25, 2020 9:50 AM, Neil Armstrong  
wrote:

> Amlogic uses a proprietary lossless image compression protocol and format
> for their hardware video codec accelerators, either video decoders or
> video input encoders.
>
> This introduces the Scatter Memory layout, means the header contains IOMMU
> references to the compressed frames content to optimize memory access
> and layout.
>
> In this mode, only the header memory address is needed, thus the content
> memory organization is tied to the current producer execution and cannot
> be saved/dumped neither transferrable between Amlogic SoCs supporting this
> modifier.

I don't think this is suitable for modifiers. User-space relies on
being able to copy a buffer from one machine to another over the
network. It would be pretty annoying for user-space to have a blacklist
of modifiers that don't work this way.

Example of such user-space:
https://gitlab.freedesktop.org/mstoeckl/waypipe/
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel