diff mbox series

arm64: dts: renesas: beacon: Fix memory corruption from TF-A

Message ID 20210924171905.347115-1-aford173@gmail.com (mailing list archive)
State Under Review
Delegated to: Geert Uytterhoeven
Headers show
Series arm64: dts: renesas: beacon: Fix memory corruption from TF-A | expand

Commit Message

Adam Ford Sept. 24, 2021, 5:19 p.m. UTC 
Trusted Firmware allocates a chunk of memory for a lossy compressor
which makes the memory unavailable to Linux and any attempts to read/write
from Linux result in memory corruption or a crash.  Fix this by reserving
the section of memory marked as unavailable by TF-A.

Fixes: a1d8a344f1ca ("arm64: dts: renesas: Introduce r8a774a1-beacon-rzg2m-kit")
Signed-off-by: Adam Ford <aford173@gmail.com>

Comments

Geert Uytterhoeven Oct. 5, 2021, 2:30 p.m. UTC | #1 
Hi Adam,

On Fri, Sep 24, 2021 at 7:19 PM Adam Ford <aford173@gmail.com> wrote:
> Trusted Firmware allocates a chunk of memory for a lossy compressor
> which makes the memory unavailable to Linux and any attempts to read/write
> from Linux result in memory corruption or a crash.  Fix this by reserving
> the section of memory marked as unavailable by TF-A.
>
> Fixes: a1d8a344f1ca ("arm64: dts: renesas: Introduce r8a774a1-beacon-rzg2m-kit")
> Signed-off-by: Adam Ford <aford173@gmail.com>

Thanks for your patch!

> --- a/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
> +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
> @@ -13,6 +13,17 @@ memory@48000000 {
>                 reg = <0x0 0x48000000 0x0 0x78000000>;
>         };
>
> +       reserved-memory {
> +               #address-cells = <2>;
> +               #size-cells = <2>;
> +               ranges;
> +
> +               lossy_decompress: lossy-decompress@54000000 {
> +                       reg = <0 0x54000000 0 0x03000000>; /* Reserved by TF-A */
> +                       no-map;
> +               };
> +       };
> +
>         osc_32k: osc_32k {
>                 compatible = "fixed-clock";
>                 #clock-cells = <0>;

I believe these days it's the responsibility of TF-A to create these nodes
in the DTB, and pass that to U-Boot.

What bootloader are you running?
Does "fdt addr $fdtcontroladdr ; fdt print" show the area as reserved?
Does TF-A print something about reserving the memory?

Thanks!

Gr{oetje,eeting}s,

                        Geert
Adam Ford Oct. 9, 2021, 11:13 a.m. UTC | #2 
On Tue, Oct 5, 2021 at 9:30 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Adam,
>
> On Fri, Sep 24, 2021 at 7:19 PM Adam Ford <aford173@gmail.com> wrote:
> > Trusted Firmware allocates a chunk of memory for a lossy compressor
> > which makes the memory unavailable to Linux and any attempts to read/write
> > from Linux result in memory corruption or a crash.  Fix this by reserving
> > the section of memory marked as unavailable by TF-A.
> >
> > Fixes: a1d8a344f1ca ("arm64: dts: renesas: Introduce r8a774a1-beacon-rzg2m-kit")
> > Signed-off-by: Adam Ford <aford173@gmail.com>
>
> Thanks for your patch!
>
> > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
> > @@ -13,6 +13,17 @@ memory@48000000 {
> >                 reg = <0x0 0x48000000 0x0 0x78000000>;
> >         };
> >
> > +       reserved-memory {
> > +               #address-cells = <2>;
> > +               #size-cells = <2>;
> > +               ranges;
> > +
> > +               lossy_decompress: lossy-decompress@54000000 {
> > +                       reg = <0 0x54000000 0 0x03000000>; /* Reserved by TF-A */
> > +                       no-map;
> > +               };
> > +       };
> > +
> >         osc_32k: osc_32k {
> >                 compatible = "fixed-clock";
> >                 #clock-cells = <0>;
>
> I believe these days it's the responsibility of TF-A to create these nodes
> in the DTB, and pass that to U-Boot.
>
> What bootloader are you running?

U-Boot 2021.04

> Does "fdt addr $fdtcontroladdr ; fdt print" show the area as reserved?

I see the memory nodes, but I don't see any reserved memory being
carved out of it.

> Does TF-A print something about reserving the memory?

Yes:

NOTICE:  BL2: DRAM Split is 2ch
NOTICE:  BL2: QoS is default setting(rev.0.19)
NOTICE:  BL2: DRAM refresh interval 1.95 usec
NOTICE:  BL2: Periodic Write DQ Training
NOTICE:  BL2: DRAM don't have ECC configuration
NOTICE:  BL2: CH0: 400000000 - 47fffffff, 2 GiB
NOTICE:  BL2: CH2: 600000000 - 67fffffff, 2 GiB
NOTICE:  BL2: Lossy Decomp areas
NOTICE:       Entry 0: DCMPAREACRAx:0x80000540 DCMPAREACRBx:0x570
NOTICE:       Entry 1: DCMPAREACRAx:0x40000000 DCMPAREACRBx:0x0
NOTICE:       Entry 2: DCMPAREACRAx:0x20000000 DCMPAREACRBx:0x0
NOTICE:  BL2: FDT at 0xe631e588

If I set the fdt address to 0xe631e588, I can see the memory nodes:

=> fdt addr 0xe631e588
=> fdt  print
/ {
    compatible = "renesas,beacon", "renesas,r8a774a1";
    #size-cells = <0x00000002>;
    #address-cells = <0x00000002>;
    reserved-memory {
        lossy-decompression@54000000 {
        renesas,formats = <0x00000000>;
        no-map;
        reg = <0x00000000 0x54000000 0x00000000 0x03000000>;
        compatible = "renesas,lossy-decompression", "shared-dma-pool";
    };
};
memory@48000000 {
    reg = <0x00000000 0x48000000 0x00000000 0x78000000>;
    device_type = "memory";
};
memory@600000000 {
    reg = <0x00000006 0x00000000 0x00000000 0x80000000>;
    device_type = "memory";
};
};
=>

I'll look into seeing how to append the memory nodes at 0xe631e588
onto the device tree we're loading and passing to the kernel

>
> Thanks!
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
Geert Uytterhoeven Oct. 11, 2021, 7:40 a.m. UTC | #3 
Hi Adam,

On Sat, Oct 9, 2021 at 1:13 PM Adam Ford <aford173@gmail.com> wrote:
> On Tue, Oct 5, 2021 at 9:30 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Fri, Sep 24, 2021 at 7:19 PM Adam Ford <aford173@gmail.com> wrote:
> > > Trusted Firmware allocates a chunk of memory for a lossy compressor
> > > which makes the memory unavailable to Linux and any attempts to read/write
> > > from Linux result in memory corruption or a crash.  Fix this by reserving
> > > the section of memory marked as unavailable by TF-A.
> > >
> > > Fixes: a1d8a344f1ca ("arm64: dts: renesas: Introduce r8a774a1-beacon-rzg2m-kit")
> > > Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > Thanks for your patch!
> >
> > > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
> > > @@ -13,6 +13,17 @@ memory@48000000 {
> > >                 reg = <0x0 0x48000000 0x0 0x78000000>;
> > >         };
> > >
> > > +       reserved-memory {
> > > +               #address-cells = <2>;
> > > +               #size-cells = <2>;
> > > +               ranges;
> > > +
> > > +               lossy_decompress: lossy-decompress@54000000 {
> > > +                       reg = <0 0x54000000 0 0x03000000>; /* Reserved by TF-A */
> > > +                       no-map;
> > > +               };
> > > +       };
> > > +
> > >         osc_32k: osc_32k {
> > >                 compatible = "fixed-clock";
> > >                 #clock-cells = <0>;
> >
> > I believe these days it's the responsibility of TF-A to create these nodes
> > in the DTB, and pass that to U-Boot.
> >
> > What bootloader are you running?
>
> U-Boot 2021.04

Hmm, that's fairly recent. Marek?

> > Does "fdt addr $fdtcontroladdr ; fdt print" show the area as reserved?
>
> I see the memory nodes, but I don't see any reserved memory being
> carved out of it.
>
> > Does TF-A print something about reserving the memory?
>
> Yes:
>
> NOTICE:  BL2: DRAM Split is 2ch
> NOTICE:  BL2: QoS is default setting(rev.0.19)
> NOTICE:  BL2: DRAM refresh interval 1.95 usec
> NOTICE:  BL2: Periodic Write DQ Training
> NOTICE:  BL2: DRAM don't have ECC configuration
> NOTICE:  BL2: CH0: 400000000 - 47fffffff, 2 GiB
> NOTICE:  BL2: CH2: 600000000 - 67fffffff, 2 GiB
> NOTICE:  BL2: Lossy Decomp areas
> NOTICE:       Entry 0: DCMPAREACRAx:0x80000540 DCMPAREACRBx:0x570
> NOTICE:       Entry 1: DCMPAREACRAx:0x40000000 DCMPAREACRBx:0x0
> NOTICE:       Entry 2: DCMPAREACRAx:0x20000000 DCMPAREACRBx:0x0
> NOTICE:  BL2: FDT at 0xe631e588

OK.

> If I set the fdt address to 0xe631e588, I can see the memory nodes:
>
> => fdt addr 0xe631e588
> => fdt  print
> / {
>     compatible = "renesas,beacon", "renesas,r8a774a1";
>     #size-cells = <0x00000002>;
>     #address-cells = <0x00000002>;
>     reserved-memory {
>         lossy-decompression@54000000 {
>         renesas,formats = <0x00000000>;
>         no-map;
>         reg = <0x00000000 0x54000000 0x00000000 0x03000000>;
>         compatible = "renesas,lossy-decompression", "shared-dma-pool";
>     };

Good, so ATF passed the reserved region to U-Boot.

> };
> memory@48000000 {
>     reg = <0x00000000 0x48000000 0x00000000 0x78000000>;
>     device_type = "memory";
> };
> memory@600000000 {
>     reg = <0x00000006 0x00000000 0x00000000 0x80000000>;
>     device_type = "memory";
> };
> };
> =>
>
> I'll look into seeing how to append the memory nodes at 0xe631e588
> onto the device tree we're loading and passing to the kernel

Now U-Boot still has to pass this to the kernel...

Gr{oetje,eeting}s,

                        Geert
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
index 090dc9c4f57b..e01cb30e03e8 100644
--- a/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
@@ -13,6 +13,17 @@  memory@48000000 {
 		reg = <0x0 0x48000000 0x0 0x78000000>;
 	};
 
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		lossy_decompress: lossy-decompress@54000000 {
+			reg = <0 0x54000000 0 0x03000000>; /* Reserved by TF-A */
+			no-map;
+		};
+	};
+
 	osc_32k: osc_32k {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;