2021-02-04 09:01:36

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH] mtd: rawnand: Do not check for bad block if bbt is unavailable

On Thu, 4 Feb 2021 14:22:21 +0530
Manivannan Sadhasivam <[email protected]> wrote:

> On Thu, Feb 04, 2021 at 09:13:36AM +0100, Miquel Raynal wrote:
> > Hi Manivannan,
> >
> > Manivannan Sadhasivam <[email protected]> wrote on Wed,
> > 03 Feb 2021 17:11:31 +0530:
> >
> > > On 3 February 2021 4:54:22 PM IST, Boris Brezillon <[email protected]> wrote:
> > > >On Wed, 03 Feb 2021 16:22:42 +0530
> > > >Manivannan Sadhasivam <[email protected]> wrote:
> > > >
> > > >> On 3 February 2021 3:49:14 PM IST, Boris Brezillon
> > > ><[email protected]> wrote:
> > > >> >On Wed, 03 Feb 2021 15:42:02 +0530
> > > >> >Manivannan Sadhasivam <[email protected]> wrote:
> > > >> >
> > > >> >> >>
> > > >> >> >> I got more information from the vendor, Telit. The access to
> > > >the
> > > >> >3rd
> > > >> >> >partition is protected by Trustzone and any access in non
> > > >privileged
> > > >> >> >mode (where Linux kernel runs) causes kernel panic and the device
> > > >> >> >reboots.
> > > >> >
> > > >> >Out of curiosity, is it a per-CS-line thing or is this section
> > > >> >protected on all CS?
> > > >> >
> > > >>
> > > >> Sorry, I didn't get your question.
> > > >
> > > >The qcom controller can handle several chips, each connected through a
> > > >different CS (chip-select) line, right? I'm wondering if the firmware
> > > >running in secure mode has the ability to block access for a specific
> > > >CS line or if all CS lines have the same constraint. That will impact
> > > >the way you describe it in your DT (in one case the secure-region
> > > >property should be under the controller node, in the other case it
> > > >should be under the NAND chip node).
> > >
> > > Right. I believe the implementation is common to all NAND chips so the property should be in the controller node.
> >
> > Looks weird: do you mean that each of the chips will have a secure area?
>
> I way I said is, the "secure-region" property will be present in the controller
> node and not in the NAND chip node since this is not related to the device
> functionality.
>
> But for referencing the NAND device, the property can have the phandle as below:
>
> secure-region = <&nand0 0xffff>;

My question was really what happens from a functional PoV. If you have
per-chip protection at the FW level, this property should be under the
NAND node. OTH, if the FW doesn't look at the selected chip before
blocking the access, it should be at the controller level. So, you
really have to understand what the secure FW does.


2021-02-04 09:12:34

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH] mtd: rawnand: Do not check for bad block if bbt is unavailable

Hi Boris,

Boris Brezillon <[email protected]> wrote on Thu, 4 Feb
2021 09:59:45 +0100:

> On Thu, 4 Feb 2021 14:22:21 +0530
> Manivannan Sadhasivam <[email protected]> wrote:
>
> > On Thu, Feb 04, 2021 at 09:13:36AM +0100, Miquel Raynal wrote:
> > > Hi Manivannan,
> > >
> > > Manivannan Sadhasivam <[email protected]> wrote on Wed,
> > > 03 Feb 2021 17:11:31 +0530:
> > >
> > > > On 3 February 2021 4:54:22 PM IST, Boris Brezillon <[email protected]> wrote:
> > > > >On Wed, 03 Feb 2021 16:22:42 +0530
> > > > >Manivannan Sadhasivam <[email protected]> wrote:
> > > > >
> > > > >> On 3 February 2021 3:49:14 PM IST, Boris Brezillon
> > > > ><[email protected]> wrote:
> > > > >> >On Wed, 03 Feb 2021 15:42:02 +0530
> > > > >> >Manivannan Sadhasivam <[email protected]> wrote:
> > > > >> >
> > > > >> >> >>
> > > > >> >> >> I got more information from the vendor, Telit. The access to
> > > > >the
> > > > >> >3rd
> > > > >> >> >partition is protected by Trustzone and any access in non
> > > > >privileged
> > > > >> >> >mode (where Linux kernel runs) causes kernel panic and the device
> > > > >> >> >reboots.
> > > > >> >
> > > > >> >Out of curiosity, is it a per-CS-line thing or is this section
> > > > >> >protected on all CS?
> > > > >> >
> > > > >>
> > > > >> Sorry, I didn't get your question.
> > > > >
> > > > >The qcom controller can handle several chips, each connected through a
> > > > >different CS (chip-select) line, right? I'm wondering if the firmware
> > > > >running in secure mode has the ability to block access for a specific
> > > > >CS line or if all CS lines have the same constraint. That will impact
> > > > >the way you describe it in your DT (in one case the secure-region
> > > > >property should be under the controller node, in the other case it
> > > > >should be under the NAND chip node).
> > > >
> > > > Right. I believe the implementation is common to all NAND chips so the property should be in the controller node.
> > >
> > > Looks weird: do you mean that each of the chips will have a secure area?
> >
> > I way I said is, the "secure-region" property will be present in the controller
> > node and not in the NAND chip node since this is not related to the device
> > functionality.
> >
> > But for referencing the NAND device, the property can have the phandle as below:
> >
> > secure-region = <&nand0 0xffff>;
>
> My question was really what happens from a functional PoV. If you have
> per-chip protection at the FW level, this property should be under the
> NAND node. OTH, if the FW doesn't look at the selected chip before
> blocking the access, it should be at the controller level. So, you
> really have to understand what the secure FW does.

I'm not so sure actually, that's why I like the phandle to nand0 -> in
any case it's not a property of the NAND chip itself, it's kind of a
host constraint, so I don't get why the property should be at the
NAND node level?

Also, we should probably support several secure regions (which could be
a way to express the fact that the FW does not look at the CS)?

2021-02-04 09:30:47

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH] mtd: rawnand: Do not check for bad block if bbt is unavailable

On Thu, 4 Feb 2021 10:04:08 +0100
Miquel Raynal <[email protected]> wrote:

> Hi Boris,
>
> Boris Brezillon <[email protected]> wrote on Thu, 4 Feb
> 2021 09:59:45 +0100:
>
> > On Thu, 4 Feb 2021 14:22:21 +0530
> > Manivannan Sadhasivam <[email protected]> wrote:
> >
> > > On Thu, Feb 04, 2021 at 09:13:36AM +0100, Miquel Raynal wrote:
> > > > Hi Manivannan,
> > > >
> > > > Manivannan Sadhasivam <[email protected]> wrote on Wed,
> > > > 03 Feb 2021 17:11:31 +0530:
> > > >
> > > > > On 3 February 2021 4:54:22 PM IST, Boris Brezillon <[email protected]> wrote:
> > > > > >On Wed, 03 Feb 2021 16:22:42 +0530
> > > > > >Manivannan Sadhasivam <[email protected]> wrote:
> > > > > >
> > > > > >> On 3 February 2021 3:49:14 PM IST, Boris Brezillon
> > > > > ><[email protected]> wrote:
> > > > > >> >On Wed, 03 Feb 2021 15:42:02 +0530
> > > > > >> >Manivannan Sadhasivam <[email protected]> wrote:
> > > > > >> >
> > > > > >> >> >>
> > > > > >> >> >> I got more information from the vendor, Telit. The access to
> > > > > >the
> > > > > >> >3rd
> > > > > >> >> >partition is protected by Trustzone and any access in non
> > > > > >privileged
> > > > > >> >> >mode (where Linux kernel runs) causes kernel panic and the device
> > > > > >> >> >reboots.
> > > > > >> >
> > > > > >> >Out of curiosity, is it a per-CS-line thing or is this section
> > > > > >> >protected on all CS?
> > > > > >> >
> > > > > >>
> > > > > >> Sorry, I didn't get your question.
> > > > > >
> > > > > >The qcom controller can handle several chips, each connected through a
> > > > > >different CS (chip-select) line, right? I'm wondering if the firmware
> > > > > >running in secure mode has the ability to block access for a specific
> > > > > >CS line or if all CS lines have the same constraint. That will impact
> > > > > >the way you describe it in your DT (in one case the secure-region
> > > > > >property should be under the controller node, in the other case it
> > > > > >should be under the NAND chip node).
> > > > >
> > > > > Right. I believe the implementation is common to all NAND chips so the property should be in the controller node.
> > > >
> > > > Looks weird: do you mean that each of the chips will have a secure area?
> > >
> > > I way I said is, the "secure-region" property will be present in the controller
> > > node and not in the NAND chip node since this is not related to the device
> > > functionality.
> > >
> > > But for referencing the NAND device, the property can have the phandle as below:
> > >
> > > secure-region = <&nand0 0xffff>;
> >
> > My question was really what happens from a functional PoV. If you have
> > per-chip protection at the FW level, this property should be under the
> > NAND node. OTH, if the FW doesn't look at the selected chip before
> > blocking the access, it should be at the controller level. So, you
> > really have to understand what the secure FW does.
>
> I'm not so sure actually, that's why I like the phandle to nand0 -> in
> any case it's not a property of the NAND chip itself, it's kind of a
> host constraint, so I don't get why the property should be at the
> NAND node level?

I would argue that we already have plenty of NAND properties that
encode things controlled by the host (ECC, partitions, HW randomizer,
boot device, and all kind of controller specific stuff) :P. Having
the props under the NAND node makes it clear what those things are
applied to, and it's also easier to parse for the driver (you already
have to parse each node to get the reg property anyway).

>
> Also, we should probably support several secure regions (which could be
> a way to express the fact that the FW does not look at the CS)?

Sure, the secure-region should probably be renamed secure-regions, even
if it's defined at the NAND chip level.

2021-02-04 09:35:18

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH] mtd: rawnand: Do not check for bad block if bbt is unavailable

Hi Boris,

Boris Brezillon <[email protected]> wrote on Thu, 4 Feb
2021 10:27:38 +0100:

> On Thu, 4 Feb 2021 10:04:08 +0100
> Miquel Raynal <[email protected]> wrote:
>
> > Hi Boris,
> >
> > Boris Brezillon <[email protected]> wrote on Thu, 4 Feb
> > 2021 09:59:45 +0100:
> >
> > > On Thu, 4 Feb 2021 14:22:21 +0530
> > > Manivannan Sadhasivam <[email protected]> wrote:
> > >
> > > > On Thu, Feb 04, 2021 at 09:13:36AM +0100, Miquel Raynal wrote:
> > > > > Hi Manivannan,
> > > > >
> > > > > Manivannan Sadhasivam <[email protected]> wrote on Wed,
> > > > > 03 Feb 2021 17:11:31 +0530:
> > > > >
> > > > > > On 3 February 2021 4:54:22 PM IST, Boris Brezillon <[email protected]> wrote:
> > > > > > >On Wed, 03 Feb 2021 16:22:42 +0530
> > > > > > >Manivannan Sadhasivam <[email protected]> wrote:
> > > > > > >
> > > > > > >> On 3 February 2021 3:49:14 PM IST, Boris Brezillon
> > > > > > ><[email protected]> wrote:
> > > > > > >> >On Wed, 03 Feb 2021 15:42:02 +0530
> > > > > > >> >Manivannan Sadhasivam <[email protected]> wrote:
> > > > > > >> >
> > > > > > >> >> >>
> > > > > > >> >> >> I got more information from the vendor, Telit. The access to
> > > > > > >the
> > > > > > >> >3rd
> > > > > > >> >> >partition is protected by Trustzone and any access in non
> > > > > > >privileged
> > > > > > >> >> >mode (where Linux kernel runs) causes kernel panic and the device
> > > > > > >> >> >reboots.
> > > > > > >> >
> > > > > > >> >Out of curiosity, is it a per-CS-line thing or is this section
> > > > > > >> >protected on all CS?
> > > > > > >> >
> > > > > > >>
> > > > > > >> Sorry, I didn't get your question.
> > > > > > >
> > > > > > >The qcom controller can handle several chips, each connected through a
> > > > > > >different CS (chip-select) line, right? I'm wondering if the firmware
> > > > > > >running in secure mode has the ability to block access for a specific
> > > > > > >CS line or if all CS lines have the same constraint. That will impact
> > > > > > >the way you describe it in your DT (in one case the secure-region
> > > > > > >property should be under the controller node, in the other case it
> > > > > > >should be under the NAND chip node).
> > > > > >
> > > > > > Right. I believe the implementation is common to all NAND chips so the property should be in the controller node.
> > > > >
> > > > > Looks weird: do you mean that each of the chips will have a secure area?
> > > >
> > > > I way I said is, the "secure-region" property will be present in the controller
> > > > node and not in the NAND chip node since this is not related to the device
> > > > functionality.
> > > >
> > > > But for referencing the NAND device, the property can have the phandle as below:
> > > >
> > > > secure-region = <&nand0 0xffff>;
> > >
> > > My question was really what happens from a functional PoV. If you have
> > > per-chip protection at the FW level, this property should be under the
> > > NAND node. OTH, if the FW doesn't look at the selected chip before
> > > blocking the access, it should be at the controller level. So, you
> > > really have to understand what the secure FW does.
> >
> > I'm not so sure actually, that's why I like the phandle to nand0 -> in
> > any case it's not a property of the NAND chip itself, it's kind of a
> > host constraint, so I don't get why the property should be at the
> > NAND node level?
>
> I would argue that we already have plenty of NAND properties that
> encode things controlled by the host (ECC, partitions, HW randomizer,
> boot device, and all kind of controller specific stuff) :P. Having
> the props under the NAND node makes it clear what those things are
> applied to, and it's also easier to parse for the driver (you already
> have to parse each node to get the reg property anyway).

Fair points.

> > Also, we should probably support several secure regions (which could be
> > a way to express the fact that the FW does not look at the CS)?
>
> Sure, the secure-region should probably be renamed secure-regions, even
> if it's defined at the NAND chip level.

Absolutely.

Thanks,
Miquèl