Re: [PATCH v4 2/3] dt-bindings: mtd: Add a property to declare secure regions in NAND chips

2021-03-08 Thread Boris Brezillon
On Mon, 8 Mar 2021 19:01:34 +0530
Manivannan Sadhasivam  wrote:

> On Mon, Mar 08, 2021 at 10:10:59AM +0100, Boris Brezillon wrote:
> > On Mon,  8 Mar 2021 11:14:46 +0530
> > Manivannan Sadhasivam  wrote:
> >   
> > > On a typical end product, a vendor may choose to secure some regions in
> > > the NAND memory which are supposed to stay intact between FW upgrades.
> > > The access to those regions will be blocked by a secure element like
> > > Trustzone. So the normal world software like Linux kernel should not
> > > touch these regions (including reading).
> > > 
> > > So let's add a property for declaring such secure regions so that the
> > > drivers can skip touching them.
> > > 
> > > Signed-off-by: Manivannan Sadhasivam 
> > > ---
> > >  Documentation/devicetree/bindings/mtd/nand-controller.yaml | 7 +++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/mtd/nand-controller.yaml 
> > > b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> > > index d0e422f4b3e0..15a674bedca3 100644
> > > --- a/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> > > +++ b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> > > @@ -143,6 +143,13 @@ patternProperties:
> > >Ready/Busy pins. Active state refers to the NAND ready state 
> > > and
> > >should be set to GPIOD_ACTIVE_HIGH unless the signal is 
> > > inverted.
> > >  
> > > +  secure-regions:
> > > +$ref: /schemas/types.yaml#/definitions/uint32-matrix
> > > +description:
> > > +  Regions in the NAND chip which are protected using a secure 
> > > element
> > > +  like Trustzone. This property contains the start address and 
> > > size of
> > > +  the secure regions present.
> > > +  
> > 
> > Since you declare this as a generic property, I think it'd be simpler
> > to do the check at the core level.
> >   
> 
> Hmm, so have the parsing logic in qcom driver and check in core or both 
> parsing
> and check in core?

Both in the core.


Re: [PATCH v4 2/3] dt-bindings: mtd: Add a property to declare secure regions in NAND chips

2021-03-08 Thread Manivannan Sadhasivam
On Mon, Mar 08, 2021 at 10:10:59AM +0100, Boris Brezillon wrote:
> On Mon,  8 Mar 2021 11:14:46 +0530
> Manivannan Sadhasivam  wrote:
> 
> > On a typical end product, a vendor may choose to secure some regions in
> > the NAND memory which are supposed to stay intact between FW upgrades.
> > The access to those regions will be blocked by a secure element like
> > Trustzone. So the normal world software like Linux kernel should not
> > touch these regions (including reading).
> > 
> > So let's add a property for declaring such secure regions so that the
> > drivers can skip touching them.
> > 
> > Signed-off-by: Manivannan Sadhasivam 
> > ---
> >  Documentation/devicetree/bindings/mtd/nand-controller.yaml | 7 +++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/nand-controller.yaml 
> > b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> > index d0e422f4b3e0..15a674bedca3 100644
> > --- a/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> > +++ b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> > @@ -143,6 +143,13 @@ patternProperties:
> >Ready/Busy pins. Active state refers to the NAND ready state and
> >should be set to GPIOD_ACTIVE_HIGH unless the signal is inverted.
> >  
> > +  secure-regions:
> > +$ref: /schemas/types.yaml#/definitions/uint32-matrix
> > +description:
> > +  Regions in the NAND chip which are protected using a secure 
> > element
> > +  like Trustzone. This property contains the start address and 
> > size of
> > +  the secure regions present.
> > +
> 
> Since you declare this as a generic property, I think it'd be simpler
> to do the check at the core level.
> 

Hmm, so have the parsing logic in qcom driver and check in core or both parsing
and check in core?

I don't think the first one makes sense.

Thanks,
Mani

> >  required:
> >- reg
> >  
> 


Re: [PATCH v4 2/3] dt-bindings: mtd: Add a property to declare secure regions in NAND chips

2021-03-08 Thread Boris Brezillon
On Mon,  8 Mar 2021 11:14:46 +0530
Manivannan Sadhasivam  wrote:

> On a typical end product, a vendor may choose to secure some regions in
> the NAND memory which are supposed to stay intact between FW upgrades.
> The access to those regions will be blocked by a secure element like
> Trustzone. So the normal world software like Linux kernel should not
> touch these regions (including reading).
> 
> So let's add a property for declaring such secure regions so that the
> drivers can skip touching them.
> 
> Signed-off-by: Manivannan Sadhasivam 
> ---
>  Documentation/devicetree/bindings/mtd/nand-controller.yaml | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/nand-controller.yaml 
> b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> index d0e422f4b3e0..15a674bedca3 100644
> --- a/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> +++ b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> @@ -143,6 +143,13 @@ patternProperties:
>Ready/Busy pins. Active state refers to the NAND ready state and
>should be set to GPIOD_ACTIVE_HIGH unless the signal is inverted.
>  
> +  secure-regions:
> +$ref: /schemas/types.yaml#/definitions/uint32-matrix
> +description:
> +  Regions in the NAND chip which are protected using a secure element
> +  like Trustzone. This property contains the start address and size 
> of
> +  the secure regions present.
> +

Since you declare this as a generic property, I think it'd be simpler
to do the check at the core level.

>  required:
>- reg
>  



[PATCH v4 2/3] dt-bindings: mtd: Add a property to declare secure regions in NAND chips

2021-03-07 Thread Manivannan Sadhasivam
On a typical end product, a vendor may choose to secure some regions in
the NAND memory which are supposed to stay intact between FW upgrades.
The access to those regions will be blocked by a secure element like
Trustzone. So the normal world software like Linux kernel should not
touch these regions (including reading).

So let's add a property for declaring such secure regions so that the
drivers can skip touching them.

Signed-off-by: Manivannan Sadhasivam 
---
 Documentation/devicetree/bindings/mtd/nand-controller.yaml | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/nand-controller.yaml 
b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
index d0e422f4b3e0..15a674bedca3 100644
--- a/Documentation/devicetree/bindings/mtd/nand-controller.yaml
+++ b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
@@ -143,6 +143,13 @@ patternProperties:
   Ready/Busy pins. Active state refers to the NAND ready state and
   should be set to GPIOD_ACTIVE_HIGH unless the signal is inverted.
 
+  secure-regions:
+$ref: /schemas/types.yaml#/definitions/uint32-matrix
+description:
+  Regions in the NAND chip which are protected using a secure element
+  like Trustzone. This property contains the start address and size of
+  the secure regions present.
+
 required:
   - reg
 
-- 
2.25.1