diff mbox

[RFC] ARM: dts: meson8b: add reserved memory zones

Message ID 20170929104132.18138-1-linus.luessing@c0d3.blue (mailing list archive)
State Not Applicable
Headers show

Commit Message

Linus Lüssing Sept. 29, 2017, 10:41 a.m. UTC
So far, the stress-ng tool for instance quickly resulted in a silent
freeze of the system with no prior notice on a serial console when
running its filesystem or memory stressor classes.

Even with a panic-on-OOM and reboot-on-panic (vm.panic_on_oom=1,
kernel.panic=10) configured, the system would neither reboot nor
would the OOM killer get any chance to otherwise do its job.

By copying the reserved memory sections used for meson8 already,
stress-ng was able to run on an Odroid C1+ just fine for several hours,
the OOM killer was able to do kill processes again and if configured
would successfully trigger a reboot of the system.

Reported-by: Linus Lüssing <linus.luessing@c0d3.blue>

---
The following stress-ng command worked fine now:
$ stress-ng -v --sequential 0 -t 120s --exclude sysfs,opcode --metrics
(5 hours runtime, tested on an Odroid C1+ with an 4.14-rc1 kernel + SMP
+ USB DTS patches)

I do not know why these memory regions would need to be reserved, I
copied them blindly from mesno8.dtsi. Furthermore I'm a little confused
because the Hardkernel sources seem to reserver a totally different
memory region (16MB from the region at 80MB to 96MB):

https://github.com/hardkernel/linux/blob/odroidc-3.10.y/arch/arm/boot/dts/meson8b_odroidc.dts#L58

If anyone more experienced and knowledege in this would like to claim
authorship and responsibility for this patch or would like to suggest
an alternative one then please feel free to do so.
---
 arch/arm/boot/dts/meson8b.dtsi | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

Comments

Linus Lüssing Sept. 29, 2017, 11:01 a.m. UTC | #1
On Fri, Sep 29, 2017 at 12:41:32PM +0200, Linus Lüssing wrote:
> The following stress-ng command worked fine now:
> $ stress-ng -v --sequential 0 -t 120s --exclude sysfs,opcode --metrics
> (5 hours runtime, tested on an Odroid C1+ with an 4.14-rc1 kernel + SMP
> + USB DTS patches)

Full log file (6.3MB) here:
http://metameute.de/~tux/linux/amlogic/stress-ng-odroidc1%2b-with-resmem%2bsmp-log.txt
Martin Blumenstingl Sept. 29, 2017, 6:49 p.m. UTC | #2
Hi Linus,

On Fri, Sep 29, 2017 at 12:41 PM, Linus Lüssing
<linus.luessing@c0d3.blue> wrote:
> So far, the stress-ng tool for instance quickly resulted in a silent
> freeze of the system with no prior notice on a serial console when
> running its filesystem or memory stressor classes.
>
> Even with a panic-on-OOM and reboot-on-panic (vm.panic_on_oom=1,
> kernel.panic=10) configured, the system would neither reboot nor
> would the OOM killer get any chance to otherwise do its job.
>
> By copying the reserved memory sections used for meson8 already,
> stress-ng was able to run on an Odroid C1+ just fine for several hours,
> the OOM killer was able to do kill processes again and if configured
> would successfully trigger a reboot of the system.
>
> Reported-by: Linus Lüssing <linus.luessing@c0d3.blue>
maybe you could add a reference to my commit (8a7f0c52e8a0) and state
that your patch solves the same problem on the Meson8b platform
with that:

Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

>
> ---
> The following stress-ng command worked fine now:
> $ stress-ng -v --sequential 0 -t 120s --exclude sysfs,opcode --metrics
> (5 hours runtime, tested on an Odroid C1+ with an 4.14-rc1 kernel + SMP
> + USB DTS patches)
great to hear that this works for you!

>
> I do not know why these memory regions would need to be reserved, I
> copied them blindly from mesno8.dtsi. Furthermore I'm a little confused
> because the Hardkernel sources seem to reserver a totally different
> memory region (16MB from the region at 80MB to 96MB):
>
> https://github.com/hardkernel/linux/blob/odroidc-3.10.y/arch/arm/boot/dts/meson8b_odroidc.dts#L58
I think that specific region is used for the video hardware, see [0]
we might need that as soon as we have a HDMI driver

>
> If anyone more experienced and knowledege in this would like to claim
> authorship and responsibility for this patch or would like to suggest
> an alternative one then please feel free to do so.
> ---
>  arch/arm/boot/dts/meson8b.dtsi | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi
> index b9b6600..aa5c79c 100644
> --- a/arch/arm/boot/dts/meson8b.dtsi
> +++ b/arch/arm/boot/dts/meson8b.dtsi
> @@ -92,6 +92,33 @@
>                 };
>         };
>
> +       reserved-memory {
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               ranges;
> +
> +               /* 2 MiB reserved for Hardware ROM Firmware? */
> +               hwrom@0 {
> +                       reg = <0x0 0x200000>;
> +                       no-map;
> +               };
> +
> +               /*
> +                * 1 MiB reserved for the "ARM Power Firmware": this is ARM
> +                * code which is responsible for system suspend. It loads a
> +                * piece of ARC code ("arc_power" in the vendor u-boot tree)
> +                * into SRAM, executes that and shuts down the (last) ARM core.
> +                * The arc_power firmware then checks various wakeup sources
> +                * (IR remote receiver, HDMI CEC, WIFI and Bluetooth wakeup or
> +                * simply the power key) and re-starts the ARM core once it
> +                * detects a wakeup request.
> +                */
> +               power-firmware@4f00000 {
> +                       reg = <0x4f00000 0x100000>;
> +                       no-map;
> +               };
> +       };
> +
>         scu@c4300000 {
>                 compatible = "arm,cortex-a5-scu";
>                 reg = <0xc4300000 0x100>;
> --
> 2.1.4
>
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic


[0] https://github.com/endlessm/linux-meson/blob/c173f4653186870f08dc9131b5d82ef4ab4151e1/arch/arm/kernel/devtree.c#L253
Linus Lüssing Oct. 1, 2017, 11:44 a.m. UTC | #3
On Fri, Sep 29, 2017 at 08:49:25PM +0200, Martin Blumenstingl wrote:
> maybe you could add a reference to my commit (8a7f0c52e8a0) and state
> that your patch solves the same problem on the Meson8b platform
> with that:
> 
> Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

Ok, will do. Before resubmitting with a Signed-off-by, I'd like to
understand a little more though.


I retested with only the 2MB reservation at the start and that
seems to be enough to fix the silent freezes.

I tried to dig into the Amlogic/Hardkernel source code and found
a PHY_OFFSET define which seems to reserve the 2MB there, if I'm
not mistaken:

https://github.com/hardkernel/linux/blob/odroidc-3.10.y/arch/arm/mach-meson8b/include/mach/memory.h#L26

Unfortunately, there is no comment in there regarding why this
might be needed. It seems that Hardkernel just copied it from the
original Amlogic sources in this commit:

    e647d67 AMLogic's patchset of 'amlogic-20140823'

I'd reference the 2MB reservation in the original Amlogic
source code somehow in the commit message if that makes sense?
Just so people know why 2MB and not xx MB.

I also had a closer look at the S805 datasheet [0]. The Memory Map
(chapter 4) has an extra "Region (boot)" column. The only
difference to the "Region (Normal)" column is the very first
entry: A 16MB region from 0x00000000 to 0x00FFFF.

Which made me wonder whether the memory we'd reserve should maybe
be 16MB instead of 2MB? Or maybe we should tell the S805 somehow
that we would like to switch from "boot" to "normal" mode?
Maybe disable the "ROM BOOT ROM clock" noted in chapter 6.3.2?


Regarding the ARM Power Firmware, any recommended readings? Also,
could I somehow check whether this is used on the Odroid C1+
(maybe some live memory dump while in uboot)?

If this part is needed, I would maybe put it into a separate
patch, as seems unrelated to the silent freezes.

Regards, Linus

[0]: https://dn.odroid.com/S805/Datasheet/S805_Datasheet%20V0.8%2020150126.pdf
Martin Blumenstingl Oct. 1, 2017, 12:12 p.m. UTC | #4
Hi Linus,

On Sun, Oct 1, 2017 at 1:44 PM, Linus Lüssing <linus.luessing@c0d3.blue> wrote:
> On Fri, Sep 29, 2017 at 08:49:25PM +0200, Martin Blumenstingl wrote:
>> maybe you could add a reference to my commit (8a7f0c52e8a0) and state
>> that your patch solves the same problem on the Meson8b platform
>> with that:
>>
>> Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>
> Ok, will do. Before resubmitting with a Signed-off-by, I'd like to
> understand a little more though.
>
>
> I retested with only the 2MB reservation at the start and that
> seems to be enough to fix the silent freezes.
this matches my observations (looking back at 8a7f0c52e8a0 I think my
description should have been a bit clearer).

> I tried to dig into the Amlogic/Hardkernel source code and found
> a PHY_OFFSET define which seems to reserve the 2MB there, if I'm
> not mistaken:
>
> https://github.com/hardkernel/linux/blob/odroidc-3.10.y/arch/arm/mach-meson8b/include/mach/memory.h#L26
interesting, I can't remember that I've seen that

> Unfortunately, there is no comment in there regarding why this
> might be needed. It seems that Hardkernel just copied it from the
> original Amlogic sources in this commit:
>
>     e647d67 AMLogic's patchset of 'amlogic-20140823'
>
> I'd reference the 2MB reservation in the original Amlogic
> source code somehow in the commit message if that makes sense?
> Just so people know why 2MB and not xx MB.
yes, this is what I typically how I do it: describe why it's needed
and where I found it in the Amlogic GPL kernel sources (if I did find
it)

> I also had a closer look at the S805 datasheet [0]. The Memory Map
> (chapter 4) has an extra "Region (boot)" column. The only
> difference to the "Region (Normal)" column is the very first
> entry: A 16MB region from 0x00000000 to 0x00FFFF.
>
> Which made me wonder whether the memory we'd reserve should maybe
> be 16MB instead of 2MB? Or maybe we should tell the S805 somehow
> that we would like to switch from "boot" to "normal" mode?
> Maybe disable the "ROM BOOT ROM clock" noted in chapter 6.3.2?
I neither know if extending the memory region to cover the first 16MiB
improves anything nor if we need to switch to "normal" mode manually
(or if that's already done by the bootloader)

> Regarding the ARM Power Firmware, any recommended readings? Also,
> could I somehow check whether this is used on the Odroid C1+
> (maybe some live memory dump while in uboot)?
>
> If this part is needed, I would maybe put it into a separate
> patch, as seems unrelated to the silent freezes.
the "ARM Power Firmware" is needed for system suspend, see my Meson8
commit 8a7f0c52e8a0 and [0]


Regards,
Martin


[0] https://www.spinics.net/lists/arm-kernel/msg579950.html
diff mbox

Patch

diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi
index b9b6600..aa5c79c 100644
--- a/arch/arm/boot/dts/meson8b.dtsi
+++ b/arch/arm/boot/dts/meson8b.dtsi
@@ -92,6 +92,33 @@ 
 		};
 	};
 
+	reserved-memory {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		/* 2 MiB reserved for Hardware ROM Firmware? */
+		hwrom@0 {
+			reg = <0x0 0x200000>;
+			no-map;
+		};
+
+		/*
+		 * 1 MiB reserved for the "ARM Power Firmware": this is ARM
+		 * code which is responsible for system suspend. It loads a
+		 * piece of ARC code ("arc_power" in the vendor u-boot tree)
+		 * into SRAM, executes that and shuts down the (last) ARM core.
+		 * The arc_power firmware then checks various wakeup sources
+		 * (IR remote receiver, HDMI CEC, WIFI and Bluetooth wakeup or
+		 * simply the power key) and re-starts the ARM core once it
+		 * detects a wakeup request.
+		 */
+		power-firmware@4f00000 {
+			reg = <0x4f00000 0x100000>;
+			no-map;
+		};
+	};
+
 	scu@c4300000 {
 		compatible = "arm,cortex-a5-scu";
 		reg = <0xc4300000 0x100>;