* [U-Boot] [PATCH] sunxi: Enable non-secure access to RTC on sun6i (A31s)
@ 2015-08-25 2:49 Chen-Yu Tsai
2015-08-25 7:34 ` Marc Zyngier
2015-08-28 9:47 ` Hans de Goede
0 siblings, 2 replies; 6+ messages in thread
From: Chen-Yu Tsai @ 2015-08-25 2:49 UTC (permalink / raw)
To: u-boot
On the A31s the RTC is by default secured. Thus when u-boot
loads the kernel in non-secure world, the RTC is unavailable. The
SoC has a TrustZone Protection Controller, which can be used to
enable non-secure access to the RTC.
On the A31 the TZPC doesn't seem to do anything, i.e. changes to
its register contents do not affect access to the RTC.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
arch/arm/cpu/armv7/sunxi/Makefile | 1 +
arch/arm/cpu/armv7/sunxi/board.c | 5 +++++
arch/arm/cpu/armv7/sunxi/tzpc.c | 18 ++++++++++++++++++
arch/arm/include/asm/arch-sunxi/tzpc.h | 23 +++++++++++++++++++++++
4 files changed, 47 insertions(+)
create mode 100644 arch/arm/cpu/armv7/sunxi/tzpc.c
create mode 100644 arch/arm/include/asm/arch-sunxi/tzpc.h
diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile
index 76c7e55..459d5d8 100644
--- a/arch/arm/cpu/armv7/sunxi/Makefile
+++ b/arch/arm/cpu/armv7/sunxi/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_MACH_SUN6I) += clock_sun6i.o
obj-$(CONFIG_MACH_SUN7I) += clock_sun4i.o
obj-$(CONFIG_MACH_SUN8I) += clock_sun6i.o
obj-$(CONFIG_MACH_SUN9I) += clock_sun9i.o
+obj-$(CONFIG_MACH_SUN6I) += tzpc.o
obj-$(CONFIG_AXP152_POWER) += pmic_bus.o
obj-$(CONFIG_AXP209_POWER) += pmic_bus.o
diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c
index f01846e..b40198b 100644
--- a/arch/arm/cpu/armv7/sunxi/board.c
+++ b/arch/arm/cpu/armv7/sunxi/board.c
@@ -23,6 +23,7 @@
#include <asm/arch/gpio.h>
#include <asm/arch/sys_proto.h>
#include <asm/arch/timer.h>
+#include <asm/arch/tzpc.h>
#include <asm/arch/mmc.h>
#include <linux/compiler.h>
@@ -115,6 +116,10 @@ void s_init(void)
"orr r0, r0, #1 << 6\n"
"mcr p15, 0, r0, c1, c0, 1\n");
#endif
+#if defined CONFIG_MACH_SUN6I
+ /* Enable non-secure access to the RTC */
+ tzpc_init();
+#endif
clock_init();
timer_init();
diff --git a/arch/arm/cpu/armv7/sunxi/tzpc.c b/arch/arm/cpu/armv7/sunxi/tzpc.c
new file mode 100644
index 0000000..5c9c69b
--- /dev/null
+++ b/arch/arm/cpu/armv7/sunxi/tzpc.c
@@ -0,0 +1,18 @@
+/*
+ * (C) Copyright 2015 Chen-Yu Tsai <wens@csie.org>
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include <asm/io.h>
+#include <asm/arch/cpu.h>
+#include <asm/arch/tzpc.h>
+
+/* Configure Trust Zone Protection Controller */
+void tzpc_init(void)
+{
+ struct sunxi_tzpc *tzpc = (struct sunxi_tzpc *)SUNXI_TZPC_BASE;
+
+ /* Enable non-secure access to the RTC */
+ writel(SUNXI_TZPC_DECPORT0_RTC, &tzpc->decport0_set);
+}
diff --git a/arch/arm/include/asm/arch-sunxi/tzpc.h b/arch/arm/include/asm/arch-sunxi/tzpc.h
new file mode 100644
index 0000000..ba4d43b
--- /dev/null
+++ b/arch/arm/include/asm/arch-sunxi/tzpc.h
@@ -0,0 +1,23 @@
+/*
+ * (C) Copyright 2015 Chen-Yu Tsai <wens@csie.org>
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#ifndef _SUNXI_TZPC_H
+#define _SUNXI_TZPC_H
+
+#ifndef __ASSEMBLY__
+struct sunxi_tzpc {
+ u32 r0size; /* 0x00 Size of secure RAM region */
+ u32 decport0_status; /* 0x04 Status of decode protection port 0 */
+ u32 decport0_set; /* 0x08 Set decode protection port 0 */
+ u32 decport0_clear; /* 0x0c Clear decode protection port 0 */
+};
+#endif
+
+#define SUNXI_TZPC_DECPORT0_RTC (1 << 1)
+
+void tzpc_init(void);
+
+#endif /* _SUNXI_TZPC_H */
--
2.5.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] sunxi: Enable non-secure access to RTC on sun6i (A31s)
2015-08-25 2:49 [U-Boot] [PATCH] sunxi: Enable non-secure access to RTC on sun6i (A31s) Chen-Yu Tsai
@ 2015-08-25 7:34 ` Marc Zyngier
2015-08-25 7:40 ` Chen-Yu Tsai
2015-08-28 9:47 ` Hans de Goede
1 sibling, 1 reply; 6+ messages in thread
From: Marc Zyngier @ 2015-08-25 7:34 UTC (permalink / raw)
To: u-boot
On Tue, 25 Aug 2015 10:49:19 +0800
Chen-Yu Tsai <wens@csie.org> wrote:
Hi,
Thanks for putting this patch together. A few comments below:
> On the A31s the RTC is by default secured. Thus when u-boot
> loads the kernel in non-secure world, the RTC is unavailable. The
> SoC has a TrustZone Protection Controller, which can be used to
> enable non-secure access to the RTC.
>
> On the A31 the TZPC doesn't seem to do anything, i.e. changes to
> its register contents do not affect access to the RTC.
Does it mean that the RTC is still inaccessible? Or that it has always
been accessible?
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
> arch/arm/cpu/armv7/sunxi/Makefile | 1 +
> arch/arm/cpu/armv7/sunxi/board.c | 5 +++++
> arch/arm/cpu/armv7/sunxi/tzpc.c | 18 ++++++++++++++++++
> arch/arm/include/asm/arch-sunxi/tzpc.h | 23 +++++++++++++++++++++++
> 4 files changed, 47 insertions(+)
> create mode 100644 arch/arm/cpu/armv7/sunxi/tzpc.c
> create mode 100644 arch/arm/include/asm/arch-sunxi/tzpc.h
>
> diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile
> index 76c7e55..459d5d8 100644
> --- a/arch/arm/cpu/armv7/sunxi/Makefile
> +++ b/arch/arm/cpu/armv7/sunxi/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_MACH_SUN6I) += clock_sun6i.o
> obj-$(CONFIG_MACH_SUN7I) += clock_sun4i.o
> obj-$(CONFIG_MACH_SUN8I) += clock_sun6i.o
> obj-$(CONFIG_MACH_SUN9I) += clock_sun9i.o
> +obj-$(CONFIG_MACH_SUN6I) += tzpc.o
>
> obj-$(CONFIG_AXP152_POWER) += pmic_bus.o
> obj-$(CONFIG_AXP209_POWER) += pmic_bus.o
> diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c
> index f01846e..b40198b 100644
> --- a/arch/arm/cpu/armv7/sunxi/board.c
> +++ b/arch/arm/cpu/armv7/sunxi/board.c
> @@ -23,6 +23,7 @@
> #include <asm/arch/gpio.h>
> #include <asm/arch/sys_proto.h>
> #include <asm/arch/timer.h>
> +#include <asm/arch/tzpc.h>
> #include <asm/arch/mmc.h>
>
> #include <linux/compiler.h>
> @@ -115,6 +116,10 @@ void s_init(void)
> "orr r0, r0, #1 << 6\n"
> "mcr p15, 0, r0, c1, c0, 1\n");
> #endif
> +#if defined CONFIG_MACH_SUN6I
> + /* Enable non-secure access to the RTC */
> + tzpc_init();
> +#endif
>
> clock_init();
> timer_init();
> diff --git a/arch/arm/cpu/armv7/sunxi/tzpc.c b/arch/arm/cpu/armv7/sunxi/tzpc.c
> new file mode 100644
> index 0000000..5c9c69b
> --- /dev/null
> +++ b/arch/arm/cpu/armv7/sunxi/tzpc.c
> @@ -0,0 +1,18 @@
> +/*
> + * (C) Copyright 2015 Chen-Yu Tsai <wens@csie.org>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <asm/io.h>
> +#include <asm/arch/cpu.h>
> +#include <asm/arch/tzpc.h>
> +
> +/* Configure Trust Zone Protection Controller */
> +void tzpc_init(void)
> +{
> + struct sunxi_tzpc *tzpc = (struct sunxi_tzpc *)SUNXI_TZPC_BASE;
> +
> + /* Enable non-secure access to the RTC */
> + writel(SUNXI_TZPC_DECPORT0_RTC, &tzpc->decport0_set);
Irk. Seem blow.
> +}
> diff --git a/arch/arm/include/asm/arch-sunxi/tzpc.h b/arch/arm/include/asm/arch-sunxi/tzpc.h
> new file mode 100644
> index 0000000..ba4d43b
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-sunxi/tzpc.h
> @@ -0,0 +1,23 @@
> +/*
> + * (C) Copyright 2015 Chen-Yu Tsai <wens@csie.org>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#ifndef _SUNXI_TZPC_H
> +#define _SUNXI_TZPC_H
Why is this sunxi specific? This seems to describe a standard ARM BP147
(http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dto0015a/index.html),
so I'd expect the various offsets and the file location to reflect this.
Another platform (FSL 2085) seems to use the same IP, so it would make
sense to clean that part of the code too.
> +
> +#ifndef __ASSEMBLY__
> +struct sunxi_tzpc {
> + u32 r0size; /* 0x00 Size of secure RAM region */
> + u32 decport0_status; /* 0x04 Status of decode protection port 0 */
> + u32 decport0_set; /* 0x08 Set decode protection port 0 */
> + u32 decport0_clear; /* 0x0c Clear decode protection port 0 */
> +};
I'm not overly fond of this way to describe a set of contiguous
register. It tends to be fairly fragile (the compiler is free to
insert some padding) and prevents the use of offsets from assembly code.
> +#endif
> +
> +#define SUNXI_TZPC_DECPORT0_RTC (1 << 1)
> +
> +void tzpc_init(void);
And this won't help if you have assembly code either.
> +
> +#endif /* _SUNXI_TZPC_H */
If I can suggest something, it would be to make this something generic,
with a standard config option describing the base, a set of offsets
from the base (as #defines), and then a set of accessors accessible from
C code.
LS2085 could also be cleaned up to reuse the same offsets (instead of
hardcoding the register addresses).
Thanks,
M.
--
Jazz is not dead. It just smells funny.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] sunxi: Enable non-secure access to RTC on sun6i (A31s)
2015-08-25 7:34 ` Marc Zyngier
@ 2015-08-25 7:40 ` Chen-Yu Tsai
2015-08-25 7:46 ` Hans de Goede
2015-08-25 10:08 ` Marc Zyngier
0 siblings, 2 replies; 6+ messages in thread
From: Chen-Yu Tsai @ 2015-08-25 7:40 UTC (permalink / raw)
To: u-boot
On Tue, Aug 25, 2015 at 3:34 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On Tue, 25 Aug 2015 10:49:19 +0800
> Chen-Yu Tsai <wens@csie.org> wrote:
>
> Hi,
>
> Thanks for putting this patch together. A few comments below:
>
>> On the A31s the RTC is by default secured. Thus when u-boot
>> loads the kernel in non-secure world, the RTC is unavailable. The
>> SoC has a TrustZone Protection Controller, which can be used to
>> enable non-secure access to the RTC.
>>
>> On the A31 the TZPC doesn't seem to do anything, i.e. changes to
>> its register contents do not affect access to the RTC.
>
> Does it mean that the RTC is still inaccessible? Or that it has always
> been accessible?
Means its always accessible.
>>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> ---
>> arch/arm/cpu/armv7/sunxi/Makefile | 1 +
>> arch/arm/cpu/armv7/sunxi/board.c | 5 +++++
>> arch/arm/cpu/armv7/sunxi/tzpc.c | 18 ++++++++++++++++++
>> arch/arm/include/asm/arch-sunxi/tzpc.h | 23 +++++++++++++++++++++++
>> 4 files changed, 47 insertions(+)
>> create mode 100644 arch/arm/cpu/armv7/sunxi/tzpc.c
>> create mode 100644 arch/arm/include/asm/arch-sunxi/tzpc.h
>>
>> diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile
>> index 76c7e55..459d5d8 100644
>> --- a/arch/arm/cpu/armv7/sunxi/Makefile
>> +++ b/arch/arm/cpu/armv7/sunxi/Makefile
>> @@ -28,6 +28,7 @@ obj-$(CONFIG_MACH_SUN6I) += clock_sun6i.o
>> obj-$(CONFIG_MACH_SUN7I) += clock_sun4i.o
>> obj-$(CONFIG_MACH_SUN8I) += clock_sun6i.o
>> obj-$(CONFIG_MACH_SUN9I) += clock_sun9i.o
>> +obj-$(CONFIG_MACH_SUN6I) += tzpc.o
>>
>> obj-$(CONFIG_AXP152_POWER) += pmic_bus.o
>> obj-$(CONFIG_AXP209_POWER) += pmic_bus.o
>> diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c
>> index f01846e..b40198b 100644
>> --- a/arch/arm/cpu/armv7/sunxi/board.c
>> +++ b/arch/arm/cpu/armv7/sunxi/board.c
>> @@ -23,6 +23,7 @@
>> #include <asm/arch/gpio.h>
>> #include <asm/arch/sys_proto.h>
>> #include <asm/arch/timer.h>
>> +#include <asm/arch/tzpc.h>
>> #include <asm/arch/mmc.h>
>>
>> #include <linux/compiler.h>
>> @@ -115,6 +116,10 @@ void s_init(void)
>> "orr r0, r0, #1 << 6\n"
>> "mcr p15, 0, r0, c1, c0, 1\n");
>> #endif
>> +#if defined CONFIG_MACH_SUN6I
>> + /* Enable non-secure access to the RTC */
>> + tzpc_init();
>> +#endif
>>
>> clock_init();
>> timer_init();
>> diff --git a/arch/arm/cpu/armv7/sunxi/tzpc.c b/arch/arm/cpu/armv7/sunxi/tzpc.c
>> new file mode 100644
>> index 0000000..5c9c69b
>> --- /dev/null
>> +++ b/arch/arm/cpu/armv7/sunxi/tzpc.c
>> @@ -0,0 +1,18 @@
>> +/*
>> + * (C) Copyright 2015 Chen-Yu Tsai <wens@csie.org>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +#include <asm/io.h>
>> +#include <asm/arch/cpu.h>
>> +#include <asm/arch/tzpc.h>
>> +
>> +/* Configure Trust Zone Protection Controller */
>> +void tzpc_init(void)
>> +{
>> + struct sunxi_tzpc *tzpc = (struct sunxi_tzpc *)SUNXI_TZPC_BASE;
>> +
>> + /* Enable non-secure access to the RTC */
>> + writel(SUNXI_TZPC_DECPORT0_RTC, &tzpc->decport0_set);
>
> Irk. Seem blow.
>
>> +}
>> diff --git a/arch/arm/include/asm/arch-sunxi/tzpc.h b/arch/arm/include/asm/arch-sunxi/tzpc.h
>> new file mode 100644
>> index 0000000..ba4d43b
>> --- /dev/null
>> +++ b/arch/arm/include/asm/arch-sunxi/tzpc.h
>> @@ -0,0 +1,23 @@
>> +/*
>> + * (C) Copyright 2015 Chen-Yu Tsai <wens@csie.org>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +#ifndef _SUNXI_TZPC_H
>> +#define _SUNXI_TZPC_H
>
> Why is this sunxi specific? This seems to describe a standard ARM BP147
> (http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dto0015a/index.html),
> so I'd expect the various offsets and the file location to reflect this.
>
> Another platform (FSL 2085) seems to use the same IP, so it would make
> sense to clean that part of the code too.
I guess we could make this generic. Seems samsung uses this as well.
>> +
>> +#ifndef __ASSEMBLY__
>> +struct sunxi_tzpc {
>> + u32 r0size; /* 0x00 Size of secure RAM region */
>> + u32 decport0_status; /* 0x04 Status of decode protection port 0 */
>> + u32 decport0_set; /* 0x08 Set decode protection port 0 */
>> + u32 decport0_clear; /* 0x0c Clear decode protection port 0 */
>> +};
>
> I'm not overly fond of this way to describe a set of contiguous
> register. It tends to be fairly fragile (the compiler is free to
> insert some padding) and prevents the use of offsets from assembly code.
Seems this is how we are doing things in U-boot...
I'm OK either way.
>> +#endif
>> +
>> +#define SUNXI_TZPC_DECPORT0_RTC (1 << 1)
>> +
>> +void tzpc_init(void);
>
> And this won't help if you have assembly code either.
>
>> +
>> +#endif /* _SUNXI_TZPC_H */
>
> If I can suggest something, it would be to make this something generic,
> with a standard config option describing the base, a set of offsets
> from the base (as #defines), and then a set of accessors accessible from
> C code.
>
> LS2085 could also be cleaned up to reuse the same offsets (instead of
> hardcoding the register addresses).
OK. Will look into it.
ChenYu
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] sunxi: Enable non-secure access to RTC on sun6i (A31s)
2015-08-25 7:40 ` Chen-Yu Tsai
@ 2015-08-25 7:46 ` Hans de Goede
2015-08-25 10:08 ` Marc Zyngier
1 sibling, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2015-08-25 7:46 UTC (permalink / raw)
To: u-boot
Hi,
On 25-08-15 09:40, Chen-Yu Tsai wrote:
> On Tue, Aug 25, 2015 at 3:34 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> On Tue, 25 Aug 2015 10:49:19 +0800
>> Chen-Yu Tsai <wens@csie.org> wrote:
>>
>> Hi,
>>
>> Thanks for putting this patch together. A few comments below:
>>
>>> On the A31s the RTC is by default secured. Thus when u-boot
>>> loads the kernel in non-secure world, the RTC is unavailable. The
>>> SoC has a TrustZone Protection Controller, which can be used to
>>> enable non-secure access to the RTC.
>>>
>>> On the A31 the TZPC doesn't seem to do anything, i.e. changes to
>>> its register contents do not affect access to the RTC.
>>
>> Does it mean that the RTC is still inaccessible? Or that it has always
>> been accessible?
>
> Means its always accessible.
Ah, so what you're saying is that on A31 the RTC is always accessible and
on A31s it depends on the TZPC settings ?
That does not make sense since both are the same die used in a different
package, so chances are this does not depend on A31 vs A31s but on a
difference in revision of the die.
It does not matter, we can just flip the bit in the TZPC everywhere to
make sure the kernel has access to the RTC.
Regards,
Hans
>
>>>
>>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>>> ---
>>> arch/arm/cpu/armv7/sunxi/Makefile | 1 +
>>> arch/arm/cpu/armv7/sunxi/board.c | 5 +++++
>>> arch/arm/cpu/armv7/sunxi/tzpc.c | 18 ++++++++++++++++++
>>> arch/arm/include/asm/arch-sunxi/tzpc.h | 23 +++++++++++++++++++++++
>>> 4 files changed, 47 insertions(+)
>>> create mode 100644 arch/arm/cpu/armv7/sunxi/tzpc.c
>>> create mode 100644 arch/arm/include/asm/arch-sunxi/tzpc.h
>>>
>>> diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile
>>> index 76c7e55..459d5d8 100644
>>> --- a/arch/arm/cpu/armv7/sunxi/Makefile
>>> +++ b/arch/arm/cpu/armv7/sunxi/Makefile
>>> @@ -28,6 +28,7 @@ obj-$(CONFIG_MACH_SUN6I) += clock_sun6i.o
>>> obj-$(CONFIG_MACH_SUN7I) += clock_sun4i.o
>>> obj-$(CONFIG_MACH_SUN8I) += clock_sun6i.o
>>> obj-$(CONFIG_MACH_SUN9I) += clock_sun9i.o
>>> +obj-$(CONFIG_MACH_SUN6I) += tzpc.o
>>>
>>> obj-$(CONFIG_AXP152_POWER) += pmic_bus.o
>>> obj-$(CONFIG_AXP209_POWER) += pmic_bus.o
>>> diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c
>>> index f01846e..b40198b 100644
>>> --- a/arch/arm/cpu/armv7/sunxi/board.c
>>> +++ b/arch/arm/cpu/armv7/sunxi/board.c
>>> @@ -23,6 +23,7 @@
>>> #include <asm/arch/gpio.h>
>>> #include <asm/arch/sys_proto.h>
>>> #include <asm/arch/timer.h>
>>> +#include <asm/arch/tzpc.h>
>>> #include <asm/arch/mmc.h>
>>>
>>> #include <linux/compiler.h>
>>> @@ -115,6 +116,10 @@ void s_init(void)
>>> "orr r0, r0, #1 << 6\n"
>>> "mcr p15, 0, r0, c1, c0, 1\n");
>>> #endif
>>> +#if defined CONFIG_MACH_SUN6I
>>> + /* Enable non-secure access to the RTC */
>>> + tzpc_init();
>>> +#endif
>>>
>>> clock_init();
>>> timer_init();
>>> diff --git a/arch/arm/cpu/armv7/sunxi/tzpc.c b/arch/arm/cpu/armv7/sunxi/tzpc.c
>>> new file mode 100644
>>> index 0000000..5c9c69b
>>> --- /dev/null
>>> +++ b/arch/arm/cpu/armv7/sunxi/tzpc.c
>>> @@ -0,0 +1,18 @@
>>> +/*
>>> + * (C) Copyright 2015 Chen-Yu Tsai <wens@csie.org>
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0+
>>> + */
>>> +
>>> +#include <asm/io.h>
>>> +#include <asm/arch/cpu.h>
>>> +#include <asm/arch/tzpc.h>
>>> +
>>> +/* Configure Trust Zone Protection Controller */
>>> +void tzpc_init(void)
>>> +{
>>> + struct sunxi_tzpc *tzpc = (struct sunxi_tzpc *)SUNXI_TZPC_BASE;
>>> +
>>> + /* Enable non-secure access to the RTC */
>>> + writel(SUNXI_TZPC_DECPORT0_RTC, &tzpc->decport0_set);
>>
>> Irk. Seem blow.
>>
>>> +}
>>> diff --git a/arch/arm/include/asm/arch-sunxi/tzpc.h b/arch/arm/include/asm/arch-sunxi/tzpc.h
>>> new file mode 100644
>>> index 0000000..ba4d43b
>>> --- /dev/null
>>> +++ b/arch/arm/include/asm/arch-sunxi/tzpc.h
>>> @@ -0,0 +1,23 @@
>>> +/*
>>> + * (C) Copyright 2015 Chen-Yu Tsai <wens@csie.org>
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0+
>>> + */
>>> +
>>> +#ifndef _SUNXI_TZPC_H
>>> +#define _SUNXI_TZPC_H
>>
>> Why is this sunxi specific? This seems to describe a standard ARM BP147
>> (http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dto0015a/index.html),
>> so I'd expect the various offsets and the file location to reflect this.
>>
>> Another platform (FSL 2085) seems to use the same IP, so it would make
>> sense to clean that part of the code too.
>
> I guess we could make this generic. Seems samsung uses this as well.
>
>>> +
>>> +#ifndef __ASSEMBLY__
>>> +struct sunxi_tzpc {
>>> + u32 r0size; /* 0x00 Size of secure RAM region */
>>> + u32 decport0_status; /* 0x04 Status of decode protection port 0 */
>>> + u32 decport0_set; /* 0x08 Set decode protection port 0 */
>>> + u32 decport0_clear; /* 0x0c Clear decode protection port 0 */
>>> +};
>>
>> I'm not overly fond of this way to describe a set of contiguous
>> register. It tends to be fairly fragile (the compiler is free to
>> insert some padding) and prevents the use of offsets from assembly code.
>
> Seems this is how we are doing things in U-boot...
> I'm OK either way.
>
>>> +#endif
>>> +
>>> +#define SUNXI_TZPC_DECPORT0_RTC (1 << 1)
>>> +
>>> +void tzpc_init(void);
>>
>> And this won't help if you have assembly code either.
>>
>>> +
>>> +#endif /* _SUNXI_TZPC_H */
>>
>> If I can suggest something, it would be to make this something generic,
>> with a standard config option describing the base, a set of offsets
>> from the base (as #defines), and then a set of accessors accessible from
>> C code.
>>
>> LS2085 could also be cleaned up to reuse the same offsets (instead of
>> hardcoding the register addresses).
>
> OK. Will look into it.
>
> ChenYu
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] sunxi: Enable non-secure access to RTC on sun6i (A31s)
2015-08-25 7:40 ` Chen-Yu Tsai
2015-08-25 7:46 ` Hans de Goede
@ 2015-08-25 10:08 ` Marc Zyngier
1 sibling, 0 replies; 6+ messages in thread
From: Marc Zyngier @ 2015-08-25 10:08 UTC (permalink / raw)
To: u-boot
On 25/08/15 08:40, Chen-Yu Tsai wrote:
> On Tue, Aug 25, 2015 at 3:34 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> On Tue, 25 Aug 2015 10:49:19 +0800
>> Chen-Yu Tsai <wens@csie.org> wrote:
>>
>> Hi,
>>
>> Thanks for putting this patch together. A few comments below:
>>
>>> On the A31s the RTC is by default secured. Thus when u-boot
>>> loads the kernel in non-secure world, the RTC is unavailable. The
>>> SoC has a TrustZone Protection Controller, which can be used to
>>> enable non-secure access to the RTC.
>>>
>>> On the A31 the TZPC doesn't seem to do anything, i.e. changes to
>>> its register contents do not affect access to the RTC.
>>
>> Does it mean that the RTC is still inaccessible? Or that it has always
>> been accessible?
>
> Means its always accessible.
>
>>>
>>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>>> ---
>>> arch/arm/cpu/armv7/sunxi/Makefile | 1 +
>>> arch/arm/cpu/armv7/sunxi/board.c | 5 +++++
>>> arch/arm/cpu/armv7/sunxi/tzpc.c | 18 ++++++++++++++++++
>>> arch/arm/include/asm/arch-sunxi/tzpc.h | 23 +++++++++++++++++++++++
>>> 4 files changed, 47 insertions(+)
>>> create mode 100644 arch/arm/cpu/armv7/sunxi/tzpc.c
>>> create mode 100644 arch/arm/include/asm/arch-sunxi/tzpc.h
>>>
>>> diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile
>>> index 76c7e55..459d5d8 100644
>>> --- a/arch/arm/cpu/armv7/sunxi/Makefile
>>> +++ b/arch/arm/cpu/armv7/sunxi/Makefile
>>> @@ -28,6 +28,7 @@ obj-$(CONFIG_MACH_SUN6I) += clock_sun6i.o
>>> obj-$(CONFIG_MACH_SUN7I) += clock_sun4i.o
>>> obj-$(CONFIG_MACH_SUN8I) += clock_sun6i.o
>>> obj-$(CONFIG_MACH_SUN9I) += clock_sun9i.o
>>> +obj-$(CONFIG_MACH_SUN6I) += tzpc.o
>>>
>>> obj-$(CONFIG_AXP152_POWER) += pmic_bus.o
>>> obj-$(CONFIG_AXP209_POWER) += pmic_bus.o
>>> diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c
>>> index f01846e..b40198b 100644
>>> --- a/arch/arm/cpu/armv7/sunxi/board.c
>>> +++ b/arch/arm/cpu/armv7/sunxi/board.c
>>> @@ -23,6 +23,7 @@
>>> #include <asm/arch/gpio.h>
>>> #include <asm/arch/sys_proto.h>
>>> #include <asm/arch/timer.h>
>>> +#include <asm/arch/tzpc.h>
>>> #include <asm/arch/mmc.h>
>>>
>>> #include <linux/compiler.h>
>>> @@ -115,6 +116,10 @@ void s_init(void)
>>> "orr r0, r0, #1 << 6\n"
>>> "mcr p15, 0, r0, c1, c0, 1\n");
>>> #endif
>>> +#if defined CONFIG_MACH_SUN6I
>>> + /* Enable non-secure access to the RTC */
>>> + tzpc_init();
>>> +#endif
>>>
>>> clock_init();
>>> timer_init();
>>> diff --git a/arch/arm/cpu/armv7/sunxi/tzpc.c b/arch/arm/cpu/armv7/sunxi/tzpc.c
>>> new file mode 100644
>>> index 0000000..5c9c69b
>>> --- /dev/null
>>> +++ b/arch/arm/cpu/armv7/sunxi/tzpc.c
>>> @@ -0,0 +1,18 @@
>>> +/*
>>> + * (C) Copyright 2015 Chen-Yu Tsai <wens@csie.org>
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0+
>>> + */
>>> +
>>> +#include <asm/io.h>
>>> +#include <asm/arch/cpu.h>
>>> +#include <asm/arch/tzpc.h>
>>> +
>>> +/* Configure Trust Zone Protection Controller */
>>> +void tzpc_init(void)
>>> +{
>>> + struct sunxi_tzpc *tzpc = (struct sunxi_tzpc *)SUNXI_TZPC_BASE;
>>> +
>>> + /* Enable non-secure access to the RTC */
>>> + writel(SUNXI_TZPC_DECPORT0_RTC, &tzpc->decport0_set);
>>
>> Irk. Seem blow.
>>
>>> +}
>>> diff --git a/arch/arm/include/asm/arch-sunxi/tzpc.h b/arch/arm/include/asm/arch-sunxi/tzpc.h
>>> new file mode 100644
>>> index 0000000..ba4d43b
>>> --- /dev/null
>>> +++ b/arch/arm/include/asm/arch-sunxi/tzpc.h
>>> @@ -0,0 +1,23 @@
>>> +/*
>>> + * (C) Copyright 2015 Chen-Yu Tsai <wens@csie.org>
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0+
>>> + */
>>> +
>>> +#ifndef _SUNXI_TZPC_H
>>> +#define _SUNXI_TZPC_H
>>
>> Why is this sunxi specific? This seems to describe a standard ARM BP147
>> (http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dto0015a/index.html),
>> so I'd expect the various offsets and the file location to reflect this.
>>
>> Another platform (FSL 2085) seems to use the same IP, so it would make
>> sense to clean that part of the code too.
>
> I guess we could make this generic. Seems samsung uses this as well.
>
>>> +
>>> +#ifndef __ASSEMBLY__
>>> +struct sunxi_tzpc {
>>> + u32 r0size; /* 0x00 Size of secure RAM region */
>>> + u32 decport0_status; /* 0x04 Status of decode protection port 0 */
>>> + u32 decport0_set; /* 0x08 Set decode protection port 0 */
>>> + u32 decport0_clear; /* 0x0c Clear decode protection port 0 */
>>> +};
>>
>> I'm not overly fond of this way to describe a set of contiguous
>> register. It tends to be fairly fragile (the compiler is free to
>> insert some padding) and prevents the use of offsets from assembly code.
>
> Seems this is how we are doing things in U-boot...
> I'm OK either way.
>
>>> +#endif
>>> +
>>> +#define SUNXI_TZPC_DECPORT0_RTC (1 << 1)
>>> +
>>> +void tzpc_init(void);
>>
>> And this won't help if you have assembly code either.
>>
>>> +
>>> +#endif /* _SUNXI_TZPC_H */
>>
>> If I can suggest something, it would be to make this something generic,
>> with a standard config option describing the base, a set of offsets
>> from the base (as #defines), and then a set of accessors accessible from
>> C code.
>>
>> LS2085 could also be cleaned up to reuse the same offsets (instead of
>> hardcoding the register addresses).
>
> OK. Will look into it.
As I was pointed out over IRC, the memory map is subtly different from
the original BP147 (hey, being awkward is be a good thing, isn't it?),
which makes it difficult to have common code for this.
So let's not bother having a generic BP147 setup for now - at least not
for sunxi. It could still be done for FSL and Samsung, but that'd be
unfair to ask you to do this.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] sunxi: Enable non-secure access to RTC on sun6i (A31s)
2015-08-25 2:49 [U-Boot] [PATCH] sunxi: Enable non-secure access to RTC on sun6i (A31s) Chen-Yu Tsai
2015-08-25 7:34 ` Marc Zyngier
@ 2015-08-28 9:47 ` Hans de Goede
1 sibling, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2015-08-28 9:47 UTC (permalink / raw)
To: u-boot
Hi,
On 25-08-15 04:49, Chen-Yu Tsai wrote:
> On the A31s the RTC is by default secured. Thus when u-boot
> loads the kernel in non-secure world, the RTC is unavailable. The
> SoC has a TrustZone Protection Controller, which can be used to
> enable non-secure access to the RTC.
>
> On the A31 the TZPC doesn't seem to do anything, i.e. changes to
> its register contents do not affect access to the RTC.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Thanks I've queued this up in my tree, and it will be
part of my next pull-req for u-boot/master.
Regards,
Hans
> ---
> arch/arm/cpu/armv7/sunxi/Makefile | 1 +
> arch/arm/cpu/armv7/sunxi/board.c | 5 +++++
> arch/arm/cpu/armv7/sunxi/tzpc.c | 18 ++++++++++++++++++
> arch/arm/include/asm/arch-sunxi/tzpc.h | 23 +++++++++++++++++++++++
> 4 files changed, 47 insertions(+)
> create mode 100644 arch/arm/cpu/armv7/sunxi/tzpc.c
> create mode 100644 arch/arm/include/asm/arch-sunxi/tzpc.h
>
> diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile
> index 76c7e55..459d5d8 100644
> --- a/arch/arm/cpu/armv7/sunxi/Makefile
> +++ b/arch/arm/cpu/armv7/sunxi/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_MACH_SUN6I) += clock_sun6i.o
> obj-$(CONFIG_MACH_SUN7I) += clock_sun4i.o
> obj-$(CONFIG_MACH_SUN8I) += clock_sun6i.o
> obj-$(CONFIG_MACH_SUN9I) += clock_sun9i.o
> +obj-$(CONFIG_MACH_SUN6I) += tzpc.o
>
> obj-$(CONFIG_AXP152_POWER) += pmic_bus.o
> obj-$(CONFIG_AXP209_POWER) += pmic_bus.o
> diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c
> index f01846e..b40198b 100644
> --- a/arch/arm/cpu/armv7/sunxi/board.c
> +++ b/arch/arm/cpu/armv7/sunxi/board.c
> @@ -23,6 +23,7 @@
> #include <asm/arch/gpio.h>
> #include <asm/arch/sys_proto.h>
> #include <asm/arch/timer.h>
> +#include <asm/arch/tzpc.h>
> #include <asm/arch/mmc.h>
>
> #include <linux/compiler.h>
> @@ -115,6 +116,10 @@ void s_init(void)
> "orr r0, r0, #1 << 6\n"
> "mcr p15, 0, r0, c1, c0, 1\n");
> #endif
> +#if defined CONFIG_MACH_SUN6I
> + /* Enable non-secure access to the RTC */
> + tzpc_init();
> +#endif
>
> clock_init();
> timer_init();
> diff --git a/arch/arm/cpu/armv7/sunxi/tzpc.c b/arch/arm/cpu/armv7/sunxi/tzpc.c
> new file mode 100644
> index 0000000..5c9c69b
> --- /dev/null
> +++ b/arch/arm/cpu/armv7/sunxi/tzpc.c
> @@ -0,0 +1,18 @@
> +/*
> + * (C) Copyright 2015 Chen-Yu Tsai <wens@csie.org>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <asm/io.h>
> +#include <asm/arch/cpu.h>
> +#include <asm/arch/tzpc.h>
> +
> +/* Configure Trust Zone Protection Controller */
> +void tzpc_init(void)
> +{
> + struct sunxi_tzpc *tzpc = (struct sunxi_tzpc *)SUNXI_TZPC_BASE;
> +
> + /* Enable non-secure access to the RTC */
> + writel(SUNXI_TZPC_DECPORT0_RTC, &tzpc->decport0_set);
> +}
> diff --git a/arch/arm/include/asm/arch-sunxi/tzpc.h b/arch/arm/include/asm/arch-sunxi/tzpc.h
> new file mode 100644
> index 0000000..ba4d43b
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-sunxi/tzpc.h
> @@ -0,0 +1,23 @@
> +/*
> + * (C) Copyright 2015 Chen-Yu Tsai <wens@csie.org>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#ifndef _SUNXI_TZPC_H
> +#define _SUNXI_TZPC_H
> +
> +#ifndef __ASSEMBLY__
> +struct sunxi_tzpc {
> + u32 r0size; /* 0x00 Size of secure RAM region */
> + u32 decport0_status; /* 0x04 Status of decode protection port 0 */
> + u32 decport0_set; /* 0x08 Set decode protection port 0 */
> + u32 decport0_clear; /* 0x0c Clear decode protection port 0 */
> +};
> +#endif
> +
> +#define SUNXI_TZPC_DECPORT0_RTC (1 << 1)
> +
> +void tzpc_init(void);
> +
> +#endif /* _SUNXI_TZPC_H */
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-08-28 9:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-25 2:49 [U-Boot] [PATCH] sunxi: Enable non-secure access to RTC on sun6i (A31s) Chen-Yu Tsai
2015-08-25 7:34 ` Marc Zyngier
2015-08-25 7:40 ` Chen-Yu Tsai
2015-08-25 7:46 ` Hans de Goede
2015-08-25 10:08 ` Marc Zyngier
2015-08-28 9:47 ` Hans de Goede
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.