| Message ID | 1444365376-10728-2-git-send-email-leo.yan@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Oct 8, 2015 at 11:36 PM, Leo Yan <leo.yan@linaro.org> wrote: > On Hi6220, below memory regions in DDR have specific purpose: > > 0x05e0,0000 - 0x05ef,ffff: For MCU firmware using at runtime; > 0x06df,f000 - 0x06df,ffff: For mailbox message data; > 0x0740,f000 - 0x0740,ffff: For MCU firmware's section; > 0x3e00,0000 - 0x3fff,ffff: For OP-TEE. > > This patch reserves these memory regions in DT. > > Signed-off-by: Leo Yan <leo.yan@linaro.org> > --- > arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > index e36a539..e3f4cb3 100644 > --- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > +++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > @@ -7,9 +7,6 @@ > > /dts-v1/; > > -/*Reserved 1MB memory for MCU*/ > -/memreserve/ 0x05e00000 0x00100000; > - Why does memreserve not work for you? You can have multiple entries. > #include "hi6220.dtsi" > > / { > @@ -24,8 +21,19 @@ > stdout-path = "serial0:115200n8"; > }; > > + /* > + * Reserve below regions from memory node: > + * > + * - 0x05e0,0000 - 0x05ef,ffff: MCU firmware runtime using > + * - 0x06df,f000 - 0x06df,ffff: Mailbox message data > + * - 0x0740,f000 - 0x0740,ffff: MCU firmware section > + * - 0x3e00,0000 - 0x3fff,ffff: OP-TEE > + */ > memory@0 { > device_type = "memory"; > - reg = <0x0 0x0 0x0 0x40000000>; > + reg = <0x00000000 0x00000000 0x00000000 0x05e00000>, > + <0x00000000 0x05f00000 0x00000000 0x00eff000>, > + <0x00000000 0x06e00000 0x00000000 0x0060f000>, > + <0x00000000 0x07410000 0x00000000 0x36bf0000>; No, don't do this. Please use memreserve or reserved-memory binding[1] or combination of both. Probably reserved-memory if you need the kernel to access some of these regions. Rob [1] Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
On Fri, Oct 09, 2015 at 08:17:16AM -0500, Rob Herring wrote: > On Thu, Oct 8, 2015 at 11:36 PM, Leo Yan <leo.yan@linaro.org> wrote: > > On Hi6220, below memory regions in DDR have specific purpose: > > > > 0x05e0,0000 - 0x05ef,ffff: For MCU firmware using at runtime; > > 0x06df,f000 - 0x06df,ffff: For mailbox message data; > > 0x0740,f000 - 0x0740,ffff: For MCU firmware's section; > > 0x3e00,0000 - 0x3fff,ffff: For OP-TEE. > > > > This patch reserves these memory regions in DT. > > > > Signed-off-by: Leo Yan <leo.yan@linaro.org> > > --- > > arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 16 ++++++++++++---- > > 1 file changed, 12 insertions(+), 4 deletions(-) > > > > diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > > index e36a539..e3f4cb3 100644 > > --- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > > +++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > > @@ -7,9 +7,6 @@ > > > > /dts-v1/; > > > > -/*Reserved 1MB memory for MCU*/ > > -/memreserve/ 0x05e00000 0x00100000; > > - > > Why does memreserve not work for you? You can have multiple entries. > > > #include "hi6220.dtsi" > > > > / { > > @@ -24,8 +21,19 @@ > > stdout-path = "serial0:115200n8"; > > }; > > > > + /* > > + * Reserve below regions from memory node: > > + * > > + * - 0x05e0,0000 - 0x05ef,ffff: MCU firmware runtime using > > + * - 0x06df,f000 - 0x06df,ffff: Mailbox message data > > + * - 0x0740,f000 - 0x0740,ffff: MCU firmware section > > + * - 0x3e00,0000 - 0x3fff,ffff: OP-TEE > > + */ > > memory@0 { > > device_type = "memory"; > > - reg = <0x0 0x0 0x0 0x40000000>; > > + reg = <0x00000000 0x00000000 0x00000000 0x05e00000>, > > + <0x00000000 0x05f00000 0x00000000 0x00eff000>, > > + <0x00000000 0x06e00000 0x00000000 0x0060f000>, > > + <0x00000000 0x07410000 0x00000000 0x36bf0000>; > > No, don't do this. Please use memreserve or reserved-memory binding[1] > or combination of both. Probably reserved-memory if you need the > kernel to access some of these regions. I disagree at least for those portions owned by the secure world. The kernel shouldn't map those at all, so memreserve isn't appropriate. That covers OP-TEE and the MCU firmware regions, and I'd expec the EFI memory map to not list those as available to the kernel. For the mailbox memory reserved-memory should be OK. Thanks, Mark.
On Fri, Oct 9, 2015 at 8:30 AM, Mark Rutland <mark.rutland@arm.com> wrote: > On Fri, Oct 09, 2015 at 08:17:16AM -0500, Rob Herring wrote: >> On Thu, Oct 8, 2015 at 11:36 PM, Leo Yan <leo.yan@linaro.org> wrote: >> > On Hi6220, below memory regions in DDR have specific purpose: >> > >> > 0x05e0,0000 - 0x05ef,ffff: For MCU firmware using at runtime; >> > 0x06df,f000 - 0x06df,ffff: For mailbox message data; >> > 0x0740,f000 - 0x0740,ffff: For MCU firmware's section; >> > 0x3e00,0000 - 0x3fff,ffff: For OP-TEE. >> > >> > This patch reserves these memory regions in DT. >> > >> > Signed-off-by: Leo Yan <leo.yan@linaro.org> >> > --- >> > arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 16 ++++++++++++---- >> > 1 file changed, 12 insertions(+), 4 deletions(-) >> > >> > diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts >> > index e36a539..e3f4cb3 100644 >> > --- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts >> > +++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts >> > @@ -7,9 +7,6 @@ >> > >> > /dts-v1/; >> > >> > -/*Reserved 1MB memory for MCU*/ >> > -/memreserve/ 0x05e00000 0x00100000; >> > - >> >> Why does memreserve not work for you? You can have multiple entries. >> >> > #include "hi6220.dtsi" >> > >> > / { >> > @@ -24,8 +21,19 @@ >> > stdout-path = "serial0:115200n8"; >> > }; >> > >> > + /* >> > + * Reserve below regions from memory node: >> > + * >> > + * - 0x05e0,0000 - 0x05ef,ffff: MCU firmware runtime using >> > + * - 0x06df,f000 - 0x06df,ffff: Mailbox message data >> > + * - 0x0740,f000 - 0x0740,ffff: MCU firmware section >> > + * - 0x3e00,0000 - 0x3fff,ffff: OP-TEE >> > + */ >> > memory@0 { >> > device_type = "memory"; >> > - reg = <0x0 0x0 0x0 0x40000000>; >> > + reg = <0x00000000 0x00000000 0x00000000 0x05e00000>, >> > + <0x00000000 0x05f00000 0x00000000 0x00eff000>, >> > + <0x00000000 0x06e00000 0x00000000 0x0060f000>, >> > + <0x00000000 0x07410000 0x00000000 0x36bf0000>; >> >> No, don't do this. Please use memreserve or reserved-memory binding[1] >> or combination of both. Probably reserved-memory if you need the >> kernel to access some of these regions. > > I disagree at least for those portions owned by the secure world. The > kernel shouldn't map those at all, so memreserve isn't appropriate. That > covers OP-TEE and the MCU firmware regions, and I'd expec the EFI memory > map to not list those as available to the kernel. I'm fine carving out the beginning or end, but otherwise think memory should correspond to the physical memory. We have a way to describe holes to keep out, so we should use them. If secure world uses the DT, then it would either want to know its region in memory or add the DT data to say what it is using. We need that to be easy to find or easy to set, respectively. The size secure world needs could vary as well. The fact that the kernel maps the memory is the kernel's problem, not a DT problem. > > For the mailbox memory reserved-memory should be OK. That only gets us from 4 regions to 3. Rob
Hi Rob, On Fri, Oct 09, 2015 at 08:50:13AM -0500, Rob Herring wrote: > On Fri, Oct 9, 2015 at 8:30 AM, Mark Rutland <mark.rutland@arm.com> wrote: > > On Fri, Oct 09, 2015 at 08:17:16AM -0500, Rob Herring wrote: > >> On Thu, Oct 8, 2015 at 11:36 PM, Leo Yan <leo.yan@linaro.org> wrote: > >> > On Hi6220, below memory regions in DDR have specific purpose: > >> > > >> > 0x05e0,0000 - 0x05ef,ffff: For MCU firmware using at runtime; > >> > 0x06df,f000 - 0x06df,ffff: For mailbox message data; > >> > 0x0740,f000 - 0x0740,ffff: For MCU firmware's section; > >> > 0x3e00,0000 - 0x3fff,ffff: For OP-TEE. > >> > > >> > This patch reserves these memory regions in DT. > >> > > >> > Signed-off-by: Leo Yan <leo.yan@linaro.org> > >> > --- > >> > arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 16 ++++++++++++---- > >> > 1 file changed, 12 insertions(+), 4 deletions(-) > >> > > >> > diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > >> > index e36a539..e3f4cb3 100644 > >> > --- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > >> > +++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > >> > @@ -7,9 +7,6 @@ > >> > > >> > /dts-v1/; > >> > > >> > -/*Reserved 1MB memory for MCU*/ > >> > -/memreserve/ 0x05e00000 0x00100000; > >> > - > >> > >> Why does memreserve not work for you? You can have multiple entries. > >> > >> > #include "hi6220.dtsi" > >> > > >> > / { > >> > @@ -24,8 +21,19 @@ > >> > stdout-path = "serial0:115200n8"; > >> > }; > >> > > >> > + /* > >> > + * Reserve below regions from memory node: > >> > + * > >> > + * - 0x05e0,0000 - 0x05ef,ffff: MCU firmware runtime using > >> > + * - 0x06df,f000 - 0x06df,ffff: Mailbox message data > >> > + * - 0x0740,f000 - 0x0740,ffff: MCU firmware section > >> > + * - 0x3e00,0000 - 0x3fff,ffff: OP-TEE > >> > + */ > >> > memory@0 { > >> > device_type = "memory"; > >> > - reg = <0x0 0x0 0x0 0x40000000>; > >> > + reg = <0x00000000 0x00000000 0x00000000 0x05e00000>, > >> > + <0x00000000 0x05f00000 0x00000000 0x00eff000>, > >> > + <0x00000000 0x06e00000 0x00000000 0x0060f000>, > >> > + <0x00000000 0x07410000 0x00000000 0x36bf0000>; > >> > >> No, don't do this. Please use memreserve or reserved-memory binding[1] > >> or combination of both. Probably reserved-memory if you need the > >> kernel to access some of these regions. > > > > I disagree at least for those portions owned by the secure world. The > > kernel shouldn't map those at all, so memreserve isn't appropriate. That > > covers OP-TEE and the MCU firmware regions, and I'd expec the EFI memory > > map to not list those as available to the kernel. > > I'm fine carving out the beginning or end, but otherwise think memory > should correspond to the physical memory. We have a way to describe > holes to keep out, so we should use them. If secure world uses the DT, > then it would either want to know its region in memory or add the DT > data to say what it is using. We need that to be easy to find or easy > to set, respectively. The size secure world needs could vary as well. > > The fact that the kernel maps the memory is the kernel's problem, not > a DT problem. > Just give more input here. In previous time, we have long discussion [1]; So actually your suggestion is exactly same what my old patch. From previous discussion, i think here have an assumtion: Use UEFI as bootloader, the kernel will ignore (or remove) memreserve and reserved-memory nodes, so just like Mark said "the EFI memory map to not list those as available to the kernel". My new patch is just to follow this and also make sure they have same behavior for different bootloader (between UEFI and uboot). [1] http://archive.arm.linux.org.uk/lurker/thread/20150819.093735.59724a58.en.html#i20150819.093735.59724a58 Thanks, Leo Yan > > > > For the mailbox memory reserved-memory should be OK. > > That only gets us from 4 regions to 3. > > Rob
On Fri, Oct 9, 2015 at 9:20 AM, Leo Yan <leo.yan@linaro.org> wrote: > Hi Rob, > > On Fri, Oct 09, 2015 at 08:50:13AM -0500, Rob Herring wrote: >> On Fri, Oct 9, 2015 at 8:30 AM, Mark Rutland <mark.rutland@arm.com> wrote: >> > On Fri, Oct 09, 2015 at 08:17:16AM -0500, Rob Herring wrote: >> >> On Thu, Oct 8, 2015 at 11:36 PM, Leo Yan <leo.yan@linaro.org> wrote: >> >> > On Hi6220, below memory regions in DDR have specific purpose: >> >> > >> >> > 0x05e0,0000 - 0x05ef,ffff: For MCU firmware using at runtime; >> >> > 0x06df,f000 - 0x06df,ffff: For mailbox message data; >> >> > 0x0740,f000 - 0x0740,ffff: For MCU firmware's section; >> >> > 0x3e00,0000 - 0x3fff,ffff: For OP-TEE. >> >> > >> >> > This patch reserves these memory regions in DT. >> >> > >> >> > Signed-off-by: Leo Yan <leo.yan@linaro.org> >> >> > --- >> >> > arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 16 ++++++++++++---- >> >> > 1 file changed, 12 insertions(+), 4 deletions(-) >> >> > >> >> > diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts >> >> > index e36a539..e3f4cb3 100644 >> >> > --- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts >> >> > +++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts >> >> > @@ -7,9 +7,6 @@ >> >> > >> >> > /dts-v1/; >> >> > >> >> > -/*Reserved 1MB memory for MCU*/ >> >> > -/memreserve/ 0x05e00000 0x00100000; >> >> > - >> >> >> >> Why does memreserve not work for you? You can have multiple entries. >> >> >> >> > #include "hi6220.dtsi" >> >> > >> >> > / { >> >> > @@ -24,8 +21,19 @@ >> >> > stdout-path = "serial0:115200n8"; >> >> > }; >> >> > >> >> > + /* >> >> > + * Reserve below regions from memory node: >> >> > + * >> >> > + * - 0x05e0,0000 - 0x05ef,ffff: MCU firmware runtime using >> >> > + * - 0x06df,f000 - 0x06df,ffff: Mailbox message data >> >> > + * - 0x0740,f000 - 0x0740,ffff: MCU firmware section >> >> > + * - 0x3e00,0000 - 0x3fff,ffff: OP-TEE >> >> > + */ >> >> > memory@0 { >> >> > device_type = "memory"; >> >> > - reg = <0x0 0x0 0x0 0x40000000>; >> >> > + reg = <0x00000000 0x00000000 0x00000000 0x05e00000>, >> >> > + <0x00000000 0x05f00000 0x00000000 0x00eff000>, >> >> > + <0x00000000 0x06e00000 0x00000000 0x0060f000>, >> >> > + <0x00000000 0x07410000 0x00000000 0x36bf0000>; >> >> >> >> No, don't do this. Please use memreserve or reserved-memory binding[1] >> >> or combination of both. Probably reserved-memory if you need the >> >> kernel to access some of these regions. >> > >> > I disagree at least for those portions owned by the secure world. The >> > kernel shouldn't map those at all, so memreserve isn't appropriate. That >> > covers OP-TEE and the MCU firmware regions, and I'd expec the EFI memory >> > map to not list those as available to the kernel. >> >> I'm fine carving out the beginning or end, but otherwise think memory >> should correspond to the physical memory. We have a way to describe >> holes to keep out, so we should use them. If secure world uses the DT, >> then it would either want to know its region in memory or add the DT >> data to say what it is using. We need that to be easy to find or easy >> to set, respectively. The size secure world needs could vary as well. >> >> The fact that the kernel maps the memory is the kernel's problem, not >> a DT problem. >> > > Just give more input here. In previous time, we have long discussion [1]; > So actually your suggestion is exactly same what my old patch. > > From previous discussion, i think here have an assumtion: Use UEFI as > bootloader, the kernel will ignore (or remove) memreserve and reserved-memory > nodes, so just like Mark said "the EFI memory map to not list those > as available to the kernel". My new patch is just to follow this and > also make sure they have same behavior for different bootloader > (between UEFI and uboot). I've read thru the thread and see 2 main conclusions. Using reserved-memory is problematic since things like grub don't support that. That is fine and we should stick with /mem-reserve/ for now. The other thing is the desire to have the memory presented to the kernel be the same whether it comes from UEFI or DT structures. I can see why there is some desire to have that alignment, but that doesn't really buy us anything. We can't eliminate some code path in the kernel doing so. So I still think that the memory node should reflect all of memory as defined by the h/w and mem-reserve should be used for any software defined reserved regions. Rob
On Wed, Oct 28, 2015 at 11:32:29PM -0500, Rob Herring wrote: > On Fri, Oct 9, 2015 at 9:20 AM, Leo Yan <leo.yan@linaro.org> wrote: > > On Fri, Oct 09, 2015 at 08:50:13AM -0500, Rob Herring wrote: > >> On Fri, Oct 9, 2015 at 8:30 AM, Mark Rutland <mark.rutland@arm.com> wrote: > >> > On Fri, Oct 09, 2015 at 08:17:16AM -0500, Rob Herring wrote: > >> >> On Thu, Oct 8, 2015 at 11:36 PM, Leo Yan <leo.yan@linaro.org> wrote: > >> >> > On Hi6220, below memory regions in DDR have specific purpose: > >> >> > > >> >> > 0x05e0,0000 - 0x05ef,ffff: For MCU firmware using at runtime; > >> >> > 0x06df,f000 - 0x06df,ffff: For mailbox message data; > >> >> > 0x0740,f000 - 0x0740,ffff: For MCU firmware's section; > >> >> > 0x3e00,0000 - 0x3fff,ffff: For OP-TEE. > >> >> > > >> >> > This patch reserves these memory regions in DT. > >> >> > > >> >> > Signed-off-by: Leo Yan <leo.yan@linaro.org> > >> >> > --- > >> >> > arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 16 ++++++++++++---- > >> >> > 1 file changed, 12 insertions(+), 4 deletions(-) > >> >> > > >> >> > diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > >> >> > index e36a539..e3f4cb3 100644 > >> >> > --- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > >> >> > +++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > >> >> > @@ -7,9 +7,6 @@ > >> >> > > >> >> > /dts-v1/; > >> >> > > >> >> > -/*Reserved 1MB memory for MCU*/ > >> >> > -/memreserve/ 0x05e00000 0x00100000; > >> >> > - > >> >> > >> >> Why does memreserve not work for you? You can have multiple entries. > >> >> > >> >> > #include "hi6220.dtsi" > >> >> > > >> >> > / { > >> >> > @@ -24,8 +21,19 @@ > >> >> > stdout-path = "serial0:115200n8"; > >> >> > }; > >> >> > > >> >> > + /* > >> >> > + * Reserve below regions from memory node: > >> >> > + * > >> >> > + * - 0x05e0,0000 - 0x05ef,ffff: MCU firmware runtime using > >> >> > + * - 0x06df,f000 - 0x06df,ffff: Mailbox message data > >> >> > + * - 0x0740,f000 - 0x0740,ffff: MCU firmware section > >> >> > + * - 0x3e00,0000 - 0x3fff,ffff: OP-TEE > >> >> > + */ > >> >> > memory@0 { > >> >> > device_type = "memory"; > >> >> > - reg = <0x0 0x0 0x0 0x40000000>; > >> >> > + reg = <0x00000000 0x00000000 0x00000000 0x05e00000>, > >> >> > + <0x00000000 0x05f00000 0x00000000 0x00eff000>, > >> >> > + <0x00000000 0x06e00000 0x00000000 0x0060f000>, > >> >> > + <0x00000000 0x07410000 0x00000000 0x36bf0000>; > >> >> > >> >> No, don't do this. Please use memreserve or reserved-memory binding[1] > >> >> or combination of both. Probably reserved-memory if you need the > >> >> kernel to access some of these regions. > >> > > >> > I disagree at least for those portions owned by the secure world. The > >> > kernel shouldn't map those at all, so memreserve isn't appropriate. That > >> > covers OP-TEE and the MCU firmware regions, and I'd expec the EFI memory > >> > map to not list those as available to the kernel. > >> > >> I'm fine carving out the beginning or end, but otherwise think memory > >> should correspond to the physical memory. We have a way to describe > >> holes to keep out, so we should use them. If secure world uses the DT, > >> then it would either want to know its region in memory or add the DT > >> data to say what it is using. We need that to be easy to find or easy > >> to set, respectively. The size secure world needs could vary as well. > >> > >> The fact that the kernel maps the memory is the kernel's problem, not > >> a DT problem. > >> > > > > Just give more input here. In previous time, we have long discussion [1]; > > So actually your suggestion is exactly same what my old patch. > > > > From previous discussion, i think here have an assumtion: Use UEFI as > > bootloader, the kernel will ignore (or remove) memreserve and reserved-memory > > nodes, so just like Mark said "the EFI memory map to not list those > > as available to the kernel". My new patch is just to follow this and > > also make sure they have same behavior for different bootloader > > (between UEFI and uboot). > > I've read thru the thread and see 2 main conclusions. Using > reserved-memory is problematic since things like grub don't support > that. That is fine and we should stick with /mem-reserve/ for now. Thanks for reviewing, Rob. One thing should note: after booting with UEFI, /memreserve/ nodes will be deleted by UEFI stub; and _ONLY_ can use /reserved-memory/ node to reserve memory regions. Ard have another patch [1], after applied this patch, then all /memreserve/ nodes and /reserved-memory/ nodes nodes will be ignored to scan after booting with UEFI stub. This is make sense, that means UEFI need provide exactly correct memory map info by self and totally not depend on DT structures. Another minor difference between /memreserve/ node and /reserved-memory/ node is: we can add property "no-map" for /reserved-memory/; so that means it will totally remove region from memory block. it's more safe for the memory region will NOT be mapped twice with different mapping attribution. [1] http://archive.arm.linux.org.uk/lurker/message/20150922.002128.46757034.en.html > The other thing is the desire to have the memory presented to the kernel > be the same whether it comes from UEFI or DT structures. I can see why > there is some desire to have that alignment, but that doesn't really > buy us anything. We can't eliminate some code path in the kernel doing > so. So I still think that the memory node should reflect all of memory > as defined by the h/w and mem-reserve should be used for any software > defined reserved regions. i think before we engaged much thinking for UEFI, that's meaningful for we found what's correct implementation for UEFI. We need make sure UEFI will do correct thing for itself. If only consider purly from DT's usage, i have no strong opinion to stick to use memory node to carve memory regions out. It's okay for me to go back to use /reserved-memory/ to reserved regions. Mark, do you agree with this? Thanks, Leo Yan
On Thu, Oct 29, 2015 at 04:33:01PM +0800, Leo Yan wrote: > On Wed, Oct 28, 2015 at 11:32:29PM -0500, Rob Herring wrote: > > On Fri, Oct 9, 2015 at 9:20 AM, Leo Yan <leo.yan@linaro.org> wrote: > > > On Fri, Oct 09, 2015 at 08:50:13AM -0500, Rob Herring wrote: > > >> On Fri, Oct 9, 2015 at 8:30 AM, Mark Rutland <mark.rutland@arm.com> wrote: > > >> > On Fri, Oct 09, 2015 at 08:17:16AM -0500, Rob Herring wrote: > > >> >> On Thu, Oct 8, 2015 at 11:36 PM, Leo Yan <leo.yan@linaro.org> wrote: > > >> >> > On Hi6220, below memory regions in DDR have specific purpose: > > >> >> > > > >> >> > 0x05e0,0000 - 0x05ef,ffff: For MCU firmware using at runtime; > > >> >> > 0x06df,f000 - 0x06df,ffff: For mailbox message data; > > >> >> > 0x0740,f000 - 0x0740,ffff: For MCU firmware's section; > > >> >> > 0x3e00,0000 - 0x3fff,ffff: For OP-TEE. > > >> >> > > > >> >> > This patch reserves these memory regions in DT. > > >> >> > > > >> >> > Signed-off-by: Leo Yan <leo.yan@linaro.org> > > >> >> > --- > > >> >> > arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 16 ++++++++++++---- > > >> >> > 1 file changed, 12 insertions(+), 4 deletions(-) > > >> >> > > > >> >> > diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > > >> >> > index e36a539..e3f4cb3 100644 > > >> >> > --- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > > >> >> > +++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > > >> >> > @@ -7,9 +7,6 @@ > > >> >> > > > >> >> > /dts-v1/; > > >> >> > > > >> >> > -/*Reserved 1MB memory for MCU*/ > > >> >> > -/memreserve/ 0x05e00000 0x00100000; > > >> >> > - > > >> >> > > >> >> Why does memreserve not work for you? You can have multiple entries. > > >> >> > > >> >> > #include "hi6220.dtsi" > > >> >> > > > >> >> > / { > > >> >> > @@ -24,8 +21,19 @@ > > >> >> > stdout-path = "serial0:115200n8"; > > >> >> > }; > > >> >> > > > >> >> > + /* > > >> >> > + * Reserve below regions from memory node: > > >> >> > + * > > >> >> > + * - 0x05e0,0000 - 0x05ef,ffff: MCU firmware runtime using > > >> >> > + * - 0x06df,f000 - 0x06df,ffff: Mailbox message data > > >> >> > + * - 0x0740,f000 - 0x0740,ffff: MCU firmware section > > >> >> > + * - 0x3e00,0000 - 0x3fff,ffff: OP-TEE > > >> >> > + */ > > >> >> > memory@0 { > > >> >> > device_type = "memory"; > > >> >> > - reg = <0x0 0x0 0x0 0x40000000>; > > >> >> > + reg = <0x00000000 0x00000000 0x00000000 0x05e00000>, > > >> >> > + <0x00000000 0x05f00000 0x00000000 0x00eff000>, > > >> >> > + <0x00000000 0x06e00000 0x00000000 0x0060f000>, > > >> >> > + <0x00000000 0x07410000 0x00000000 0x36bf0000>; > > >> >> > > >> >> No, don't do this. Please use memreserve or reserved-memory binding[1] > > >> >> or combination of both. Probably reserved-memory if you need the > > >> >> kernel to access some of these regions. > > >> > > > >> > I disagree at least for those portions owned by the secure world. The > > >> > kernel shouldn't map those at all, so memreserve isn't appropriate. That > > >> > covers OP-TEE and the MCU firmware regions, and I'd expec the EFI memory > > >> > map to not list those as available to the kernel. > > >> > > >> I'm fine carving out the beginning or end, but otherwise think memory > > >> should correspond to the physical memory. We have a way to describe > > >> holes to keep out, so we should use them. If secure world uses the DT, > > >> then it would either want to know its region in memory or add the DT > > >> data to say what it is using. We need that to be easy to find or easy > > >> to set, respectively. The size secure world needs could vary as well. > > >> > > >> The fact that the kernel maps the memory is the kernel's problem, not > > >> a DT problem. > > >> > > > > > > Just give more input here. In previous time, we have long discussion [1]; > > > So actually your suggestion is exactly same what my old patch. > > > > > > From previous discussion, i think here have an assumtion: Use UEFI as > > > bootloader, the kernel will ignore (or remove) memreserve and reserved-memory > > > nodes, so just like Mark said "the EFI memory map to not list those > > > as available to the kernel". My new patch is just to follow this and > > > also make sure they have same behavior for different bootloader > > > (between UEFI and uboot). > > > > I've read thru the thread and see 2 main conclusions. Using > > reserved-memory is problematic since things like grub don't support > > that. That is fine and we should stick with /mem-reserve/ for now. > > Thanks for reviewing, Rob. > > One thing should note: after booting with UEFI, /memreserve/ nodes > will be deleted by UEFI stub; and _ONLY_ can use /reserved-memory/ > node to reserve memory regions. > > Ard have another patch [1], after applied this patch, then all > /memreserve/ nodes and /reserved-memory/ nodes nodes will be ignored > to scan after booting with UEFI stub. > > This is make sense, that means UEFI need provide exactly correct memory > map info by self and totally not depend on DT structures. > > Another minor difference between /memreserve/ node and /reserved-memory/ > node is: we can add property "no-map" for /reserved-memory/; so that > means it will totally remove region from memory block. it's more safe > for the memory region will NOT be mapped twice with different mapping > attribution. > > [1] http://archive.arm.linux.org.uk/lurker/message/20150922.002128.46757034.en.html > > > The other thing is the desire to have the memory presented to the kernel > > be the same whether it comes from UEFI or DT structures. I can see why > > there is some desire to have that alignment, but that doesn't really > > buy us anything. We can't eliminate some code path in the kernel doing > > so. So I still think that the memory node should reflect all of memory > > as defined by the h/w and mem-reserve should be used for any software > > defined reserved regions. > > i think before we engaged much thinking for UEFI, that's meaningful for > we found what's correct implementation for UEFI. We need make sure UEFI > will do correct thing for itself. > > If only consider purly from DT's usage, i have no strong opinion to > stick to use memory node to carve memory regions out. It's okay for me to > go back to use /reserved-memory/ to reserved regions. > > Mark, do you agree with this? Ping ... Hi Mark, Could u help confirm for this? i'm planning to resend new version patch series in tommorrow, but it's better can get your feedback firstly. Thanks, Leo Yan
On Thu, Nov 05, 2015 at 09:54:50PM +0800, Leo Yan wrote: > On Thu, Oct 29, 2015 at 04:33:01PM +0800, Leo Yan wrote: > > On Wed, Oct 28, 2015 at 11:32:29PM -0500, Rob Herring wrote: > > > On Fri, Oct 9, 2015 at 9:20 AM, Leo Yan <leo.yan@linaro.org> wrote: > > > > On Fri, Oct 09, 2015 at 08:50:13AM -0500, Rob Herring wrote: > > > >> On Fri, Oct 9, 2015 at 8:30 AM, Mark Rutland <mark.rutland@arm.com> wrote: > > > >> > On Fri, Oct 09, 2015 at 08:17:16AM -0500, Rob Herring wrote: > > > >> >> On Thu, Oct 8, 2015 at 11:36 PM, Leo Yan <leo.yan@linaro.org> wrote: > > > >> >> > On Hi6220, below memory regions in DDR have specific purpose: > > > >> >> > > > > >> >> > 0x05e0,0000 - 0x05ef,ffff: For MCU firmware using at runtime; > > > >> >> > 0x06df,f000 - 0x06df,ffff: For mailbox message data; > > > >> >> > 0x0740,f000 - 0x0740,ffff: For MCU firmware's section; > > > >> >> > 0x3e00,0000 - 0x3fff,ffff: For OP-TEE. > > > >> >> > > > > >> >> > This patch reserves these memory regions in DT. > > > >> >> > > > > >> >> > Signed-off-by: Leo Yan <leo.yan@linaro.org> > > > >> >> > --- > > > >> >> > arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 16 ++++++++++++---- > > > >> >> > 1 file changed, 12 insertions(+), 4 deletions(-) > > > >> >> > > > > >> >> > diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > > > >> >> > index e36a539..e3f4cb3 100644 > > > >> >> > --- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > > > >> >> > +++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > > > >> >> > @@ -7,9 +7,6 @@ > > > >> >> > > > > >> >> > /dts-v1/; > > > >> >> > > > > >> >> > -/*Reserved 1MB memory for MCU*/ > > > >> >> > -/memreserve/ 0x05e00000 0x00100000; > > > >> >> > - > > > >> >> > > > >> >> Why does memreserve not work for you? You can have multiple entries. > > > >> >> > > > >> >> > #include "hi6220.dtsi" > > > >> >> > > > > >> >> > / { > > > >> >> > @@ -24,8 +21,19 @@ > > > >> >> > stdout-path = "serial0:115200n8"; > > > >> >> > }; > > > >> >> > > > > >> >> > + /* > > > >> >> > + * Reserve below regions from memory node: > > > >> >> > + * > > > >> >> > + * - 0x05e0,0000 - 0x05ef,ffff: MCU firmware runtime using > > > >> >> > + * - 0x06df,f000 - 0x06df,ffff: Mailbox message data > > > >> >> > + * - 0x0740,f000 - 0x0740,ffff: MCU firmware section > > > >> >> > + * - 0x3e00,0000 - 0x3fff,ffff: OP-TEE > > > >> >> > + */ > > > >> >> > memory@0 { > > > >> >> > device_type = "memory"; > > > >> >> > - reg = <0x0 0x0 0x0 0x40000000>; > > > >> >> > + reg = <0x00000000 0x00000000 0x00000000 0x05e00000>, > > > >> >> > + <0x00000000 0x05f00000 0x00000000 0x00eff000>, > > > >> >> > + <0x00000000 0x06e00000 0x00000000 0x0060f000>, > > > >> >> > + <0x00000000 0x07410000 0x00000000 0x36bf0000>; > > > >> >> > > > >> >> No, don't do this. Please use memreserve or reserved-memory binding[1] > > > >> >> or combination of both. Probably reserved-memory if you need the > > > >> >> kernel to access some of these regions. > > > >> > > > > >> > I disagree at least for those portions owned by the secure world. The > > > >> > kernel shouldn't map those at all, so memreserve isn't appropriate. That > > > >> > covers OP-TEE and the MCU firmware regions, and I'd expec the EFI memory > > > >> > map to not list those as available to the kernel. > > > >> > > > >> I'm fine carving out the beginning or end, but otherwise think memory > > > >> should correspond to the physical memory. We have a way to describe > > > >> holes to keep out, so we should use them. If secure world uses the DT, > > > >> then it would either want to know its region in memory or add the DT > > > >> data to say what it is using. We need that to be easy to find or easy > > > >> to set, respectively. The size secure world needs could vary as well. > > > >> > > > >> The fact that the kernel maps the memory is the kernel's problem, not > > > >> a DT problem. > > > >> > > > > > > > > Just give more input here. In previous time, we have long discussion [1]; > > > > So actually your suggestion is exactly same what my old patch. > > > > > > > > From previous discussion, i think here have an assumtion: Use UEFI as > > > > bootloader, the kernel will ignore (or remove) memreserve and reserved-memory > > > > nodes, so just like Mark said "the EFI memory map to not list those > > > > as available to the kernel". My new patch is just to follow this and > > > > also make sure they have same behavior for different bootloader > > > > (between UEFI and uboot). > > > > > > I've read thru the thread and see 2 main conclusions. Using > > > reserved-memory is problematic since things like grub don't support > > > that. That is fine and we should stick with /mem-reserve/ for now. > > > > Thanks for reviewing, Rob. > > > > One thing should note: after booting with UEFI, /memreserve/ nodes > > will be deleted by UEFI stub; and _ONLY_ can use /reserved-memory/ > > node to reserve memory regions. > > > > Ard have another patch [1], after applied this patch, then all > > /memreserve/ nodes and /reserved-memory/ nodes nodes will be ignored > > to scan after booting with UEFI stub. > > > > This is make sense, that means UEFI need provide exactly correct memory > > map info by self and totally not depend on DT structures. > > > > Another minor difference between /memreserve/ node and /reserved-memory/ > > node is: we can add property "no-map" for /reserved-memory/; so that > > means it will totally remove region from memory block. it's more safe > > for the memory region will NOT be mapped twice with different mapping > > attribution. > > > > [1] http://archive.arm.linux.org.uk/lurker/message/20150922.002128.46757034.en.html > > > > > The other thing is the desire to have the memory presented to the kernel > > > be the same whether it comes from UEFI or DT structures. I can see why > > > there is some desire to have that alignment, but that doesn't really > > > buy us anything. We can't eliminate some code path in the kernel doing > > > so. So I still think that the memory node should reflect all of memory > > > as defined by the h/w and mem-reserve should be used for any software > > > defined reserved regions. > > > > i think before we engaged much thinking for UEFI, that's meaningful for > > we found what's correct implementation for UEFI. We need make sure UEFI > > will do correct thing for itself. > > > > If only consider purly from DT's usage, i have no strong opinion to > > stick to use memory node to carve memory regions out. It's okay for me to > > go back to use /reserved-memory/ to reserved regions. > > > > Mark, do you agree with this? > > Ping ... > > Hi Mark, > > Could u help confirm for this? i'm planning to resend new version > patch series in tommorrow, but it's better can get your feedback > firstly. Sorry for the delay. I'm still of the opinion that given the kernel has no business even reading this memory, it does not make sense to use a memreserve. Given that, and the points about other software not knowing aobut /reserved-memory/, I don't think it makes sense to describe the region in the memory nodes. Thanks, Mark.
On Thu, Nov 05, 2015 at 04:13:15PM +0000, Mark Rutland wrote: > On Thu, Nov 05, 2015 at 09:54:50PM +0800, Leo Yan wrote: > > On Thu, Oct 29, 2015 at 04:33:01PM +0800, Leo Yan wrote: > > > On Wed, Oct 28, 2015 at 11:32:29PM -0500, Rob Herring wrote: > > > > On Fri, Oct 9, 2015 at 9:20 AM, Leo Yan <leo.yan@linaro.org> wrote: > > > > > On Fri, Oct 09, 2015 at 08:50:13AM -0500, Rob Herring wrote: > > > > >> On Fri, Oct 9, 2015 at 8:30 AM, Mark Rutland <mark.rutland@arm.com> wrote: > > > > >> > On Fri, Oct 09, 2015 at 08:17:16AM -0500, Rob Herring wrote: > > > > >> >> On Thu, Oct 8, 2015 at 11:36 PM, Leo Yan <leo.yan@linaro.org> wrote: > > > > >> >> > On Hi6220, below memory regions in DDR have specific purpose: > > > > >> >> > > > > > >> >> > 0x05e0,0000 - 0x05ef,ffff: For MCU firmware using at runtime; > > > > >> >> > 0x06df,f000 - 0x06df,ffff: For mailbox message data; > > > > >> >> > 0x0740,f000 - 0x0740,ffff: For MCU firmware's section; > > > > >> >> > 0x3e00,0000 - 0x3fff,ffff: For OP-TEE. > > > > >> >> > > > > > >> >> > This patch reserves these memory regions in DT. > > > > >> >> > > > > > >> >> > Signed-off-by: Leo Yan <leo.yan@linaro.org> > > > > >> >> > --- > > > > >> >> > arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 16 ++++++++++++---- > > > > >> >> > 1 file changed, 12 insertions(+), 4 deletions(-) > > > > >> >> > > > > > >> >> > diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > > > > >> >> > index e36a539..e3f4cb3 100644 > > > > >> >> > --- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > > > > >> >> > +++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > > > > >> >> > @@ -7,9 +7,6 @@ > > > > >> >> > > > > > >> >> > /dts-v1/; > > > > >> >> > > > > > >> >> > -/*Reserved 1MB memory for MCU*/ > > > > >> >> > -/memreserve/ 0x05e00000 0x00100000; > > > > >> >> > - > > > > >> >> > > > > >> >> Why does memreserve not work for you? You can have multiple entries. > > > > >> >> > > > > >> >> > #include "hi6220.dtsi" > > > > >> >> > > > > > >> >> > / { > > > > >> >> > @@ -24,8 +21,19 @@ > > > > >> >> > stdout-path = "serial0:115200n8"; > > > > >> >> > }; > > > > >> >> > > > > > >> >> > + /* > > > > >> >> > + * Reserve below regions from memory node: > > > > >> >> > + * > > > > >> >> > + * - 0x05e0,0000 - 0x05ef,ffff: MCU firmware runtime using > > > > >> >> > + * - 0x06df,f000 - 0x06df,ffff: Mailbox message data > > > > >> >> > + * - 0x0740,f000 - 0x0740,ffff: MCU firmware section > > > > >> >> > + * - 0x3e00,0000 - 0x3fff,ffff: OP-TEE > > > > >> >> > + */ > > > > >> >> > memory@0 { > > > > >> >> > device_type = "memory"; > > > > >> >> > - reg = <0x0 0x0 0x0 0x40000000>; > > > > >> >> > + reg = <0x00000000 0x00000000 0x00000000 0x05e00000>, > > > > >> >> > + <0x00000000 0x05f00000 0x00000000 0x00eff000>, > > > > >> >> > + <0x00000000 0x06e00000 0x00000000 0x0060f000>, > > > > >> >> > + <0x00000000 0x07410000 0x00000000 0x36bf0000>; > > > > >> >> > > > > >> >> No, don't do this. Please use memreserve or reserved-memory binding[1] > > > > >> >> or combination of both. Probably reserved-memory if you need the > > > > >> >> kernel to access some of these regions. > > > > >> > > > > > >> > I disagree at least for those portions owned by the secure world. The > > > > >> > kernel shouldn't map those at all, so memreserve isn't appropriate. That > > > > >> > covers OP-TEE and the MCU firmware regions, and I'd expec the EFI memory > > > > >> > map to not list those as available to the kernel. > > > > >> > > > > >> I'm fine carving out the beginning or end, but otherwise think memory > > > > >> should correspond to the physical memory. We have a way to describe > > > > >> holes to keep out, so we should use them. If secure world uses the DT, > > > > >> then it would either want to know its region in memory or add the DT > > > > >> data to say what it is using. We need that to be easy to find or easy > > > > >> to set, respectively. The size secure world needs could vary as well. > > > > >> > > > > >> The fact that the kernel maps the memory is the kernel's problem, not > > > > >> a DT problem. > > > > >> > > > > > > > > > > Just give more input here. In previous time, we have long discussion [1]; > > > > > So actually your suggestion is exactly same what my old patch. > > > > > > > > > > From previous discussion, i think here have an assumtion: Use UEFI as > > > > > bootloader, the kernel will ignore (or remove) memreserve and reserved-memory > > > > > nodes, so just like Mark said "the EFI memory map to not list those > > > > > as available to the kernel". My new patch is just to follow this and > > > > > also make sure they have same behavior for different bootloader > > > > > (between UEFI and uboot). > > > > > > > > I've read thru the thread and see 2 main conclusions. Using > > > > reserved-memory is problematic since things like grub don't support > > > > that. That is fine and we should stick with /mem-reserve/ for now. > > > > > > Thanks for reviewing, Rob. > > > > > > One thing should note: after booting with UEFI, /memreserve/ nodes > > > will be deleted by UEFI stub; and _ONLY_ can use /reserved-memory/ > > > node to reserve memory regions. > > > > > > Ard have another patch [1], after applied this patch, then all > > > /memreserve/ nodes and /reserved-memory/ nodes nodes will be ignored > > > to scan after booting with UEFI stub. > > > > > > This is make sense, that means UEFI need provide exactly correct memory > > > map info by self and totally not depend on DT structures. > > > > > > Another minor difference between /memreserve/ node and /reserved-memory/ > > > node is: we can add property "no-map" for /reserved-memory/; so that > > > means it will totally remove region from memory block. it's more safe > > > for the memory region will NOT be mapped twice with different mapping > > > attribution. > > > > > > [1] http://archive.arm.linux.org.uk/lurker/message/20150922.002128.46757034.en.html > > > > > > > The other thing is the desire to have the memory presented to the kernel > > > > be the same whether it comes from UEFI or DT structures. I can see why > > > > there is some desire to have that alignment, but that doesn't really > > > > buy us anything. We can't eliminate some code path in the kernel doing > > > > so. So I still think that the memory node should reflect all of memory > > > > as defined by the h/w and mem-reserve should be used for any software > > > > defined reserved regions. > > > > > > i think before we engaged much thinking for UEFI, that's meaningful for > > > we found what's correct implementation for UEFI. We need make sure UEFI > > > will do correct thing for itself. > > > > > > If only consider purly from DT's usage, i have no strong opinion to > > > stick to use memory node to carve memory regions out. It's okay for me to > > > go back to use /reserved-memory/ to reserved regions. > > > > > > Mark, do you agree with this? > > > > Ping ... > > > > Hi Mark, > > > > Could u help confirm for this? i'm planning to resend new version > > patch series in tommorrow, but it's better can get your feedback > > firstly. > > Sorry for the delay. > > I'm still of the opinion that given the kernel has no business even > reading this memory, it does not make sense to use a memreserve. > > Given that, and the points about other software not knowing aobut > /reserved-memory/, I don't think it makes sense to describe the region > in the memory nodes. Here "other software" means other OS but not Linux, right? i rememebered before you meantioned /reserved-memory/ is dedicated for Linux kernel and i checked ePAPR has not defined this property. If we cannot use /reserved-memory/ currently just because other OS don't know it, then we should firstly commit one patch to totally remove it from Linux kernel; Haojian also brought up this question at very early time. Basically i'm reluctant to engage UEFI anymore for this discussion :), but if you are meaning "other software" as UEFI, IMOH, this is also not make sense to cannot use /reserved-memory/. Because after last time's discussion, Haojian has implemented UEFI memory map and we already verified it on Hikey. UEFI will create memory map for itself and it's no matter with DT's memory node and have no dependancy with /reserved-memory/. But suppose /reserved-memory/ will kept in Linux kernel and and now i'm just committing patches for Linux kernel. So i'd like to summary some rules for memory reservasion from my own understanding (also have posted on IRC): - Memory node is just to describe physical memory layout; Before there have many discussion for DT binding should describe the hardware, in a fashion which is completely OS-agnostic[1]. i think it also can apply to memory node, we just use memory node to describe physical memory layout, but not describe software's usage. - /memreserve/ is used to allow kernel to map linear virtual kernel address, usually it will not be used by driver; otherwise kernel will map the memory region twice with different memory attribution; - /reserved-memory/ is used to the memory regions, which will be remove from memory block and can be mapped by driver with ioremap. This will be easily for us follow up these rules and use it. [1] https://lwn.net/Articles/560523/ Thanks, Leo Yan
On Fri, Nov 06, 2015 at 09:19:37AM +0800, Leo Yan wrote: > On Thu, Nov 05, 2015 at 04:13:15PM +0000, Mark Rutland wrote: > > On Thu, Nov 05, 2015 at 09:54:50PM +0800, Leo Yan wrote: > > > On Thu, Oct 29, 2015 at 04:33:01PM +0800, Leo Yan wrote: > > > > On Wed, Oct 28, 2015 at 11:32:29PM -0500, Rob Herring wrote: > > > > > On Fri, Oct 9, 2015 at 9:20 AM, Leo Yan <leo.yan@linaro.org> wrote: > > > > > > On Fri, Oct 09, 2015 at 08:50:13AM -0500, Rob Herring wrote: > > > > > >> On Fri, Oct 9, 2015 at 8:30 AM, Mark Rutland <mark.rutland@arm.com> wrote: > > > > > >> > On Fri, Oct 09, 2015 at 08:17:16AM -0500, Rob Herring wrote: > > > > > >> >> On Thu, Oct 8, 2015 at 11:36 PM, Leo Yan <leo.yan@linaro.org> wrote: > > > > > >> >> > On Hi6220, below memory regions in DDR have specific purpose: > > > > > >> >> > > > > > > >> >> > 0x05e0,0000 - 0x05ef,ffff: For MCU firmware using at runtime; > > > > > >> >> > 0x06df,f000 - 0x06df,ffff: For mailbox message data; > > > > > >> >> > 0x0740,f000 - 0x0740,ffff: For MCU firmware's section; > > > > > >> >> > 0x3e00,0000 - 0x3fff,ffff: For OP-TEE. > > > > > >> >> > > > > > > >> >> > This patch reserves these memory regions in DT. > > > > > >> >> > > > > > > >> >> > Signed-off-by: Leo Yan <leo.yan@linaro.org> > > > > > >> >> > --- > > > > > >> >> > arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 16 ++++++++++++---- > > > > > >> >> > 1 file changed, 12 insertions(+), 4 deletions(-) > > > > > >> >> > > > > > > >> >> > diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > > > > > >> >> > index e36a539..e3f4cb3 100644 > > > > > >> >> > --- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > > > > > >> >> > +++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > > > > > >> >> > @@ -7,9 +7,6 @@ > > > > > >> >> > > > > > > >> >> > /dts-v1/; > > > > > >> >> > > > > > > >> >> > -/*Reserved 1MB memory for MCU*/ > > > > > >> >> > -/memreserve/ 0x05e00000 0x00100000; > > > > > >> >> > - > > > > > >> >> > > > > > >> >> Why does memreserve not work for you? You can have multiple entries. > > > > > >> >> > > > > > >> >> > #include "hi6220.dtsi" > > > > > >> >> > > > > > > >> >> > / { > > > > > >> >> > @@ -24,8 +21,19 @@ > > > > > >> >> > stdout-path = "serial0:115200n8"; > > > > > >> >> > }; > > > > > >> >> > > > > > > >> >> > + /* > > > > > >> >> > + * Reserve below regions from memory node: > > > > > >> >> > + * > > > > > >> >> > + * - 0x05e0,0000 - 0x05ef,ffff: MCU firmware runtime using > > > > > >> >> > + * - 0x06df,f000 - 0x06df,ffff: Mailbox message data > > > > > >> >> > + * - 0x0740,f000 - 0x0740,ffff: MCU firmware section > > > > > >> >> > + * - 0x3e00,0000 - 0x3fff,ffff: OP-TEE > > > > > >> >> > + */ > > > > > >> >> > memory@0 { > > > > > >> >> > device_type = "memory"; > > > > > >> >> > - reg = <0x0 0x0 0x0 0x40000000>; > > > > > >> >> > + reg = <0x00000000 0x00000000 0x00000000 0x05e00000>, > > > > > >> >> > + <0x00000000 0x05f00000 0x00000000 0x00eff000>, > > > > > >> >> > + <0x00000000 0x06e00000 0x00000000 0x0060f000>, > > > > > >> >> > + <0x00000000 0x07410000 0x00000000 0x36bf0000>; > > > > > >> >> [...] > > > > > I've read thru the thread and see 2 main conclusions. Using > > > > > reserved-memory is problematic since things like grub don't support > > > > > that. That is fine and we should stick with /mem-reserve/ for now. > > > > > > > > > > The other thing is the desire to have the memory presented to the kernel > > > > > be the same whether it comes from UEFI or DT structures. I can see why > > > > > there is some desire to have that alignment, but that doesn't really > > > > > buy us anything. We can't eliminate some code path in the kernel doing > > > > > so. So I still think that the memory node should reflect all of memory > > > > > as defined by the h/w and mem-reserve should be used for any software > > > > > defined reserved regions. [...] > > Sorry for the delay. > > > > I'm still of the opinion that given the kernel has no business even > > reading this memory, it does not make sense to use a memreserve. > > > > Given that, and the points about other software not knowing aobut > > /reserved-memory/, I don't think it makes sense to describe the region > > in the memory nodes. [...] Hi Rob, Mark, Sorry its taken me so long to bump this thread. So I can have more confidence discussing this I have reviewed the existing files in arch/arm/boot and arch/arm64/boot and done a bit of analysis. My summary is below: In arch/arm/boot: - In arch/arm, Using /reserved-memory/ and /memreserve/ are very common case to reserve memory regions; - /memreserve/ is mainly used to reserve memory for spin-table which used by SMP booting; - /reserved-memory/ are used to reserve memory regions for security (TrustZone, secure RAM, etc), message transferring; So we can say usually use this method to reserve memory for device driver or other modules which maps these memory regions later. - There also have some DT files using memory node to carve out memory regions, but from roughly analysis we can see the most cases are introduced by memory controller for discontinuous memory space for DDR; or there have some silicon bug so some regions are not stable so need get rid of them (Armada-xp); In arch/arm64/boot: - There have no any file using /reserved-memory/; - Several files are using /memreserve/ for spin-table boot method; - ARM platforms (Juno, FVP) use memory nodes to carve out memory regions for security; - DT files in arch/arm64 are quite consistent with Mark's suggestion. In my review I haven't really found anything that pushes me in one direction or the other so, without consensus from the DT maintainers, I really don't know how to progress here. Thanks, Leo Yan
On Mon, Jan 11, 2016 at 9:40 AM, Leo Yan <leo.yan@linaro.org> wrote: > On Fri, Nov 06, 2015 at 09:19:37AM +0800, Leo Yan wrote: >> On Thu, Nov 05, 2015 at 04:13:15PM +0000, Mark Rutland wrote: >> > On Thu, Nov 05, 2015 at 09:54:50PM +0800, Leo Yan wrote: >> > > On Thu, Oct 29, 2015 at 04:33:01PM +0800, Leo Yan wrote: >> > > > On Wed, Oct 28, 2015 at 11:32:29PM -0500, Rob Herring wrote: >> > > > > On Fri, Oct 9, 2015 at 9:20 AM, Leo Yan <leo.yan@linaro.org> wrote: >> > > > > > On Fri, Oct 09, 2015 at 08:50:13AM -0500, Rob Herring wrote: >> > > > > >> On Fri, Oct 9, 2015 at 8:30 AM, Mark Rutland <mark.rutland@arm.com> wrote: >> > > > > >> > On Fri, Oct 09, 2015 at 08:17:16AM -0500, Rob Herring wrote: >> > > > > >> >> On Thu, Oct 8, 2015 at 11:36 PM, Leo Yan <leo.yan@linaro.org> wrote: >> > > > > >> >> > On Hi6220, below memory regions in DDR have specific purpose: >> > > > > >> >> > >> > > > > >> >> > 0x05e0,0000 - 0x05ef,ffff: For MCU firmware using at runtime; >> > > > > >> >> > 0x06df,f000 - 0x06df,ffff: For mailbox message data; >> > > > > >> >> > 0x0740,f000 - 0x0740,ffff: For MCU firmware's section; >> > > > > >> >> > 0x3e00,0000 - 0x3fff,ffff: For OP-TEE. >> > > > > >> >> > >> > > > > >> >> > This patch reserves these memory regions in DT. >> > > > > >> >> > >> > > > > >> >> > Signed-off-by: Leo Yan <leo.yan@linaro.org> >> > > > > >> >> > --- >> > > > > >> >> > arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 16 ++++++++++++---- >> > > > > >> >> > 1 file changed, 12 insertions(+), 4 deletions(-) >> > > > > >> >> > >> > > > > >> >> > diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts >> > > > > >> >> > index e36a539..e3f4cb3 100644 >> > > > > >> >> > --- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts >> > > > > >> >> > +++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts >> > > > > >> >> > @@ -7,9 +7,6 @@ >> > > > > >> >> > >> > > > > >> >> > /dts-v1/; >> > > > > >> >> > >> > > > > >> >> > -/*Reserved 1MB memory for MCU*/ >> > > > > >> >> > -/memreserve/ 0x05e00000 0x00100000; >> > > > > >> >> > - >> > > > > >> >> >> > > > > >> >> Why does memreserve not work for you? You can have multiple entries. >> > > > > >> >> >> > > > > >> >> > #include "hi6220.dtsi" >> > > > > >> >> > >> > > > > >> >> > / { >> > > > > >> >> > @@ -24,8 +21,19 @@ >> > > > > >> >> > stdout-path = "serial0:115200n8"; >> > > > > >> >> > }; >> > > > > >> >> > >> > > > > >> >> > + /* >> > > > > >> >> > + * Reserve below regions from memory node: >> > > > > >> >> > + * >> > > > > >> >> > + * - 0x05e0,0000 - 0x05ef,ffff: MCU firmware runtime using >> > > > > >> >> > + * - 0x06df,f000 - 0x06df,ffff: Mailbox message data >> > > > > >> >> > + * - 0x0740,f000 - 0x0740,ffff: MCU firmware section >> > > > > >> >> > + * - 0x3e00,0000 - 0x3fff,ffff: OP-TEE >> > > > > >> >> > + */ >> > > > > >> >> > memory@0 { >> > > > > >> >> > device_type = "memory"; >> > > > > >> >> > - reg = <0x0 0x0 0x0 0x40000000>; >> > > > > >> >> > + reg = <0x00000000 0x00000000 0x00000000 0x05e00000>, >> > > > > >> >> > + <0x00000000 0x05f00000 0x00000000 0x00eff000>, >> > > > > >> >> > + <0x00000000 0x06e00000 0x00000000 0x0060f000>, >> > > > > >> >> > + <0x00000000 0x07410000 0x00000000 0x36bf0000>; >> > > > > >> >> > > [...] > >> > > > > I've read thru the thread and see 2 main conclusions. Using >> > > > > reserved-memory is problematic since things like grub don't support >> > > > > that. That is fine and we should stick with /mem-reserve/ for now. >> > > > > >> > > > > The other thing is the desire to have the memory presented to the kernel >> > > > > be the same whether it comes from UEFI or DT structures. I can see why >> > > > > there is some desire to have that alignment, but that doesn't really >> > > > > buy us anything. We can't eliminate some code path in the kernel doing >> > > > > so. So I still think that the memory node should reflect all of memory >> > > > > as defined by the h/w and mem-reserve should be used for any software >> > > > > defined reserved regions. > > [...] > >> > Sorry for the delay. >> > >> > I'm still of the opinion that given the kernel has no business even >> > reading this memory, it does not make sense to use a memreserve. >> > >> > Given that, and the points about other software not knowing aobut >> > /reserved-memory/, I don't think it makes sense to describe the region >> > in the memory nodes. > > [...] > > Hi Rob, Mark, > > Sorry its taken me so long to bump this thread. > > So I can have more confidence discussing this I have reviewed the > existing files in arch/arm/boot and arch/arm64/boot and done a bit of > analysis. My summary is below: > > In arch/arm/boot: > > - In arch/arm, Using /reserved-memory/ and /memreserve/ are very common > case to reserve memory regions; > - /memreserve/ is mainly used to reserve memory for spin-table which > used by SMP booting; > - /reserved-memory/ are used to reserve memory regions for security > (TrustZone, secure RAM, etc), message transferring; So we can say > usually use this method to reserve memory for device driver or other > modules which maps these memory regions later. > > - There also have some DT files using memory node to carve out memory > regions, but from roughly analysis we can see the most cases are > introduced by memory controller for discontinuous memory space for > DDR; or there have some silicon bug so some regions are not stable > so need get rid of them (Armada-xp); > > In arch/arm64/boot: > > - There have no any file using /reserved-memory/; > - Several files are using /memreserve/ for spin-table boot method; > - ARM platforms (Juno, FVP) use memory nodes to carve out memory regions > for security; > - DT files in arch/arm64 are quite consistent with Mark's suggestion. > > In my review I haven't really found anything that pushes me in one > direction or the other so, without consensus from the DT maintainers, I > really don't know how to progress here. While I still think regions should be marked as reserved rather than carving up the memory node, it's not really worth holding up this platform further. The kernel can support either way. However, Mark's argument AIUI was regions that are never accessed should be done the way you have done it. I don't think the mailbox region falls in this category. I'm not sure about OP-TEE, but we should have consistency for it if the OP-TEE driver needs to find its memory region. Cc'ing Joakim for comment. Rob
On Mon, Jan 18, 2016 at 09:07:14AM -0600, Rob Herring wrote: > On Mon, Jan 11, 2016 at 9:40 AM, Leo Yan <leo.yan@linaro.org> wrote: > > On Fri, Nov 06, 2015 at 09:19:37AM +0800, Leo Yan wrote: > >> On Thu, Nov 05, 2015 at 04:13:15PM +0000, Mark Rutland wrote: > >> > On Thu, Nov 05, 2015 at 09:54:50PM +0800, Leo Yan wrote: > >> > > On Thu, Oct 29, 2015 at 04:33:01PM +0800, Leo Yan wrote: > >> > > > On Wed, Oct 28, 2015 at 11:32:29PM -0500, Rob Herring wrote: > >> > > > > On Fri, Oct 9, 2015 at 9:20 AM, Leo Yan <leo.yan@linaro.org> wrote: > >> > > > > > On Fri, Oct 09, 2015 at 08:50:13AM -0500, Rob Herring wrote: > >> > > > > >> On Fri, Oct 9, 2015 at 8:30 AM, Mark Rutland <mark.rutland@arm.com> wrote: > >> > > > > >> > On Fri, Oct 09, 2015 at 08:17:16AM -0500, Rob Herring wrote: > >> > > > > >> >> On Thu, Oct 8, 2015 at 11:36 PM, Leo Yan <leo.yan@linaro.org> wrote: > >> > > > > >> >> > On Hi6220, below memory regions in DDR have specific purpose: > >> > > > > >> >> > > >> > > > > >> >> > 0x05e0,0000 - 0x05ef,ffff: For MCU firmware using at runtime; > >> > > > > >> >> > 0x06df,f000 - 0x06df,ffff: For mailbox message data; > >> > > > > >> >> > 0x0740,f000 - 0x0740,ffff: For MCU firmware's section; > >> > > > > >> >> > 0x3e00,0000 - 0x3fff,ffff: For OP-TEE. > >> > > > > >> >> > > >> > > > > >> >> > This patch reserves these memory regions in DT. > >> > > > > >> >> > > >> > > > > >> >> > Signed-off-by: Leo Yan <leo.yan@linaro.org> > >> > > > > >> >> > --- > >> > > > > >> >> > arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 16 ++++++++++++---- > >> > > > > >> >> > 1 file changed, 12 insertions(+), 4 deletions(-) > >> > > > > >> >> > > >> > > > > >> >> > diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > >> > > > > >> >> > index e36a539..e3f4cb3 100644 > >> > > > > >> >> > --- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > >> > > > > >> >> > +++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > >> > > > > >> >> > @@ -7,9 +7,6 @@ > >> > > > > >> >> > > >> > > > > >> >> > /dts-v1/; > >> > > > > >> >> > > >> > > > > >> >> > -/*Reserved 1MB memory for MCU*/ > >> > > > > >> >> > -/memreserve/ 0x05e00000 0x00100000; > >> > > > > >> >> > - > >> > > > > >> >> > >> > > > > >> >> Why does memreserve not work for you? You can have multiple entries. > >> > > > > >> >> > >> > > > > >> >> > #include "hi6220.dtsi" > >> > > > > >> >> > > >> > > > > >> >> > / { > >> > > > > >> >> > @@ -24,8 +21,19 @@ > >> > > > > >> >> > stdout-path = "serial0:115200n8"; > >> > > > > >> >> > }; > >> > > > > >> >> > > >> > > > > >> >> > + /* > >> > > > > >> >> > + * Reserve below regions from memory node: > >> > > > > >> >> > + * > >> > > > > >> >> > + * - 0x05e0,0000 - 0x05ef,ffff: MCU firmware runtime using > >> > > > > >> >> > + * - 0x06df,f000 - 0x06df,ffff: Mailbox message data > >> > > > > >> >> > + * - 0x0740,f000 - 0x0740,ffff: MCU firmware section > >> > > > > >> >> > + * - 0x3e00,0000 - 0x3fff,ffff: OP-TEE > >> > > > > >> >> > + */ > >> > > > > >> >> > memory@0 { > >> > > > > >> >> > device_type = "memory"; > >> > > > > >> >> > - reg = <0x0 0x0 0x0 0x40000000>; > >> > > > > >> >> > + reg = <0x00000000 0x00000000 0x00000000 0x05e00000>, > >> > > > > >> >> > + <0x00000000 0x05f00000 0x00000000 0x00eff000>, > >> > > > > >> >> > + <0x00000000 0x06e00000 0x00000000 0x0060f000>, > >> > > > > >> >> > + <0x00000000 0x07410000 0x00000000 0x36bf0000>; > >> > > > > >> >> > > > > [...] > > > >> > > > > I've read thru the thread and see 2 main conclusions. Using > >> > > > > reserved-memory is problematic since things like grub don't support > >> > > > > that. That is fine and we should stick with /mem-reserve/ for now. > >> > > > > > >> > > > > The other thing is the desire to have the memory presented to the kernel > >> > > > > be the same whether it comes from UEFI or DT structures. I can see why > >> > > > > there is some desire to have that alignment, but that doesn't really > >> > > > > buy us anything. We can't eliminate some code path in the kernel doing > >> > > > > so. So I still think that the memory node should reflect all of memory > >> > > > > as defined by the h/w and mem-reserve should be used for any software > >> > > > > defined reserved regions. > > > > [...] > > > >> > Sorry for the delay. > >> > > >> > I'm still of the opinion that given the kernel has no business even > >> > reading this memory, it does not make sense to use a memreserve. > >> > > >> > Given that, and the points about other software not knowing aobut > >> > /reserved-memory/, I don't think it makes sense to describe the region > >> > in the memory nodes. > > > > [...] > > > > Hi Rob, Mark, > > > > Sorry its taken me so long to bump this thread. > > > > So I can have more confidence discussing this I have reviewed the > > existing files in arch/arm/boot and arch/arm64/boot and done a bit of > > analysis. My summary is below: > > > > In arch/arm/boot: > > > > - In arch/arm, Using /reserved-memory/ and /memreserve/ are very common > > case to reserve memory regions; > > - /memreserve/ is mainly used to reserve memory for spin-table which > > used by SMP booting; > > - /reserved-memory/ are used to reserve memory regions for security > > (TrustZone, secure RAM, etc), message transferring; So we can say > > usually use this method to reserve memory for device driver or other > > modules which maps these memory regions later. > > > > - There also have some DT files using memory node to carve out memory > > regions, but from roughly analysis we can see the most cases are > > introduced by memory controller for discontinuous memory space for > > DDR; or there have some silicon bug so some regions are not stable > > so need get rid of them (Armada-xp); > > > > In arch/arm64/boot: > > > > - There have no any file using /reserved-memory/; > > - Several files are using /memreserve/ for spin-table boot method; > > - ARM platforms (Juno, FVP) use memory nodes to carve out memory regions > > for security; > > - DT files in arch/arm64 are quite consistent with Mark's suggestion. > > > > In my review I haven't really found anything that pushes me in one > > direction or the other so, without consensus from the DT maintainers, I > > really don't know how to progress here. > > While I still think regions should be marked as reserved rather than > carving up the memory node, it's not really worth holding up this > platform further. The kernel can support either way. > > However, Mark's argument AIUI was regions that are never accessed > should be done the way you have done it. I don't think the mailbox > region falls in this category. I'm not sure about OP-TEE, but we > should have consistency for it if the OP-TEE driver needs to find its > memory region. Cc'ing Joakim for comment. My understanding was that the OP-TEE memory was that reserved for use by the secure side (rather than a shared region). To avoid (non-secure) speculative accesses triggering aborts from some trustzone controller, and other issues, no memory reserved for secure side use should be mapped by the non-secure side, and hence a memreserve is insufficient. Hopefully Joakim can shed light on that. There is a general problem with memreserve in that it is currently ill-defined for arm and arm64, and its implications are not well understood (by the nature of the ARM cache architecture, and problems like mismatched aliases being not well understood). Due to this, I think that its use should be avoided unless strictly necessary. Having an alias as part of the linear mapping creates a number of problems which are avoided most simply by never having that alias (i.e. not describing the region as memory at all). Establishing that pattern now saves us pain in future. One major reason for memreserve being used was to allow the linear mapping to be used to access objects. Practically every case has moved over to using a dynamic mapping (fixmap, or {io,mem}remap), as that's required to handle a number of edge cases. For data structures (e.g. the DTB) that are only expected to be accessed by the kernel, a memreserve is fine provided sufficient cache maintenance has been performed already. For everything else (e.g. anything shared with or owned by another agent), its use is questionable at best (and I think the spin-table story is an unfortunate mistake that we have to live with). Thanks, Mark.
Add Victor as well. On Mon, Jan 18, 2016 at 03:42:59PM +0000, Mark Rutland wrote: [...] > > While I still think regions should be marked as reserved rather than > > carving up the memory node, it's not really worth holding up this > > platform further. The kernel can support either way. > > > > However, Mark's argument AIUI was regions that are never accessed > > should be done the way you have done it. I don't think the mailbox > > region falls in this category. I'm not sure about OP-TEE, but we > > should have consistency for it if the OP-TEE driver needs to find its > > memory region. Cc'ing Joakim for comment. > > My understanding was that the OP-TEE memory was that reserved for use by > the secure side (rather than a shared region). To avoid (non-secure) > speculative accesses triggering aborts from some trustzone controller, > and other issues, no memory reserved for secure side use should be > mapped by the non-secure side, and hence a memreserve is insufficient. > Hopefully Joakim can shed light on that. I checked with Victor and OP-TEE memory region is reserved for below purpose: 3f00,0000 - 3fff,ffff: OP-TEE's execution region 3ef0,0000 - 3eff,ffff: OP-TEE's shared memory, so this region will be mapped both for secure world and normal world. 3e00,0000 - 3eef,ffff: Reserved for future using... Not sure if this is same with ARM's Juno/TC2 platform? will leave to Joakim for confirmation. Here just wander if can directly carve out whole region at one time? Or should distinguish the regions will be accessed by kernel later and use different way to reserve them? Especially readed below Mark brought cache alias issue, the most safe way is totally to avoid linear mapping. Thanks, Leo Yan > There is a general problem with memreserve in that it is currently > ill-defined for arm and arm64, and its implications are not well > understood (by the nature of the ARM cache architecture, and problems > like mismatched aliases being not well understood). Due to this, I think > that its use should be avoided unless strictly necessary. > > Having an alias as part of the linear mapping creates a number of > problems which are avoided most simply by never having that alias (i.e. > not describing the region as memory at all). Establishing that pattern > now saves us pain in future. > > One major reason for memreserve being used was to allow the linear > mapping to be used to access objects. Practically every case has moved > over to using a dynamic mapping (fixmap, or {io,mem}remap), as that's > required to handle a number of edge cases. > > For data structures (e.g. the DTB) that are only expected to be accessed > by the kernel, a memreserve is fine provided sufficient cache > maintenance has been performed already. For everything else (e.g. > anything shared with or owned by another agent), its use is questionable > at best (and I think the spin-table story is an unfortunate mistake that > we have to live with). > > Thanks, > Mark.
diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts index e36a539..e3f4cb3 100644 --- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts +++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts @@ -7,9 +7,6 @@ /dts-v1/; -/*Reserved 1MB memory for MCU*/ -/memreserve/ 0x05e00000 0x00100000; - #include "hi6220.dtsi" / { @@ -24,8 +21,19 @@ stdout-path = "serial0:115200n8"; }; + /* + * Reserve below regions from memory node: + * + * - 0x05e0,0000 - 0x05ef,ffff: MCU firmware runtime using + * - 0x06df,f000 - 0x06df,ffff: Mailbox message data + * - 0x0740,f000 - 0x0740,ffff: MCU firmware section + * - 0x3e00,0000 - 0x3fff,ffff: OP-TEE + */ memory@0 { device_type = "memory"; - reg = <0x0 0x0 0x0 0x40000000>; + reg = <0x00000000 0x00000000 0x00000000 0x05e00000>, + <0x00000000 0x05f00000 0x00000000 0x00eff000>, + <0x00000000 0x06e00000 0x00000000 0x0060f000>, + <0x00000000 0x07410000 0x00000000 0x36bf0000>; }; };
On Hi6220, below memory regions in DDR have specific purpose: 0x05e0,0000 - 0x05ef,ffff: For MCU firmware using at runtime; 0x06df,f000 - 0x06df,ffff: For mailbox message data; 0x0740,f000 - 0x0740,ffff: For MCU firmware's section; 0x3e00,0000 - 0x3fff,ffff: For OP-TEE. This patch reserves these memory regions in DT. Signed-off-by: Leo Yan <leo.yan@linaro.org> --- arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)