Message-ID: <32f2d748c6926f57be032c60cecfdc790ea2c1c0.camel@ndufresne.ca>
Date: Fri, 28 Jun 2024 16:34:03 -0400
From: Nicolas Dufresne <nicolas@...fresne.ca>
To: Thierry Reding <thierry.reding@...il.com>, "mripard@...nel.org"
	 <mripard@...nel.org>
Cc: Christian König <christian.koenig@....com>,  Jason-JH
 Lin (林睿祥)
 <Jason-JH.Lin@...iatek.com>, "daniel@...ll.ch" <daniel@...ll.ch>, 
 "quic_vjitta@...cinc.com" <quic_vjitta@...cinc.com>,
 "angelogioacchino.delregno@...labora.com"
 <angelogioacchino.delregno@...labora.com>, "sumit.semwal@...aro.org"
 <sumit.semwal@...aro.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>, 
 "jkardatzke@...gle.com" <jkardatzke@...gle.com>,
 "krzysztof.kozlowski+dt@...aro.org" <krzysztof.kozlowski+dt@...aro.org>,
 "joakim.bech@...aro.org" <joakim.bech@...aro.org>, Youlin Pei
 (裴友林) <youlin.pei@...iatek.com>,
 "logang@...tatee.com" <logang@...tatee.com>, 
 "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
 Kuohong Wang (王國鴻)
 <kuohong.wang@...iatek.com>, Jianjiao Zeng
 (曾健姣) <Jianjiao.Zeng@...iatek.com>,
 "contact@...rsion.fr" <contact@...rsion.fr>, 
 "benjamin.gaignard@...labora.com" <benjamin.gaignard@...labora.com>,
 "matthias.bgg@...il.com" <matthias.bgg@...il.com>,
 "linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
 "linaro-mm-sig@...ts.linaro.org" <linaro-mm-sig@...ts.linaro.org>,
 "willy@...radead.org" <willy@...radead.org>,  "pavel@....cz"
 <pavel@....cz>, "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>, 
 "Brian.Starkey@....com" <Brian.Starkey@....com>, "robh+dt@...nel.org"
 <robh+dt@...nel.org>,  "linux-media@...r.kernel.org"
 <linux-media@...r.kernel.org>, "devicetree@...r.kernel.org"
 <devicetree@...r.kernel.org>, "tjmercier@...gle.com"
 <tjmercier@...gle.com>,  "jstultz@...gle.com" <jstultz@...gle.com>,
 "linux-arm-kernel@...ts.infradead.org"
 <linux-arm-kernel@...ts.infradead.org>, "robin.murphy@....com"
 <robin.murphy@....com>, Yong Wu (吴勇)
 <Yong.Wu@...iatek.com>, "linux-kernel@...r.kernel.org"
 <linux-kernel@...r.kernel.org>, "ppaalanen@...il.com" <ppaalanen@...il.com>
Subject: Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory

Hi Thierry,

Le vendredi 28 juin 2024 à 16:11 +0200, Thierry Reding a écrit :
> On Fri, Jun 28, 2024 at 03:21:51PM GMT, mripard@...nel.org wrote:
> > On Fri, Jun 28, 2024 at 01:47:01PM GMT, Thierry Reding wrote:
> > > On Thu, Jun 27, 2024 at 04:40:02PM GMT, mripard@...nel.org wrote:
> > > > On Thu, Jun 27, 2024 at 08:57:40AM GMT, Christian König wrote:
> > > > > Am 27.06.24 um 05:21 schrieb Jason-JH Lin (林睿祥):
> > > > > > 
> > > > > > On Wed, 2024-06-26 at 19:56 +0200, Daniel Vetter wrote:
> > > > > > >   > External email : Please do not click links or open attachments
> > > > > > until
> > > > > > > you have verified the sender or the content.
> > > > > > >  On Wed, Jun 26, 2024 at 12:49:02PM +0200, Christian König wrote:
> > > > > > > > Am 26.06.24 um 10:05 schrieb Jason-JH Lin (林睿祥):
> > > > > > > > > > > I think I have the same problem as the ECC_FLAG mention in:
> > > > > > > > > > > > > > https://lore.kernel.org/linux-media/20240515-dma-buf-ecc-heap-v1-0-54cbbd049511@kernel.org/
> > > > > > > > > > > > > I think it would be better to have the user configurable
> > > > > > > private
> > > > > > > > > > > information in dma-buf, so all the drivers who have the same
> > > > > > > > > > > requirement can get their private information from dma-buf
> > > > > > > directly
> > > > > > > > > > > and
> > > > > > > > > > > no need to change or add the interface.
> > > > > > > > > > > > > What's your opinion in this point?
> > > > > > > > > >  > Well of hand I don't see the need for that.
> > > > > > > > > > > What happens if you get a non-secure buffer imported in your
> > > > > > > secure
> > > > > > > > > > device?
> > > > > > > > > > > > We use the same mediatek-drm driver for secure and
> > > > > > non-secure
> > > > > > > buffer.
> > > > > > > > > If non-secure buffer imported to mediatek-drm driver, it's go to
> > > > > > > the
> > > > > > > > > normal flow with normal hardware settings.
> > > > > > > > > > > > We use different configurations to make hardware have
> > > > > > different
> > > > > > > > > permission to access the buffer it should access.
> > > > > > > > > > > > So if we can't get the information of "the buffer is
> > > > > > allocated
> > > > > > > from
> > > > > > > > > restricted_mtk_cma" when importing the buffer into the driver, we
> > > > > > > won't
> > > > > > > > > be able to configure the hardware correctly.
> > > > > > > > > > Why can't you get this information from userspace?
> > > > > > > > Same reason amd and i915/xe also pass this around internally in the
> > > > > > > kernel, it's just that for those gpus the render and kms node are the
> > > > > > > same
> > > > > > > driver so this is easy.
> > > > > > > 
> > > > > 
> > > > > The reason I ask is that encryption here looks just like another parameter
> > > > > for the buffer, e.g. like format, stride, tilling etc..
> > > > > 
> > > > > So instead of this during buffer import:
> > > > > 
> > > > > mtk_gem->secure = (!strncmp(attach->dmabuf->exp_name, "restricted", 10));
> > > > > mtk_gem->dma_addr = sg_dma_address(sg->sgl);
> > > > > mtk_gem->size = attach->dmabuf->size;
> > > > > mtk_gem->sg = sg;
> > > > > 
> > > > > You can trivially say during use hey this buffer is encrypted.
> > > > > 
> > > > > At least that's my 10 mile high view, maybe I'm missing some extensive key
> > > > > exchange or something like that.
> > > > 
> > > > That doesn't work in all cases, unfortunately.
> > > > 
> > > > If you're doing secure video playback, the firmware is typically in
> > > > charge of the frame decryption/decoding, and you'd get dma-buf back that
> > > > aren't accessible by the CPU (or at least, not at the execution level
> > > > Linux runs with).
> > > 
> > > Can you clarify which firmware you're talking about? Is this secure
> > > firmware, or firmware running on the video decoding hardware?
> > 
> > Secure firmware
> 
> Ah... interesting. So you actually need to interop with that firmware in
> order to start decryption/decoding. That's quite different from how this
> works on Tegra. Well, maybe not entirely. For Tegra there is firmware
> that runs on the hardware decoder and which has access to the keys, so
> in that way I guess it's similar to your use-case, except the firmware
> runs on a different chip.

That is something interesting for the linux-media discussions too. So in one
case, you have a seperate TF-A in the secure firmware following the CDM
specification, and it gives you back a restricted bitstream buffer. You then
don't need any CDM specific session/key information into the CODEC driver.

But in the case of Tegra, it would mean the CODEC driver is not agnostic to the
CDM, so we can expect (if this endup as a V4L2 driver) some controls for
Widewine, Playready and other CDM ? (adding explicit CDM API in the kernel is a
hot potato imho, I myself would try and stay away from that at all cost, and
focus on restricted storage feature only).

Nicolas


> > > > So nobody can map that buffer, and the firmware driver is the one who
> > > > knows that this buffer cannot be accessed by anyone. Putting this on the
> > > > userspace to know would be pretty weird, and wouldn't solve the case
> > > > where the kernel would try to map it.
> > > 
> > > Doesn't userspace need to know from the start whether it's trying to do
> > > secure playback or not?
> > 
> > It does, but it won't know the capabilities of the buffer it gets back
> > from the secure firmware.
> 
> I think that's kind of the point. Does it really have to know the
> capabilities? Isn't it enough to know that it's got some sort of
> protected buffer back and then use it more or less blindly? I mean
> these are things that have to be tightly coupled no matter what, so
> how much point is there in trying to validate what you get?
> 
> > > Typically this involves more than just the decoding part. You'd
> > > typically set up things like HDCP as part of the process, so userspace
> > > probably already does know that the buffers being passed around are
> > > protected.
> > > 
> > > Also, the kernel shouldn't really be mapping these buffers unless
> > > explicitly told to. In most cases you also wouldn't want the kernel to
> > > map these kinds of buffers, right? Are there any specific cases where
> > > you expect the kernel to need to map these?
> > > 
> > > I've been looking at this on the Tegra side recently and the way it
> > > works on these chips is that you basically get an opaque carveout region
> > > that has been locked down by secure firmware or early bootloaders, so
> > > only certain hardware blocks can access it. We can allocate from that
> > > carveout and then pass the buffers around.
> > 
> > So you allocate both the input and output buffers (and from different
> > regions) from the application, and pass both to the secure firmware?
> > 
> > Yeah, I guess that would work then.
> 
> It doesn't really matter who allocates the buffers. It could be the
> application allocating the scanout buffer from a DRM/KMS device and the
> input buffer from the multimedia decoder. Or it could be the application
> allocating both buffers from different DMA-BUF heaps. In the end it
> shouldn't really matter where they are coming from. It's effectively up
> to the application to pass the right buffers into the right IOCTLs.
> 
> > > It may be possible to use these protected carveout regions exclusively
> > > from the DRM/KMS driver and share them with multimedia engines via DMA-
> > > BUF, but I've also been looking into perhaps using DMA-BUF heaps to
> > > expose the carveout, which would make this a bit more flexible and allow
> > > either userspace to allocate the buffers or have multiple kernel drivers
> > > share the carveout via the DMA-BUF heap. Though the latter would require
> > > that there be in-kernel APIs for heaps, so not too sure about that yet.
> > 
> > What would be the advantage of using a heap compared to having all these
> > devices in DT use the reserved-memory property and point to that
> > carveout? It should already work today.
> 
> You can't just have all of these point to a common reserved-memory node
> because there can be multiple concurrent users. You could have multiple
> protected streams running at the same time. DMA-BUF heaps allows us to
> expose a central provider for the protected memory so that allocations
> can be properly arbitrated.
> 
> Thierry

Powered by blists - more mailing lists