Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
Am 28.06.24 um 15:40 schrieb mrip...@kernel.org: [SNIP] So nobody can map that buffer, and the firmware driver is the one who knows that this buffer cannot be accessed by anyone. On most hw I know you can actually map that buffer, it's just that the CPU sees only garbage in it because you don't have the necessary decryption keys around. So you can always map and access the buffer, but only if you're in the right "context" the content would be correct? Exactly that, yes. You need to have access to the decryption keys. I think that part is pretty different than what ARM SoCs are doing, where they would typically prevent any CPU access and fault on access. Yeah, that's indeed an important difference Nicolas noted as well. 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. But that's exactly how all other implementations work as far as I know. I mean what do you do if the kernel maps the encrypted buffer? On AMD we also block userspace and kernel CPU accesses, but that is only done to make it easier to find bugs not for correctness. And userspace absolutely needs to be aware that a buffer is encrypted, cause otherwise it could potentially try to access it with the CPU. I absolutely agree. I guess our discussion is whether it's something that should be implicit and understood by applications, or if it should be explicit and discoverable. Oh good point as well. But I think that's more a question for the userspace stack design. E.g. it can be handled explicitly by the application itself or implicitly by some V4L or VA-API library or something similar. For the kernel UAPI design we agreed at some point that we don't want to have any implicit properties on the DMA-buf which are carried around by the kernel, e.g. the whole format metadata for example. One notable exception to this is the actual hw topology, e.g. when for example a device has a special interconnect to another device and you need to make sure that the devices are powered up and down in a specific order to make things work. This should then made known to the core kernel using device link structure. E.g. similar to how it's used between GPU and HDMI audio block for example. Regards, Christian. Maxime
Re: [Linaro-mm-sig] Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
Am 28.06.24 um 22:16 schrieb Nicolas Dufresne: [SNIP] 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.. I'm mostly a reader of the thread here, but I'd like to avoid basic mistakes. The buffer in question are "protected", meaning that the CPU HW does not have access to the underlying pages (or zone in the case of Meditatek). This is different from encrypted buffers, which don't need this level of protection, as without the security key to decrypt them, their content is close to random data. Thanks for that clarification, this difference was absolutely not obvious. In that case having a separate heap for this memory is indeed the easiest approach. My question is still what would happen if the CPU tries to access this protected buffer? Or does the CPU not even have an address to do that? Just out of curiosity, I mean the exporting heap should then somehow reject any attempt to mmap() or vmap() the buffer content. Thanks, Christian.
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, mrip...@kernel.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, mrip...@kernel.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-54cbbd049...@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
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
Hi, Le jeudi 27 juin 2024 à 16:40 +0200, mrip...@kernel.org a écrit : > > 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). > > 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. Userspace will be the one calling into the CDM TF-A to get the bitstream buffer to be decrypted, not the firmware. The encrypted buffers are not using restricted memory. Userspace is also responsible for calling into the MTK restricted heap to allocate the destination buffer (on other platform it could be CMA heaps + TF-A call to restrict the allocated memory, I've seen some discussions related to this, but its not possible on Mediatek HW). I think its fair to assume that userspace always know which buffers are restricted or not in the SVP process. Nicolas
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
Hi Christian, Le jeudi 27 juin 2024 à 08:57 +0200, Christian König a écrit : > 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-54cbbd049...@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.. I'm mostly a reader of the thread here, but I'd like to avoid basic mistakes. The buffer in question are "protected", meaning that the CPU HW does not have access to the underlying pages (or zone in the case of Meditatek). This is different from encrypted buffers, which don't need this level of protection, as without the security key to decrypt them, their content is close to random data. > > 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. If we take secure video path as an example, in the context of digital right management, the handling of user session, retrieval of the device specific "key" is entirely something for userspace to handle and the kernel have no business into that. As long as the data is encrypted, its safe to carry around like any other buffers. It is only once decryption (usally done by a TF-A) that restricted memory start being used. Initially in the form of a compressed video stream, and eventually in the format of raw images. > > > > > But on arm you have split designs everywhere and dma-buf > > > import/export, so > > > something else is needed. And neither current kms uapi nor > > > protocols/extensions have provisions for this (afaik) because it > > > works on > > > the big gpus, and on android it's just hacked up with backchannels. > > > > > > So yeah essentially I think we probably need something like this, as > > > much > > > as it sucks. I see it somewhat similar to handling pcip2pdma > > > limitations > > > in the kernel too. > > > > > > Not sure where/how it should be handled though, and maybe I've missed > > > something around protocols, in which case I guess we should add some > > > secure buffer flags to the ADDFB2 ioctl. > > > > Thanks for your hint, I'll try to add the secure flag to the ADDFB2 > > ioctl. If it works, I'll send the patch. > > Yeah, exactly what I would suggest as well. > > I'm not an expert for that part, but as far as I know we already have > bunch of device specific tilling flags in there. > > Adding an MTK_ENCRYPTED flag should be trivial. Just to be clear, my comment was just a concept correction, I also think its nice to give a ADDFB2 flag a try, from my userspace experience, this is an easy place to provide this type of information. Also, the V4L2 proposal for the same endup with a flag at during buffer queue configuration, which is pretty close. Nicolas > > Regards, > Christian. > > > > > R
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
On Fri, Jun 28, 2024 at 03:57:49PM +0200, Thierry Reding wrote: > On Fri, Jun 28, 2024 at 02:34:24PM GMT, Christian König wrote: > > Am 28.06.24 um 13:47 schrieb Thierry Reding: > > > [SNIP] > > > > > 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? > > > > > > > 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? 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. > > > > > > 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. > > > > Yeah as far as I can see that would be a perfectly valid use case for > > DMA-Buf heaps. > > > > One question here: How does the HDCP setup work on Tegra? From your comment > > I guess you pass most of the information through userspace as well. > > Well, we don't currently support HDCP at all. I do have proof-of-concept > patches from a long time ago and, yes, judging by that we'd need to > control all of this from userspace. The way I imagine that this would > work is that userspace needs to first set the "Content Protection" and > "HDCP Content Type" properties and wait for the state change. Once that > has happened it can go and allocate the protected memory and start > decoding into it and then scan out from these buffers. Yeah this is how this is meant to work, because some userspace is happy with just hdcp, while the buffers are not encrypted/protected, the only protection is system lockdown with secure boot. So enable hdcp first, then allocate secure buffers and display them there. And the hardware needs to make sure that if we ever drop hcpd, the screen goes black (or encrypted garbage) when trying to display encrypted/secure buffers. > > Or is there any info inside the DMA-buf for this? In other words would you > > also need to know if a buffer is then allocated from this special carveout? > > I don't think so. It's possible to scan out an unprotected buffer with > HDCP enabled. It may also be possible to scan out a protected buffer > even if HDCP wasn't enabled, though you would obviously want to prevent > that somehow. Not sure if there's a common way to do this, but I guess > in end-user devices you'd need a fully trusted boot chain to do that in > a compliant way. Where I know how it works it's hardware making that guarantee. > It's been a long time since I looke
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
On Fri, Jun 28, 2024 at 03:21:51PM GMT, mrip...@kernel.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, mrip...@kernel.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-54cbbd049...@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. > > > 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
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
On Fri, Jun 28, 2024 at 02:34:24PM GMT, Christian König wrote: > Am 28.06.24 um 13:47 schrieb Thierry Reding: > > [SNIP] > > > > 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? > > > > > 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? 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. > > > > 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. > > Yeah as far as I can see that would be a perfectly valid use case for > DMA-Buf heaps. > > One question here: How does the HDCP setup work on Tegra? From your comment > I guess you pass most of the information through userspace as well. Well, we don't currently support HDCP at all. I do have proof-of-concept patches from a long time ago and, yes, judging by that we'd need to control all of this from userspace. The way I imagine that this would work is that userspace needs to first set the "Content Protection" and "HDCP Content Type" properties and wait for the state change. Once that has happened it can go and allocate the protected memory and start decoding into it and then scan out from these buffers. > Or is there any info inside the DMA-buf for this? In other words would you > also need to know if a buffer is then allocated from this special carveout? I don't think so. It's possible to scan out an unprotected buffer with HDCP enabled. It may also be possible to scan out a protected buffer even if HDCP wasn't enabled, though you would obviously want to prevent that somehow. Not sure if there's a common way to do this, but I guess in end-user devices you'd need a fully trusted boot chain to do that in a compliant way. It's been a long time since I looked at this, but I seem to recall that at the time all software that could do DRM-protected playback on Linux was proprietary for reasons like these. Thierry signature.asc Description: PGP signature
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
On Fri, Jun 28, 2024 at 01:42:27PM GMT, Christian König wrote: > Am 27.06.24 um 16:40 schrieb mrip...@kernel.org: > > [SNIP] > > > > > > > > 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). > > Yeah, that's perfectly fine. At least the AMD encryption solution > works exactly like that as well. > > So nobody can map that buffer, and the firmware driver is the one who > > knows that this buffer cannot be accessed by anyone. > > On most hw I know you can actually map that buffer, it's just that the > CPU sees only garbage in it because you don't have the necessary > decryption keys around. So you can always map and access the buffer, but only if you're in the right "context" the content would be correct? I think that part is pretty different than what ARM SoCs are doing, where they would typically prevent any CPU access and fault on access. > > 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. > > But that's exactly how all other implementations work as far as I know. I > mean what do you do if the kernel maps the encrypted buffer? > > On AMD we also block userspace and kernel CPU accesses, but that is only > done to make it easier to find bugs not for correctness. > > And userspace absolutely needs to be aware that a buffer is encrypted, cause > otherwise it could potentially try to access it with the CPU. I absolutely agree. I guess our discussion is whether it's something that should be implicit and understood by applications, or if it should be explicit and discoverable. Maxime signature.asc Description: PGP signature
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
On Fri, Jun 28, 2024 at 01:47:01PM GMT, Thierry Reding wrote: > On Thu, Jun 27, 2024 at 04:40:02PM GMT, mrip...@kernel.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-54cbbd049...@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 > > 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. > 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 may be possible to use these protected carveout regions exclusively > from the DRM/KMS driver and
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
Am 28.06.24 um 13:47 schrieb Thierry Reding: [SNIP] 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? 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? 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. 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. Yeah as far as I can see that would be a perfectly valid use case for DMA-Buf heaps. One question here: How does the HDCP setup work on Tegra? From your comment I guess you pass most of the information through userspace as well. Or is there any info inside the DMA-buf for this? In other words would you also need to know if a buffer is then allocated from this special carveout? Thanks, Christian. Thierry
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
On Thu, Jun 27, 2024 at 04:40:02PM GMT, mrip...@kernel.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-54cbbd049...@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? > 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? 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. 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. Thierry signature.asc Description: PGP signature
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
Am 27.06.24 um 16:40 schrieb mrip...@kernel.org: [SNIP] 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). Yeah, that's perfectly fine. At least the AMD encryption solution works exactly like that as well. So nobody can map that buffer, and the firmware driver is the one who knows that this buffer cannot be accessed by anyone. On most hw I know you can actually map that buffer, it's just that the CPU sees only garbage in it because you don't have the necessary decryption keys around. 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. But that's exactly how all other implementations work as far as I know. I mean what do you do if the kernel maps the encrypted buffer? On AMD we also block userspace and kernel CPU accesses, but that is only done to make it easier to find bugs not for correctness. And userspace absolutely needs to be aware that a buffer is encrypted, cause otherwise it could potentially try to access it with the CPU. Regards, Christian. Maxime
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
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-54cbbd049...@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). 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. Maxime signature.asc Description: PGP signature
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
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-54cbbd049...@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. > But on arm you have split designs everywhere and dma-buf > import/export, so > something else is needed. And neither current kms uapi nor > protocols/extensions have provisions for this (afaik) because it > works on > the big gpus, and on android it's just hacked up with backchannels. > > So yeah essentially I think we probably need something like this, as > much > as it sucks. I see it somewhat similar to handling pcip2pdma > limitations > in the kernel too. > > Not sure where/how it should be handled though, and maybe I've missed > something around protocols, in which case I guess we should add some > secure buffer flags to the ADDFB2 ioctl. Thanks for your hint, I'll try to add the secure flag to the ADDFB2 ioctl. If it works, I'll send the patch. Yeah, exactly what I would suggest as well. I'm not an expert for that part, but as far as I know we already have bunch of device specific tilling flags in there. Adding an MTK_ENCRYPTED flag should be trivial. Regards, Christian. Regards, Jason-JH.Lin > -Sima * MEDIATEK Confidentiality Notice The information contained in this e-mail message (including any attachments) may be confidential, proprietary, privileged, or otherwise exempt from disclosure under applicable laws. It is intended to be conveyed only to the designated recipient(s). Any use, dissemination, distribution, printing, retaining or copying of this e-mail (including its attachments) by unintended recipient(s) is strictly prohibited and may be unlawful. If you are not an intended recipient of this e-mail, or believe that you have received this e-mail in error, please notify the sender immediately (by replying to this e-mail), delete any and all copies of this e-mail (including any attachments) from your system, and do not disclose the content of this e-mail to any other person. Thank you!
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
Am 27.06.24 um 05:17 schrieb Jason-JH Lin (林睿祥):
On Wed, 2024-06-26 at 12:49 +0200, Christian König wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> Am 26.06.24 um 10:05 schrieb Jason-JH Lin (林睿祥):
> > >
> > > > In the step 3), we need to verify the dma-buf is allocated from
> > > > "restricted_mtk_cma", but there is no way to pass the
> > secure flag
> > > > or
> > > > private data from userspace to the import interface in DRM
> > driver.
> > >
> > > Why do you need to verify that?
> >
> > I need to know the imported buffer is allocated from restricted cma
> > and
> > mark it as a secure buffer in mediatek-drm driver. Then, I will add
> > some configuration to the hardware if the buffer is secure buffer,
> > so
> > that it can get the permission to access the secure buffer.
>
> Yeah so far that makes sense. This is basically what other drivers do
> with secure buffers as well.
>
> But why do you want the kernel to transport that information? Usually
> drivers get the information from userspace what to do with a buffer.
>
> In other words the format, stride, tilling and also if it's a secure
> buffer or not comes from userspace.
>
Thanks for your clear explanation.
I think this is what I want, but I can't find any DRM interface to pass
the secure flag from userspace.
Well stuff like that is usually device driver specific.
So you should probably use something device specific which tells the
driver that this buffer is encrypted.
> What the hardware usually handles internally is things like
> encryption keys, but you eventually get the information where to look
> for the key from userspace as well.
>
> Handling inside the kernel would only be necessary if userspace could
> for example crash the system with invalid parameters. But for
> encryption that is usually not the case.
>
Yes, that's true.
> > >
> > > > So I can only verify it like this now:
> > > > struct drm_gem_object *mtk_gem_prime_import_sg_table(struct
> > > > drm_device
> > > > *dev, struct dma_buf_attachment *attach, struct sg_table *sg)
> > > > {
> > > > struct mtk_gem_obj *mtk_gem;
> > > >
> > > > /* check if the entries in the sg_table are contiguous */
> > > > if (drm_prime_get_contiguous_size(sg) <
> > attach->dmabuf->size) {
> > > > DRM_ERROR("sg_table is not contiguous");
> > > > return ERR_PTR(-EINVAL);
> > > > }
> > > > mtk_gem = mtk_gem_init(dev, attach->dmabuf->size);
> > > > if (IS_ERR(mtk_gem))
> > > > return ERR_CAST(mtk_gem);
> > > >
> > > > + 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;
> > > >
> > > > return &mtk_gem->base;
> > > > }
> > >
> > > Complete NAK from my side to that approach. Importing of a DMA-
> > buf
> > > should be independent of the exporter.
> > >
> > > What you could do is to provide the secure buffer from a device
> > and
> > > not a device heap.
> > >
> >
> > You mean I should allocate buffer in mediate-drm driver not
> > userspace?
>
> Well that depends. The question is if you have multiple drivers which
> needs to work with this secure buffer?
>
> If yes then you should have a general allocation heap for it. If no
> then the buffers could as well be allocated from the driver interface
> directly.
>
Yes, this buffer needs work with GPU and DRM drivers, so this general
"restricted_mtk_cma" will allocated in userspace, then being passed to
GPU and DRM.
Well do you really need a separate heap for that? In other words is only
a certain part of the system memory capable of being encrypted?
Or would the "normal" CMA heap do as well and you only need to setup
your hardware properly for encryption?
Additional to that in most other drivers buffer sharing and encryption
are two separate things. In other words other drivers do something like
this:
1. Allocate the buffer.
2. Import the buffer using DRM_IOCTL_PRIME_FD_TO_HANDLE.
3. Set additional buffer properties, e.g. format, stride, tilling, if
it's secure, which encryption key to use, where to map etc...
So as far as I can see the problem you are facing is that you try to
mangle everything into DRM_IOCTL_PRIME_FD_TO_HANDLE. Why not make that a
separate IOCTL?
I mean we intentionally don't provide things like format, stride,
tilling etc.. to DRM_IOCTL_PRIME_FD_TO_HANDLE. Encryption is just
another of those parameters.
Regards,
Christian.
> > I just have modified this to userspace by the comment here:
> >
> >
https://patchwork.kernel.org/project/linux-mediatek/patch/20240403102701.369-3-shawn.s...@mediatek.com/#25806766
> >
> > > > I think I have the same problem as the ECC_FLAG mention in:
> > > >
> > > >
> >
https://lore.kernel.org/linux-media/20240515-dma-buf-
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
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-54cbbd049...@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. But on arm you have split designs everywhere and dma-buf import/export, so something else is needed. And neither current kms uapi nor protocols/extensions have provisions for this (afaik) because it works on the big gpus, and on android it's just hacked up with backchannels. So yeah essentially I think we probably need something like this, as much as it sucks. I see it somewhat similar to handling pcip2pdma limitations in the kernel too. Not sure where/how it should be handled though, and maybe I've missed something around protocols, in which case I guess we should add some secure buffer flags to the ADDFB2 ioctl. -Sima -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
Am 26.06.24 um 10:05 schrieb Jason-JH Lin (林睿祥):
>
> > In the step 3), we need to verify the dma-buf is allocated from
> > "restricted_mtk_cma", but there is no way to pass the secure flag
> > or
> > private data from userspace to the import interface in DRM driver.
>
> Why do you need to verify that?
I need to know the imported buffer is allocated from restricted cma and
mark it as a secure buffer in mediatek-drm driver. Then, I will add
some configuration to the hardware if the buffer is secure buffer, so
that it can get the permission to access the secure buffer.
Yeah so far that makes sense. This is basically what other drivers do
with secure buffers as well.
But why do you want the kernel to transport that information? Usually
drivers get the information from userspace what to do with a buffer.
In other words the format, stride, tilling and also if it's a secure
buffer or not comes from userspace.
What the hardware usually handles internally is things like encryption
keys, but you eventually get the information where to look for the key
from userspace as well.
Handling inside the kernel would only be necessary if userspace could
for example crash the system with invalid parameters. But for encryption
that is usually not the case.
>
> > So I can only verify it like this now:
> > struct drm_gem_object *mtk_gem_prime_import_sg_table(struct
> > drm_device
> > *dev, struct dma_buf_attachment *attach, struct sg_table *sg)
> > {
> > struct mtk_gem_obj *mtk_gem;
> >
> > /* check if the entries in the sg_table are contiguous */
> > if (drm_prime_get_contiguous_size(sg) < attach->dmabuf->size) {
> > DRM_ERROR("sg_table is not contiguous");
> > return ERR_PTR(-EINVAL);
> > }
> > mtk_gem = mtk_gem_init(dev, attach->dmabuf->size);
> > if (IS_ERR(mtk_gem))
> > return ERR_CAST(mtk_gem);
> >
> > + 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;
> >
> > return &mtk_gem->base;
> > }
>
> Complete NAK from my side to that approach. Importing of a DMA-buf
> should be independent of the exporter.
>
> What you could do is to provide the secure buffer from a device and
> not a device heap.
>
You mean I should allocate buffer in mediate-drm driver not userspace?
Well that depends. The question is if you have multiple drivers which
needs to work with this secure buffer?
If yes then you should have a general allocation heap for it. If no then
the buffers could as well be allocated from the driver interface directly.
I just have modified this to userspace by the comment here:
https://patchwork.kernel.org/project/linux-mediatek/patch/20240403102701.369-3-shawn.s...@mediatek.com/#25806766
> > 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-54cbbd049...@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?
Regards,
Christian.
Regards,
Jason-JH.Lin
>
> Regards,
> Christian.
>
> > Regards,
> > Jason-JH.Lin
> >
> > > Regards,
> > > Christian.
> >
> > * MEDIATEK Confidentiality Notice
> >
> > The information contained in this e-mail message (including any
> > attachments) may be confidential, proprietary, privileged, or
> > otherwise
> > exempt from disclosure under applicable laws. It is intended to be
> > conveyed only to the designated recipient(s). Any use,
> > dissemination,
> > distribution, printing, retaining or copying of this e-mail
> > (including its
> > attachments) by unintended recipient(s) is strictly prohibited and
> > may
> > be unlawful. If you are not an intended recipient of this e-mail,
> > or believe
> >
> > that you have received this e-mail in error, please notify the
> > sender
> > immediately (by replying to this e-mail), delete any and all copies
> > of
> > this e-mail (including a
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
Am 25.06.24 um 13:02 schrieb Jason-JH Lin (林睿祥):
Hi Christian,
On Tue, 2024-05-21 at 20:36 +0200, Christian König wrote:
> Am 20.05.24 um 09:58 schrieb Yong Wu (吴勇):
> > On Thu, 2024-05-16 at 10:17 +0200, Christian König wrote:
> > >
> > > External email : Please do not click links or open attachments
> > > until
> > > you have verified the sender or the content.
> > > Am 15.05.24 um 13:23 schrieb Yong Wu:
> > > > Introduce a FLAG for the restricted memory which means the
> > > > memory
> > >
> > > is
> > > > protected by TEE or hypervisor, then it's inaccessiable for
> > > > kernel.
> > > >
> > > > Currently we don't use sg_dma_unmark_restricted, thus this
> > >
> > > interface
> > > > has not been added.
> > >
> > > Why should that be part of the scatterlist? It doesn't seem to
> > > affect
> > > any of it's functionality.
> > >
> > > As far as I can see the scatterlist shouldn't be the transport of
> > > this
> > > kind of information.
> >
> > Thanks for the review. I will remove this.
> >
> > In our user scenario, DRM will import these buffers and check if
> > this
> > is a restricted buffer. If yes, it will use secure GCE takes over.
> >
> > If this judgment is not suitable to be placed in scatterlist. I
> > don't
> > know if it is ok to limit this inside dma-buf. Adding such an
> > interface:
> >
> > static bool dma_buf_is_restricted(struct dma_buf *dmabuf)
> > {
> > return !strncmp(dmabuf->exp_name, "restricted", 10);
> > }
>
> No, usually stuff like that doesn't belong into DMA buf either.
>
> Question here really is who controls the security status of the
> memory
> backing the buffer?
>
> In other words who tells the exporter that it should allocate and
> fill a
> buffer with encrypted data?
>
> If that is userspace then that is part of the format information and
> it
> is also userspace who should tell the importer that it needs to work
> with encrypted data.
>
> The kernel is intentionally not involved in stuff like that.
>
Here is the expected protected content buffer flow in DRM:
1) userspace allocates a dma-buf FD from the "restricted_mtk_cma" by
DMA_HEAP_IOCTL_ALLOC.
2) userspace imports that dma-buf into the device using prime for the
drm_file.
3) userspace uses the already implemented driver import code for the
special cases of protected content buffer.
What is so special on that case?
In the step 3), we need to verify the dma-buf is allocated from
"restricted_mtk_cma", but there is no way to pass the secure flag or
private data from userspace to the import interface in DRM driver.
Why do you need to verify that?
So I can only verify it like this now:
struct drm_gem_object *mtk_gem_prime_import_sg_table(struct drm_device
*dev, struct dma_buf_attachment *attach, struct sg_table *sg)
{
struct mtk_gem_obj *mtk_gem;
/* check if the entries in the sg_table are contiguous */
if (drm_prime_get_contiguous_size(sg) < attach->dmabuf->size) {
DRM_ERROR("sg_table is not contiguous");
return ERR_PTR(-EINVAL);
}
mtk_gem = mtk_gem_init(dev, attach->dmabuf->size);
if (IS_ERR(mtk_gem))
return ERR_CAST(mtk_gem);
+ 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;
return &mtk_gem->base;
}
Complete NAK from my side to that approach. Importing of a DMA-buf
should be independent of the exporter.
What you could do is to provide the secure buffer from a device and not
a device heap.
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-54cbbd049...@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?
Regards,
Christian.
Regards,
Jason-JH.Lin
> Regards,
> Christian.
* MEDIATEK Confidentiality Notice
The information contained in this e-mail message (including any
attachments) may be confidential, proprietary, privileged, or otherwise
exempt from disclosure under applicable laws. It is intended to be
conveyed only to the designated recipient(s). Any use, dissemination,
distribution, printing, retaining or copying of this e-mail (including its
attachments) by unintended recipient(s) is strictly prohibited and may
be unlawful. If you are not an intended recipient of this e-mail, or believe
that you have received this e-mail in error, please notify the sender
immediately (by replying to this e-mail), delete any and all copies of
this e-mail (including any attachments) from your system, and do
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
Am 20.05.24 um 09:58 schrieb Yong Wu (吴勇):
On Thu, 2024-05-16 at 10:17 +0200, Christian König wrote:
External email : Please do not click links or open attachments until
you have verified the sender or the content.
Am 15.05.24 um 13:23 schrieb Yong Wu:
Introduce a FLAG for the restricted memory which means the memory
is
protected by TEE or hypervisor, then it's inaccessiable for kernel.
Currently we don't use sg_dma_unmark_restricted, thus this
interface
has not been added.
Why should that be part of the scatterlist? It doesn't seem to
affect
any of it's functionality.
As far as I can see the scatterlist shouldn't be the transport of
this
kind of information.
Thanks for the review. I will remove this.
In our user scenario, DRM will import these buffers and check if this
is a restricted buffer. If yes, it will use secure GCE takes over.
If this judgment is not suitable to be placed in scatterlist. I don't
know if it is ok to limit this inside dma-buf. Adding such an
interface:
static bool dma_buf_is_restricted(struct dma_buf *dmabuf)
{
return !strncmp(dmabuf->exp_name, "restricted", 10);
}
No, usually stuff like that doesn't belong into DMA buf either.
Question here really is who controls the security status of the memory
backing the buffer?
In other words who tells the exporter that it should allocate and fill a
buffer with encrypted data?
If that is userspace then that is part of the format information and it
is also userspace who should tell the importer that it needs to work
with encrypted data.
The kernel is intentionally not involved in stuff like that.
Regards,
Christian.
Thanks.
Regards,
Christian.
Signed-off-by: Yong Wu
---
include/linux/scatterlist.h | 34
++
1 file changed, 34 insertions(+)
diff --git a/include/linux/scatterlist.h
b/include/linux/scatterlist.h
index 77df3d7b18a6..a6ad9018eca0 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -282,6 +282,7 @@ static inline void sg_unmark_end(struct
scatterlist *sg)
#define SG_DMA_BUS_ADDRESS(1 << 0)
#define SG_DMA_SWIOTLB(1 << 1)
+#define SG_DMA_RESTRICTED(2 << 1)
/**
* sg_dma_is_bus_address - Return whether a given segment was
marked
@@ -352,6 +353,31 @@ static inline void sg_dma_mark_swiotlb(struct
scatterlist *sg)
sg->dma_flags |= SG_DMA_SWIOTLB;
}
+/**
+ * sg_dma_mark_restricted - Mark the scatterlist for restricted
buffer.
+ * @sg:SG entry
+ *
+ * Description:
+ * Marks a a scatterlist for the restricted buffer that may be
inaccessiable
+ * in kernel if it is protected.
+ */
+static inline void sg_dma_mark_restricted(struct scatterlist *sg)
+{
+sg->dma_flags |= SG_DMA_RESTRICTED;
+}
+
+/**
+ * sg_dma_is_restricted - Return whether the scatterlist was
marked as restricted
+ *buffer.
+ * @sg:SG entry
+ *
+ * Description:
+ * Returns true if the scatterlist was marked as restricted
buffer.
+ */
+static inline bool sg_dma_is_restricted(struct scatterlist *sg)
+{
+return sg->dma_flags & SG_DMA_RESTRICTED;
+}
#else
static inline bool sg_dma_is_bus_address(struct scatterlist *sg)
@@ -372,6 +398,14 @@ static inline void sg_dma_mark_swiotlb(struct
scatterlist *sg)
{
}
+static inline bool sg_dma_is_restricted(struct scatterlist *sg)
+{
+return false;
+}
+
+static inline void sg_dma_mark_restrited(struct scatterlist *sg)
+{
+}
#endif/* CONFIG_NEED_SG_DMA_FLAGS */
/**
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
On Thu, 2024-05-16 at 11:59 +0200, AngeloGioacchino Del Regno wrote: > Il 15/05/24 13:23, Yong Wu ha scritto: > > Introduce a FLAG for the restricted memory which means the memory > > is > > protected by TEE or hypervisor, then it's inaccessiable for kernel. > > > > Currently we don't use sg_dma_unmark_restricted, thus this > > interface > > has not been added. > > > > Signed-off-by: Yong Wu > > --- > > include/linux/scatterlist.h | 34 > > ++ > > 1 file changed, 34 insertions(+) > > > > diff --git a/include/linux/scatterlist.h > > b/include/linux/scatterlist.h > > index 77df3d7b18a6..a6ad9018eca0 100644 > > --- a/include/linux/scatterlist.h > > +++ b/include/linux/scatterlist.h > > @@ -282,6 +282,7 @@ static inline void sg_unmark_end(struct > > scatterlist *sg) > > > > #define SG_DMA_BUS_ADDRESS(1 << 0) > > #define SG_DMA_SWIOTLB(1 << 1) > > +#define SG_DMA_RESTRICTED (2 << 1) > > I think you wanted to write (1 << 2) here :-) Apparently, you are right:) Thanks. > > Cheers, > Angelo
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
On Thu, 2024-05-16 at 10:17 +0200, Christian König wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> Am 15.05.24 um 13:23 schrieb Yong Wu:
> > Introduce a FLAG for the restricted memory which means the memory
> is
> > protected by TEE or hypervisor, then it's inaccessiable for kernel.
> >
> > Currently we don't use sg_dma_unmark_restricted, thus this
> interface
> > has not been added.
>
> Why should that be part of the scatterlist? It doesn't seem to
> affect
> any of it's functionality.
>
> As far as I can see the scatterlist shouldn't be the transport of
> this
> kind of information.
Thanks for the review. I will remove this.
In our user scenario, DRM will import these buffers and check if this
is a restricted buffer. If yes, it will use secure GCE takes over.
If this judgment is not suitable to be placed in scatterlist. I don't
know if it is ok to limit this inside dma-buf. Adding such an
interface:
static bool dma_buf_is_restricted(struct dma_buf *dmabuf)
{
return !strncmp(dmabuf->exp_name, "restricted", 10);
}
Thanks.
>
> Regards,
> Christian.
>
> >
> > Signed-off-by: Yong Wu
> > ---
> > include/linux/scatterlist.h | 34
> ++
> > 1 file changed, 34 insertions(+)
> >
> > diff --git a/include/linux/scatterlist.h
> b/include/linux/scatterlist.h
> > index 77df3d7b18a6..a6ad9018eca0 100644
> > --- a/include/linux/scatterlist.h
> > +++ b/include/linux/scatterlist.h
> > @@ -282,6 +282,7 @@ static inline void sg_unmark_end(struct
> scatterlist *sg)
> >
> > #define SG_DMA_BUS_ADDRESS(1 << 0)
> > #define SG_DMA_SWIOTLB(1 << 1)
> > +#define SG_DMA_RESTRICTED(2 << 1)
> >
> > /**
> >* sg_dma_is_bus_address - Return whether a given segment was
> marked
> > @@ -352,6 +353,31 @@ static inline void sg_dma_mark_swiotlb(struct
> scatterlist *sg)
> > sg->dma_flags |= SG_DMA_SWIOTLB;
> > }
> >
> > +/**
> > + * sg_dma_mark_restricted - Mark the scatterlist for restricted
> buffer.
> > + * @sg:SG entry
> > + *
> > + * Description:
> > + * Marks a a scatterlist for the restricted buffer that may be
> inaccessiable
> > + * in kernel if it is protected.
> > + */
> > +static inline void sg_dma_mark_restricted(struct scatterlist *sg)
> > +{
> > +sg->dma_flags |= SG_DMA_RESTRICTED;
> > +}
> > +
> > +/**
> > + * sg_dma_is_restricted - Return whether the scatterlist was
> marked as restricted
> > + *buffer.
> > + * @sg:SG entry
> > + *
> > + * Description:
> > + * Returns true if the scatterlist was marked as restricted
> buffer.
> > + */
> > +static inline bool sg_dma_is_restricted(struct scatterlist *sg)
> > +{
> > +return sg->dma_flags & SG_DMA_RESTRICTED;
> > +}
> > #else
> >
> > static inline bool sg_dma_is_bus_address(struct scatterlist *sg)
> > @@ -372,6 +398,14 @@ static inline void sg_dma_mark_swiotlb(struct
> scatterlist *sg)
> > {
> > }
> >
> > +static inline bool sg_dma_is_restricted(struct scatterlist *sg)
> > +{
> > +return false;
> > +}
> > +
> > +static inline void sg_dma_mark_restrited(struct scatterlist *sg)
> > +{
> > +}
> > #endif/* CONFIG_NEED_SG_DMA_FLAGS */
> >
> > /**
>
>
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
Il 15/05/24 13:23, Yong Wu ha scritto:
Introduce a FLAG for the restricted memory which means the memory is
protected by TEE or hypervisor, then it's inaccessiable for kernel.
Currently we don't use sg_dma_unmark_restricted, thus this interface
has not been added.
Signed-off-by: Yong Wu
---
include/linux/scatterlist.h | 34 ++
1 file changed, 34 insertions(+)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index 77df3d7b18a6..a6ad9018eca0 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -282,6 +282,7 @@ static inline void sg_unmark_end(struct scatterlist *sg)
#define SG_DMA_BUS_ADDRESS (1 << 0)
#define SG_DMA_SWIOTLB(1 << 1)
+#define SG_DMA_RESTRICTED (2 << 1)
I think you wanted to write (1 << 2) here :-)
Cheers,
Angelo
/**
* sg_dma_is_bus_address - Return whether a given segment was marked
@@ -352,6 +353,31 @@ static inline void sg_dma_mark_swiotlb(struct scatterlist
*sg)
sg->dma_flags |= SG_DMA_SWIOTLB;
}
+/**
+ * sg_dma_mark_restricted - Mark the scatterlist for restricted buffer.
+ * @sg:SG entry
+ *
+ * Description:
+ * Marks a a scatterlist for the restricted buffer that may be inaccessiable
+ * in kernel if it is protected.
+ */
+static inline void sg_dma_mark_restricted(struct scatterlist *sg)
+{
+ sg->dma_flags |= SG_DMA_RESTRICTED;
+}
+
+/**
+ * sg_dma_is_restricted - Return whether the scatterlist was marked as
restricted
+ *buffer.
+ * @sg:SG entry
+ *
+ * Description:
+ * Returns true if the scatterlist was marked as restricted buffer.
+ */
+static inline bool sg_dma_is_restricted(struct scatterlist *sg)
+{
+ return sg->dma_flags & SG_DMA_RESTRICTED;
+}
#else
static inline bool sg_dma_is_bus_address(struct scatterlist *sg)
@@ -372,6 +398,14 @@ static inline void sg_dma_mark_swiotlb(struct scatterlist
*sg)
{
}
+static inline bool sg_dma_is_restricted(struct scatterlist *sg)
+{
+ return false;
+}
+
+static inline void sg_dma_mark_restrited(struct scatterlist *sg)
+{
+}
#endif/* CONFIG_NEED_SG_DMA_FLAGS */
/**
Re: [PATCH v5 2/9] scatterlist: Add a flag for the restricted memory
Am 15.05.24 um 13:23 schrieb Yong Wu:
Introduce a FLAG for the restricted memory which means the memory is
protected by TEE or hypervisor, then it's inaccessiable for kernel.
Currently we don't use sg_dma_unmark_restricted, thus this interface
has not been added.
Why should that be part of the scatterlist? It doesn't seem to affect
any of it's functionality.
As far as I can see the scatterlist shouldn't be the transport of this
kind of information.
Regards,
Christian.
Signed-off-by: Yong Wu
---
include/linux/scatterlist.h | 34 ++
1 file changed, 34 insertions(+)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index 77df3d7b18a6..a6ad9018eca0 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -282,6 +282,7 @@ static inline void sg_unmark_end(struct scatterlist *sg)
#define SG_DMA_BUS_ADDRESS (1 << 0)
#define SG_DMA_SWIOTLB(1 << 1)
+#define SG_DMA_RESTRICTED (2 << 1)
/**
* sg_dma_is_bus_address - Return whether a given segment was marked
@@ -352,6 +353,31 @@ static inline void sg_dma_mark_swiotlb(struct scatterlist
*sg)
sg->dma_flags |= SG_DMA_SWIOTLB;
}
+/**
+ * sg_dma_mark_restricted - Mark the scatterlist for restricted buffer.
+ * @sg:SG entry
+ *
+ * Description:
+ * Marks a a scatterlist for the restricted buffer that may be inaccessiable
+ * in kernel if it is protected.
+ */
+static inline void sg_dma_mark_restricted(struct scatterlist *sg)
+{
+ sg->dma_flags |= SG_DMA_RESTRICTED;
+}
+
+/**
+ * sg_dma_is_restricted - Return whether the scatterlist was marked as
restricted
+ *buffer.
+ * @sg:SG entry
+ *
+ * Description:
+ * Returns true if the scatterlist was marked as restricted buffer.
+ */
+static inline bool sg_dma_is_restricted(struct scatterlist *sg)
+{
+ return sg->dma_flags & SG_DMA_RESTRICTED;
+}
#else
static inline bool sg_dma_is_bus_address(struct scatterlist *sg)
@@ -372,6 +398,14 @@ static inline void sg_dma_mark_swiotlb(struct scatterlist
*sg)
{
}
+static inline bool sg_dma_is_restricted(struct scatterlist *sg)
+{
+ return false;
+}
+
+static inline void sg_dma_mark_restrited(struct scatterlist *sg)
+{
+}
#endif/* CONFIG_NEED_SG_DMA_FLAGS */
/**
