Message ID | 1474648719-2910-1-git-send-email-sudeep.holla@arm.com |
---|---|
State | Superseded |
Headers | show |
On Fri, 2016-09-23 at 17:38 +0100, Sudeep Holla wrote: > Commit f225d39d3093 ("vexpress: Check TC2 firmware support before defaulting > to nonsec booting") added support to check if the firmware on TC2 is > configured appropriately before booting in nonsec/hyp mode. > > However when booting in non-secure/hyp mode, CCI control must be done in > secure firmware and can't be done in non-secure/hyp mode. In order to > ensure that, this patch disables the cci slave port inteface so that it > is not accessed at all. > > Cc: Jon Medhurst <tixy@linaro.org> > Acked-by: Marc Zyngier <marc.zyngier@arm.com> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > --- This works for me (unsurprisingly) when booting in secure mode. What kernel and firmware config do I need to use for non-sec mode? I tried vexpress_defconfig, with bits 12 and 13 cleared in SCC: 0x700 but I get nothing out the console after "Starting kernel ..." -- Tixy _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On 26/09/16 12:30, Jon Medhurst (Tixy) wrote: > On Fri, 2016-09-23 at 17:38 +0100, Sudeep Holla wrote: >> Commit f225d39d3093 ("vexpress: Check TC2 firmware support before defaulting >> to nonsec booting") added support to check if the firmware on TC2 is >> configured appropriately before booting in nonsec/hyp mode. >> >> However when booting in non-secure/hyp mode, CCI control must be done in >> secure firmware and can't be done in non-secure/hyp mode. In order to >> ensure that, this patch disables the cci slave port inteface so that it >> is not accessed at all. >> >> Cc: Jon Medhurst <tixy@linaro.org> >> Acked-by: Marc Zyngier <marc.zyngier@arm.com> >> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> >> --- > > This works for me (unsurprisingly) when booting in secure mode. What > kernel and firmware config do I need to use for non-sec mode? I tried > vexpress_defconfig, with bits 12 and 13 cleared in SCC: 0x700 > but I get nothing out the console after "Starting kernel ..." > I just flipped bit 12 and 13 in SCC: 0x700 to switch between MCPM/secure and HYP/non-secure mode. All other images/settings remain the same. So IIUC, with vexpress_defconfig + above SCC change you are seeing a hand, I will check that. I am using multi_v7_defconfig. -- Regards, Sudeep _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On 26/09/16 12:35, Sudeep Holla wrote: > > > On 26/09/16 12:30, Jon Medhurst (Tixy) wrote: >> On Fri, 2016-09-23 at 17:38 +0100, Sudeep Holla wrote: >>> Commit f225d39d3093 ("vexpress: Check TC2 firmware support before >>> defaulting >>> to nonsec booting") added support to check if the firmware on TC2 is >>> configured appropriately before booting in nonsec/hyp mode. >>> >>> However when booting in non-secure/hyp mode, CCI control must be done in >>> secure firmware and can't be done in non-secure/hyp mode. In order to >>> ensure that, this patch disables the cci slave port inteface so that it >>> is not accessed at all. >>> >>> Cc: Jon Medhurst <tixy@linaro.org> >>> Acked-by: Marc Zyngier <marc.zyngier@arm.com> >>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> >>> --- >> >> This works for me (unsurprisingly) when booting in secure mode. What >> kernel and firmware config do I need to use for non-sec mode? I tried >> vexpress_defconfig, with bits 12 and 13 cleared in SCC: 0x700 >> but I get nothing out the console after "Starting kernel ..." >> > > I just flipped bit 12 and 13 in SCC: 0x700 to switch between MCPM/secure > and HYP/non-secure mode. All other images/settings remain the same. > > So IIUC, with vexpress_defconfig + above SCC change you are seeing a > hand, I will check that. I am using multi_v7_defconfig. > + the patches from Lorenzo fixing MCPM code in the kernel [1] as mentioned first email carrying this patch. -- Regards, Sudeep [1] http://www.spinics.net/lists/arm-kernel/msg533715.html _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Fri, 2016-09-23 at 17:38 +0100, Sudeep Holla wrote: > Commit f225d39d3093 ("vexpress: Check TC2 firmware support before defaulting > to nonsec booting") added support to check if the firmware on TC2 is > configured appropriately before booting in nonsec/hyp mode. > > However when booting in non-secure/hyp mode, CCI control must be done in > secure firmware and can't be done in non-secure/hyp mode. In order to > ensure that, this patch disables the cci slave port inteface so that it > is not accessed at all. > > Cc: Jon Medhurst <tixy@linaro.org> > Acked-by: Marc Zyngier <marc.zyngier@arm.com> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > --- Acked-by: Jon Medhurst <tixy@linaro.org> Tested-by: Jon Medhurst <tixy@linaro.org> > board/armltd/vexpress/vexpress_tc2.c | 52 ++++++++++++++++++++++++++++++++++++ > configs/vexpress_ca15_tc2_defconfig | 1 + > 2 files changed, 53 insertions(+) > > Hi, > > This change is needed to avoid the kernel panic while attempting to access > CCI ports when booting in non-sec/HYP mode. The kernel patches to fix > this are available @[1] > > Regards, > Sudeep > > v1->v2: > - Fix compilation with !CONFIG_ARMV7_NONSEC(Thanks to Tixy) > > [1] http://www.spinics.net/lists/arm-kernel/msg533715.html > > diff --git a/board/armltd/vexpress/vexpress_tc2.c b/board/armltd/vexpress/vexpress_tc2.c > index ebb41a8833ab..c7adf950f579 100644 > --- a/board/armltd/vexpress/vexpress_tc2.c > +++ b/board/armltd/vexpress/vexpress_tc2.c > @@ -7,7 +7,11 @@ > * SPDX-License-Identifier: GPL-2.0+ > */ > > +#include <asm/armv7.h> > #include <asm/io.h> > +#include <asm/u-boot.h> > +#include <common.h> > +#include <libfdt.h> > > #define SCC_BASE 0x7fff0000 > > @@ -31,3 +35,51 @@ bool armv7_boot_nonsec_default(void) > return (readl((u32 *)(SCC_BASE + 0x700)) & ((1 << 12) | (1 << 13))) == 0; > #endif > } > + > +#ifdef CONFIG_OF_BOARD_SETUP > +int ft_board_setup(void *fdt, bd_t *bd) > +{ > + int offset, tmp, len; > + const struct fdt_property *prop; > + const char *cci_compatible = "arm,cci-400-ctrl-if"; > + > +#ifdef CONFIG_ARMV7_NONSEC > + if (!armv7_boot_nonsec()) > + return 0; > +#else > + return 0; > +#endif > + /* Booting in nonsec mode, disable CCI access */ > + offset = fdt_path_offset(fdt, "/cpus"); > + if (offset < 0) { > + printf("couldn't find /cpus\n"); > + return offset; > + } > + > + /* delete cci-control-port in each cpu node */ > + for (tmp = fdt_first_subnode(fdt, offset); tmp >= 0; > + tmp = fdt_next_subnode(fdt, tmp)) > + fdt_delprop(fdt, tmp, "cci-control-port"); > + > + /* disable all ace cci slave ports */ > + offset = fdt_node_offset_by_prop_value(fdt, offset, "compatible", > + cci_compatible, 20); > + while (offset > 0) { > + prop = fdt_get_property(fdt, offset, "interface-type", > + &len); > + if (!prop) > + continue; > + if (len < 4) > + continue; > + if (strcmp(prop->data, "ace")) > + continue; > + > + fdt_setprop_string(fdt, offset, "status", "disabled"); > + > + offset = fdt_node_offset_by_prop_value(fdt, offset, "compatible", > + cci_compatible, 20); > + } > + > + return 0; > +} > +#endif /* CONFIG_OF_BOARD_SETUP */ > diff --git a/configs/vexpress_ca15_tc2_defconfig b/configs/vexpress_ca15_tc2_defconfig > index 2f141dda06c6..5154803b7c65 100644 > --- a/configs/vexpress_ca15_tc2_defconfig > +++ b/configs/vexpress_ca15_tc2_defconfig > @@ -1,5 +1,6 @@ > CONFIG_ARM=y > CONFIG_TARGET_VEXPRESS_CA15_TC2=y > +CONFIG_OF_BOARD_SETUP=y > CONFIG_HUSH_PARSER=y > # CONFIG_CMD_CONSOLE is not set > # CONFIG_CMD_BOOTD is not set > -- > 2.7.4 > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On 26/09/16 13:30, Jon Medhurst (Tixy) wrote: > On Fri, 2016-09-23 at 17:38 +0100, Sudeep Holla wrote: >> Commit f225d39d3093 ("vexpress: Check TC2 firmware support before defaulting >> to nonsec booting") added support to check if the firmware on TC2 is >> configured appropriately before booting in nonsec/hyp mode. >> >> However when booting in non-secure/hyp mode, CCI control must be done in >> secure firmware and can't be done in non-secure/hyp mode. In order to >> ensure that, this patch disables the cci slave port inteface so that it >> is not accessed at all. >> >> Cc: Jon Medhurst <tixy@linaro.org> >> Acked-by: Marc Zyngier <marc.zyngier@arm.com> >> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> >> --- > > Acked-by: Jon Medhurst <tixy@linaro.org> > Tested-by: Jon Medhurst <tixy@linaro.org> So, can I assume the missing kernel patches to be reason for boot hang ? Just wanted to know if I need to investigate that any further ? -- Regards, Sudeep _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Mon, 2016-09-26 at 13:38 +0100, Sudeep Holla wrote: > > On 26/09/16 13:30, Jon Medhurst (Tixy) wrote: > > On Fri, 2016-09-23 at 17:38 +0100, Sudeep Holla wrote: > >> Commit f225d39d3093 ("vexpress: Check TC2 firmware support before defaulting > >> to nonsec booting") added support to check if the firmware on TC2 is > >> configured appropriately before booting in nonsec/hyp mode. > >> > >> However when booting in non-secure/hyp mode, CCI control must be done in > >> secure firmware and can't be done in non-secure/hyp mode. In order to > >> ensure that, this patch disables the cci slave port inteface so that it > >> is not accessed at all. > >> > >> Cc: Jon Medhurst <tixy@linaro.org> > >> Acked-by: Marc Zyngier <marc.zyngier@arm.com> > >> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > >> --- > > > > Acked-by: Jon Medhurst <tixy@linaro.org> > > Tested-by: Jon Medhurst <tixy@linaro.org> > > So, can I assume the missing kernel patches to be reason for boot hang ? > Just wanted to know if I need to investigate that any further ? Sorry, yes they were the reason and no further investigation needed. I remembered getting nonsec mode working some month's ago without such patches, but I remember now that was by disabling MCPM in the kernel. This morning I tried these U-Boot patches successfully with: - Upstream vexpress_defconfig kernel booting with 'sec' mode - That kernel with Lorenzo's patches in both 'sec' and 'nonsec' - As above with CONFIG_BL_SWITCHER enabled When booting in nonsec I also verified the device-tree modifications made by this patch by seeing the following files existed and contained the word 'disabled'... /proc/device-tree/cci@2c090000/slave-if@4000/status /proc/device-tree/cci@2c090000/slave-if@5000/status -- Tixy _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On 26/09/16 14:19, Jon Medhurst (Tixy) wrote: > On Mon, 2016-09-26 at 13:38 +0100, Sudeep Holla wrote: >> >> On 26/09/16 13:30, Jon Medhurst (Tixy) wrote: >>> On Fri, 2016-09-23 at 17:38 +0100, Sudeep Holla wrote: >>>> Commit f225d39d3093 ("vexpress: Check TC2 firmware support before defaulting >>>> to nonsec booting") added support to check if the firmware on TC2 is >>>> configured appropriately before booting in nonsec/hyp mode. >>>> >>>> However when booting in non-secure/hyp mode, CCI control must be done in >>>> secure firmware and can't be done in non-secure/hyp mode. In order to >>>> ensure that, this patch disables the cci slave port inteface so that it >>>> is not accessed at all. >>>> >>>> Cc: Jon Medhurst <tixy@linaro.org> >>>> Acked-by: Marc Zyngier <marc.zyngier@arm.com> >>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> >>>> --- >>> >>> Acked-by: Jon Medhurst <tixy@linaro.org> >>> Tested-by: Jon Medhurst <tixy@linaro.org> >> >> So, can I assume the missing kernel patches to be reason for boot hang ? >> Just wanted to know if I need to investigate that any further ? > > Sorry, yes they were the reason and no further investigation needed. I > remembered getting nonsec mode working some month's ago without such > patches, but I remember now that was by disabling MCPM in the kernel. > > This morning I tried these U-Boot patches successfully with: > - Upstream vexpress_defconfig kernel booting with 'sec' mode > - That kernel with Lorenzo's patches in both 'sec' and 'nonsec' > - As above with CONFIG_BL_SWITCHER enabled > > When booting in nonsec I also verified the device-tree modifications > made by this patch by seeing the following files existed and contained > the word 'disabled'... > > /proc/device-tree/cci@2c090000/slave-if@4000/status > /proc/device-tree/cci@2c090000/slave-if@5000/status > That was very detailed :), thanks for testing and confirming. I too did similar examination and didn't find anything odd so far. -- Regards, Sudeep _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
diff --git a/board/armltd/vexpress/vexpress_tc2.c b/board/armltd/vexpress/vexpress_tc2.c index ebb41a8833ab..c7adf950f579 100644 --- a/board/armltd/vexpress/vexpress_tc2.c +++ b/board/armltd/vexpress/vexpress_tc2.c @@ -7,7 +7,11 @@ * SPDX-License-Identifier: GPL-2.0+ */ +#include <asm/armv7.h> #include <asm/io.h> +#include <asm/u-boot.h> +#include <common.h> +#include <libfdt.h> #define SCC_BASE 0x7fff0000 @@ -31,3 +35,51 @@ bool armv7_boot_nonsec_default(void) return (readl((u32 *)(SCC_BASE + 0x700)) & ((1 << 12) | (1 << 13))) == 0; #endif } + +#ifdef CONFIG_OF_BOARD_SETUP +int ft_board_setup(void *fdt, bd_t *bd) +{ + int offset, tmp, len; + const struct fdt_property *prop; + const char *cci_compatible = "arm,cci-400-ctrl-if"; + +#ifdef CONFIG_ARMV7_NONSEC + if (!armv7_boot_nonsec()) + return 0; +#else + return 0; +#endif + /* Booting in nonsec mode, disable CCI access */ + offset = fdt_path_offset(fdt, "/cpus"); + if (offset < 0) { + printf("couldn't find /cpus\n"); + return offset; + } + + /* delete cci-control-port in each cpu node */ + for (tmp = fdt_first_subnode(fdt, offset); tmp >= 0; + tmp = fdt_next_subnode(fdt, tmp)) + fdt_delprop(fdt, tmp, "cci-control-port"); + + /* disable all ace cci slave ports */ + offset = fdt_node_offset_by_prop_value(fdt, offset, "compatible", + cci_compatible, 20); + while (offset > 0) { + prop = fdt_get_property(fdt, offset, "interface-type", + &len); + if (!prop) + continue; + if (len < 4) + continue; + if (strcmp(prop->data, "ace")) + continue; + + fdt_setprop_string(fdt, offset, "status", "disabled"); + + offset = fdt_node_offset_by_prop_value(fdt, offset, "compatible", + cci_compatible, 20); + } + + return 0; +} +#endif /* CONFIG_OF_BOARD_SETUP */ diff --git a/configs/vexpress_ca15_tc2_defconfig b/configs/vexpress_ca15_tc2_defconfig index 2f141dda06c6..5154803b7c65 100644 --- a/configs/vexpress_ca15_tc2_defconfig +++ b/configs/vexpress_ca15_tc2_defconfig @@ -1,5 +1,6 @@ CONFIG_ARM=y CONFIG_TARGET_VEXPRESS_CA15_TC2=y +CONFIG_OF_BOARD_SETUP=y CONFIG_HUSH_PARSER=y # CONFIG_CMD_CONSOLE is not set # CONFIG_CMD_BOOTD is not set