| Message ID | 1444365376-10728-2-git-send-email-leo.yan@linaro.org |
|---|---|
| State | Superseded |
| 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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(-)