[v5,2/9] scatterlist: Add a flag for the restricted memory

Message ID 20240515112308.10171-3-yong.wu@mediatek.com (mailing list archive)
State New
Headers
Series dma-buf: heaps: Add restricted heap |

Commit Message

Yong Wu (吴勇) May 15, 2024, 11:23 a.m. UTC
  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 <yong.wu@mediatek.com>
---
 include/linux/scatterlist.h | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)
  

Comments

Christian König May 16, 2024, 8:17 a.m. UTC | #1
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 <yong.wu@mediatek.com>
> ---
>   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 */
>   
>   /**
  
AngeloGioacchino Del Regno May 16, 2024, 9:59 a.m. UTC | #2
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 <yong.wu@mediatek.com>
> ---
>   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 */
>   
>   /**
  
Yong Wu (吴勇) May 20, 2024, 7:58 a.m. UTC | #3
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 <yong.wu@mediatek.com>
> > ---
> >   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 */
> >   
> >   /**
> 
>
  
Yong Wu (吴勇) May 20, 2024, 9:53 a.m. UTC | #4
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 <yong.wu@mediatek.com>
> > ---
> >   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
  
Christian König May 21, 2024, 6:36 p.m. UTC | #5
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 <yong.wu@mediatek.com>
>>> ---
>>>    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 */
>>>    
>>>    /**
>>
  

Patch

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 */
 
 /**