mbox series

[v4,0/2] Add support for unprotected spare data page

Message ID 20220519190112.6344-1-ansuelsmth@gmail.com
Headers show
Series Add support for unprotected spare data page | expand

Message

Christian Marangi May 19, 2022, 7:01 p.m. UTC
Some background about this.
On original qsdk ipq8064 based firmware there was a big separation from
boot partition and user partition. With boot partition we refer to
partition used to init the router (bootloader, spm firmware and other
internal stuff) With user partition we refer to linux partition and data
partition not used to init the router.
When someone had to write to these boot partition a special mode was
needed, to switch the nand driver to this special configuration.

Upstream version of the nandc driver totally dropped this and the result
is that if someone try to read data from these partition a CRC warning
is printed and if someone try to write that (if for example someone
wants to replace the bootloader) result is a broken system as the data
is badly written.

This series comes to fix this.

A user can declare offset and size of these special partition using the
qcom,boot-pages binding.

An initial implementation of this assumed that the boot-pages started
from the start of the nand but we discover that some device have backup
of these special partition and we can have situation where we have this
partition scheme
- APPSBL (require special mode)
- APPSBLENV (doesn't require special mode)
- ART
- APPSBLBK (back of APPSBL require special mode)
- APPSBLENVBK (back of APPSBLENV doesn't require special mode)
With this configuration we need to declare sparse boot page and we can't
assume boot-pages always starts from the start of the nand.

A user can use this form to declare sparse boot pages
qcom,boot-pages = <0x0 0x0c80000 0x0c80000 0x0500000>;

The driver internally will parse this array, convert it to nand pages
and check internally on every read/write if this special configuration
should used for that page or the normal one.

The reason for all of this is that qcom FOR SOME REASON, disable ECC for
spare data only for these boot partition and we need to reflect this
special configuration to mute these warning and to permit actually
writing to these pages.

v4:
- Fix wrong compatible set for boot-pages (ipq8074 instead of ipq806x)
v3:
- Fix typo in Docmunetation commit desription
- Add items description for uint32-matrix
v2:
- Add fixes from Krzysztof in Documentation

Ansuel Smith (2):
  mtd: nand: raw: qcom_nandc: add support for unprotected spare data
    pages
  dt-bindings: mtd: qcom_nandc: document qcom,boot-pages binding

 .../devicetree/bindings/mtd/qcom,nandc.yaml   |  26 +++
 drivers/mtd/nand/raw/qcom_nandc.c             | 148 +++++++++++++++++-
 2 files changed, 169 insertions(+), 5 deletions(-)

Comments

Miquel Raynal June 3, 2022, 1:49 p.m. UTC | #1
Hi,

ansuelsmth@gmail.com wrote on Thu, 19 May 2022 21:01:10 +0200:

> Some background about this.
> On original qsdk ipq8064 based firmware there was a big separation from
> boot partition and user partition. With boot partition we refer to
> partition used to init the router (bootloader, spm firmware and other
> internal stuff) With user partition we refer to linux partition and data
> partition not used to init the router.
> When someone had to write to these boot partition a special mode was
> needed, to switch the nand driver to this special configuration.
> 
> Upstream version of the nandc driver totally dropped this and the result
> is that if someone try to read data from these partition a CRC warning
> is printed and if someone try to write that (if for example someone
> wants to replace the bootloader) result is a broken system as the data
> is badly written.
> 
> This series comes to fix this.
> 
> A user can declare offset and size of these special partition using the
> qcom,boot-pages binding.
> 
> An initial implementation of this assumed that the boot-pages started
> from the start of the nand but we discover that some device have backup
> of these special partition and we can have situation where we have this
> partition scheme
> - APPSBL (require special mode)
> - APPSBLENV (doesn't require special mode)
> - ART
> - APPSBLBK (back of APPSBL require special mode)
> - APPSBLENVBK (back of APPSBLENV doesn't require special mode)
> With this configuration we need to declare sparse boot page and we can't
> assume boot-pages always starts from the start of the nand.
> 
> A user can use this form to declare sparse boot pages
> qcom,boot-pages = <0x0 0x0c80000 0x0c80000 0x0500000>;
> 
> The driver internally will parse this array, convert it to nand pages
> and check internally on every read/write if this special configuration
> should used for that page or the normal one.
> 
> The reason for all of this is that qcom FOR SOME REASON, disable ECC for
> spare data only for these boot partition and we need to reflect this
> special configuration to mute these warning and to permit actually
> writing to these pages.

Manivannan, any feedback on this?

> 
> v4:
> - Fix wrong compatible set for boot-pages (ipq8074 instead of ipq806x)
> v3:
> - Fix typo in Docmunetation commit desription
> - Add items description for uint32-matrix
> v2:
> - Add fixes from Krzysztof in Documentation
> 
> Ansuel Smith (2):
>   mtd: nand: raw: qcom_nandc: add support for unprotected spare data
>     pages
>   dt-bindings: mtd: qcom_nandc: document qcom,boot-pages binding
> 
>  .../devicetree/bindings/mtd/qcom,nandc.yaml   |  26 +++
>  drivers/mtd/nand/raw/qcom_nandc.c             | 148 +++++++++++++++++-
>  2 files changed, 169 insertions(+), 5 deletions(-)
> 

Thanks,
Miquèl
Manivannan Sadhasivam June 3, 2022, 2:34 p.m. UTC | #2
On Fri, Jun 03, 2022 at 03:49:34PM +0200, Miquel Raynal wrote:
> Hi,
> 
> ansuelsmth@gmail.com wrote on Thu, 19 May 2022 21:01:10 +0200:
> 
> > Some background about this.
> > On original qsdk ipq8064 based firmware there was a big separation from
> > boot partition and user partition. With boot partition we refer to
> > partition used to init the router (bootloader, spm firmware and other
> > internal stuff) With user partition we refer to linux partition and data
> > partition not used to init the router.
> > When someone had to write to these boot partition a special mode was
> > needed, to switch the nand driver to this special configuration.
> > 
> > Upstream version of the nandc driver totally dropped this and the result
> > is that if someone try to read data from these partition a CRC warning
> > is printed and if someone try to write that (if for example someone
> > wants to replace the bootloader) result is a broken system as the data
> > is badly written.
> > 
> > This series comes to fix this.
> > 
> > A user can declare offset and size of these special partition using the
> > qcom,boot-pages binding.
> > 
> > An initial implementation of this assumed that the boot-pages started
> > from the start of the nand but we discover that some device have backup
> > of these special partition and we can have situation where we have this
> > partition scheme
> > - APPSBL (require special mode)
> > - APPSBLENV (doesn't require special mode)
> > - ART
> > - APPSBLBK (back of APPSBL require special mode)
> > - APPSBLENVBK (back of APPSBLENV doesn't require special mode)
> > With this configuration we need to declare sparse boot page and we can't
> > assume boot-pages always starts from the start of the nand.
> > 
> > A user can use this form to declare sparse boot pages
> > qcom,boot-pages = <0x0 0x0c80000 0x0c80000 0x0500000>;
> > 
> > The driver internally will parse this array, convert it to nand pages
> > and check internally on every read/write if this special configuration
> > should used for that page or the normal one.
> > 
> > The reason for all of this is that qcom FOR SOME REASON, disable ECC for
> > spare data only for these boot partition and we need to reflect this
> > special configuration to mute these warning and to permit actually
> > writing to these pages.
> 
> Manivannan, any feedback on this?
> 

Sorry for the delay. I will check internally on some of the unknown
implementations mentioned and provide my comments.

Thanks,
Mani

> > 
> > v4:
> > - Fix wrong compatible set for boot-pages (ipq8074 instead of ipq806x)
> > v3:
> > - Fix typo in Docmunetation commit desription
> > - Add items description for uint32-matrix
> > v2:
> > - Add fixes from Krzysztof in Documentation
> > 
> > Ansuel Smith (2):
> >   mtd: nand: raw: qcom_nandc: add support for unprotected spare data
> >     pages
> >   dt-bindings: mtd: qcom_nandc: document qcom,boot-pages binding
> > 
> >  .../devicetree/bindings/mtd/qcom,nandc.yaml   |  26 +++
> >  drivers/mtd/nand/raw/qcom_nandc.c             | 148 +++++++++++++++++-
> >  2 files changed, 169 insertions(+), 5 deletions(-)
> > 
> 
> Thanks,
> Miquèl
Manivannan Sadhasivam June 3, 2022, 3:18 p.m. UTC | #3
On Thu, May 19, 2022 at 09:01:10PM +0200, Ansuel Smith wrote:
> Some background about this.
> On original qsdk ipq8064 based firmware there was a big separation from
> boot partition and user partition. With boot partition we refer to
> partition used to init the router (bootloader, spm firmware and other
> internal stuff) With user partition we refer to linux partition and data
> partition not used to init the router.
> When someone had to write to these boot partition a special mode was
> needed, to switch the nand driver to this special configuration.
> 
> Upstream version of the nandc driver totally dropped this and the result
> is that if someone try to read data from these partition a CRC warning
> is printed and if someone try to write that (if for example someone
> wants to replace the bootloader) result is a broken system as the data
> is badly written.
> 

Can you please point me to the downstream/vendor driver that has this
implementation?

Thanks,
Mani

> This series comes to fix this.
> 
> A user can declare offset and size of these special partition using the
> qcom,boot-pages binding.
> 
> An initial implementation of this assumed that the boot-pages started
> from the start of the nand but we discover that some device have backup
> of these special partition and we can have situation where we have this
> partition scheme
> - APPSBL (require special mode)
> - APPSBLENV (doesn't require special mode)
> - ART
> - APPSBLBK (back of APPSBL require special mode)
> - APPSBLENVBK (back of APPSBLENV doesn't require special mode)
> With this configuration we need to declare sparse boot page and we can't
> assume boot-pages always starts from the start of the nand.
> 
> A user can use this form to declare sparse boot pages
> qcom,boot-pages = <0x0 0x0c80000 0x0c80000 0x0500000>;
> 
> The driver internally will parse this array, convert it to nand pages
> and check internally on every read/write if this special configuration
> should used for that page or the normal one.
> 
> The reason for all of this is that qcom FOR SOME REASON, disable ECC for
> spare data only for these boot partition and we need to reflect this
> special configuration to mute these warning and to permit actually
> writing to these pages.
> 
> v4:
> - Fix wrong compatible set for boot-pages (ipq8074 instead of ipq806x)
> v3:
> - Fix typo in Docmunetation commit desription
> - Add items description for uint32-matrix
> v2:
> - Add fixes from Krzysztof in Documentation
> 
> Ansuel Smith (2):
>   mtd: nand: raw: qcom_nandc: add support for unprotected spare data
>     pages
>   dt-bindings: mtd: qcom_nandc: document qcom,boot-pages binding
> 
>  .../devicetree/bindings/mtd/qcom,nandc.yaml   |  26 +++
>  drivers/mtd/nand/raw/qcom_nandc.c             | 148 +++++++++++++++++-
>  2 files changed, 169 insertions(+), 5 deletions(-)
> 
> -- 
> 2.34.1
>
Christian Marangi June 3, 2022, 3:23 p.m. UTC | #4
On Fri, Jun 03, 2022 at 08:48:06PM +0530, Manivannan Sadhasivam wrote:
> On Thu, May 19, 2022 at 09:01:10PM +0200, Ansuel Smith wrote:
> > Some background about this.
> > On original qsdk ipq8064 based firmware there was a big separation from
> > boot partition and user partition. With boot partition we refer to
> > partition used to init the router (bootloader, spm firmware and other
> > internal stuff) With user partition we refer to linux partition and data
> > partition not used to init the router.
> > When someone had to write to these boot partition a special mode was
> > needed, to switch the nand driver to this special configuration.
> > 
> > Upstream version of the nandc driver totally dropped this and the result
> > is that if someone try to read data from these partition a CRC warning
> > is printed and if someone try to write that (if for example someone
> > wants to replace the bootloader) result is a broken system as the data
> > is badly written.
> > 
> 
> Can you please point me to the downstream/vendor driver that has this
> implementation?
> 
> Thanks,
> Mani
>

Sure, is it good if I give you a gist link with the source of driver?

> > This series comes to fix this.
> > 
> > A user can declare offset and size of these special partition using the
> > qcom,boot-pages binding.
> > 
> > An initial implementation of this assumed that the boot-pages started
> > from the start of the nand but we discover that some device have backup
> > of these special partition and we can have situation where we have this
> > partition scheme
> > - APPSBL (require special mode)
> > - APPSBLENV (doesn't require special mode)
> > - ART
> > - APPSBLBK (back of APPSBL require special mode)
> > - APPSBLENVBK (back of APPSBLENV doesn't require special mode)
> > With this configuration we need to declare sparse boot page and we can't
> > assume boot-pages always starts from the start of the nand.
> > 
> > A user can use this form to declare sparse boot pages
> > qcom,boot-pages = <0x0 0x0c80000 0x0c80000 0x0500000>;
> > 
> > The driver internally will parse this array, convert it to nand pages
> > and check internally on every read/write if this special configuration
> > should used for that page or the normal one.
> > 
> > The reason for all of this is that qcom FOR SOME REASON, disable ECC for
> > spare data only for these boot partition and we need to reflect this
> > special configuration to mute these warning and to permit actually
> > writing to these pages.
> > 
> > v4:
> > - Fix wrong compatible set for boot-pages (ipq8074 instead of ipq806x)
> > v3:
> > - Fix typo in Docmunetation commit desription
> > - Add items description for uint32-matrix
> > v2:
> > - Add fixes from Krzysztof in Documentation
> > 
> > Ansuel Smith (2):
> >   mtd: nand: raw: qcom_nandc: add support for unprotected spare data
> >     pages
> >   dt-bindings: mtd: qcom_nandc: document qcom,boot-pages binding
> > 
> >  .../devicetree/bindings/mtd/qcom,nandc.yaml   |  26 +++
> >  drivers/mtd/nand/raw/qcom_nandc.c             | 148 +++++++++++++++++-
> >  2 files changed, 169 insertions(+), 5 deletions(-)
> > 
> > -- 
> > 2.34.1
> > 
> 
> -- 
> மணிவண்ணன் சதாசிவம்
Christian Marangi June 3, 2022, 3:25 p.m. UTC | #5
On Fri, Jun 03, 2022 at 08:48:06PM +0530, Manivannan Sadhasivam wrote:
> On Thu, May 19, 2022 at 09:01:10PM +0200, Ansuel Smith wrote:
> > Some background about this.
> > On original qsdk ipq8064 based firmware there was a big separation from
> > boot partition and user partition. With boot partition we refer to
> > partition used to init the router (bootloader, spm firmware and other
> > internal stuff) With user partition we refer to linux partition and data
> > partition not used to init the router.
> > When someone had to write to these boot partition a special mode was
> > needed, to switch the nand driver to this special configuration.
> > 
> > Upstream version of the nandc driver totally dropped this and the result
> > is that if someone try to read data from these partition a CRC warning
> > is printed and if someone try to write that (if for example someone
> > wants to replace the bootloader) result is a broken system as the data
> > is badly written.
> > 
> 
> Can you please point me to the downstream/vendor driver that has this
> implementation?
> 
> Thanks,
> Mani
>

Actually found the repo...This is the link [1].

My implementation is a variant of this since originally they used a
sysfs entry to swap the ecc configuration.

[1] https://github.com/marxfang/ipq807x-spf100-cs/blob/master/qsdk/qca/src/linux-4.4/drivers/mtd/nand/qcom_nandc.c

> > This series comes to fix this.
> > 
> > A user can declare offset and size of these special partition using the
> > qcom,boot-pages binding.
> > 
> > An initial implementation of this assumed that the boot-pages started
> > from the start of the nand but we discover that some device have backup
> > of these special partition and we can have situation where we have this
> > partition scheme
> > - APPSBL (require special mode)
> > - APPSBLENV (doesn't require special mode)
> > - ART
> > - APPSBLBK (back of APPSBL require special mode)
> > - APPSBLENVBK (back of APPSBLENV doesn't require special mode)
> > With this configuration we need to declare sparse boot page and we can't
> > assume boot-pages always starts from the start of the nand.
> > 
> > A user can use this form to declare sparse boot pages
> > qcom,boot-pages = <0x0 0x0c80000 0x0c80000 0x0500000>;
> > 
> > The driver internally will parse this array, convert it to nand pages
> > and check internally on every read/write if this special configuration
> > should used for that page or the normal one.
> > 
> > The reason for all of this is that qcom FOR SOME REASON, disable ECC for
> > spare data only for these boot partition and we need to reflect this
> > special configuration to mute these warning and to permit actually
> > writing to these pages.
> > 
> > v4:
> > - Fix wrong compatible set for boot-pages (ipq8074 instead of ipq806x)
> > v3:
> > - Fix typo in Docmunetation commit desription
> > - Add items description for uint32-matrix
> > v2:
> > - Add fixes from Krzysztof in Documentation
> > 
> > Ansuel Smith (2):
> >   mtd: nand: raw: qcom_nandc: add support for unprotected spare data
> >     pages
> >   dt-bindings: mtd: qcom_nandc: document qcom,boot-pages binding
> > 
> >  .../devicetree/bindings/mtd/qcom,nandc.yaml   |  26 +++
> >  drivers/mtd/nand/raw/qcom_nandc.c             | 148 +++++++++++++++++-
> >  2 files changed, 169 insertions(+), 5 deletions(-)
> > 
> > -- 
> > 2.34.1
> > 
> 
> -- 
> மணிவண்ணன் சதாசிவம்