[PATCH 0/2] ARM: EXYNOS: Allow to use architected timer

[PATCH 0/2] ARM: EXYNOS: Allow to use architected timer

[Pavel Fedin]

Exynos platform officially uses "multi-core timer" AKA MCT. However, it
also has a possibility to access the same timer using CP15 (AKA
architected timer) registers.

In order to be able to use KVM, we need CP15 timer support. In its turn,
MCT needs to be turned on before any accesses will work. This patchset
allows to specify "CP15 compatibility mode" for the MCT by inserting
armv7-timer node as a child for MCT device. In this case MCT driver will
only enable the timer, leaving the rest for architected timer driver.

Pavel Fedin (2):
  exynos_mct: Allow to use architected timer mode
  ARM: dts: Use architected timer on Exynos 5410

 arch/arm/boot/dts/exynos5410.dtsi | 11 +++++++++++
 drivers/clocksource/exynos_mct.c  | 19 +++++++++++++------
 2 files changed, 24 insertions(+), 6 deletions(-)

-- 
2.4.4

[PATCH 1/2] exynos_mct: Allow to use architected timer mode

[Pavel Fedin]

MCT is actually compatible with architected timer and can be accessed
using CP15 registers. This allows to use virtualization extensions on
Exynos boards, provided you have the appropriate bootloader.

Compatibility mode is enabled if armv7-timer device node is inserted
into MCT node. In this case the driver will only enable counter and
exit, leaving the rest for the architected timer driver.

Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
 drivers/clocksource/exynos_mct.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index 029f96a..a48492d 100644
--- a/drivers/clocksource/exynos_mct.c
+++ b/drivers/clocksource/exynos_mct.c
@@ -510,7 +510,7 @@ static struct notifier_block exynos4_mct_cpu_nb = {
 	.notifier_call = exynos4_mct_cpu_notify,
 };
 
-static void __init exynos4_timer_resources(struct device_node *np, void __iomem *base)
+static void __init exynos4_timer_resources(struct device_node *np)
 {
 	int err, cpu;
 	struct mct_clock_event_device *mevt = this_cpu_ptr(&percpu_mct_tick);
@@ -527,10 +527,6 @@ static void __init exynos4_timer_resources(struct device_node *np, void __iomem
 		panic("%s: unable to retrieve mct clock instance\n", __func__);
 	clk_prepare_enable(mct_clk);
 
-	reg_base = base;
-	if (!reg_base)
-		panic("%s: unable to ioremap mct address space\n", __func__);
-
 	if (mct_int_type == MCT_INT_PPI) {
 
 		err = request_percpu_irq(mct_irqs[MCT_L0_IRQ],
@@ -574,8 +570,19 @@ out_irq:
 
 static void __init mct_init_dt(struct device_node *np, unsigned int int_type)
 {
+	struct device_node *child;
 	u32 nr_irqs, i;
 
+	reg_base = of_iomap(np, 0);
+	if (!reg_base)
+		panic("%s: unable to ioremap mct address space\n", __func__);
+
+	for_each_child_of_node(np, child)
+		if (of_device_is_compatible(child, "arm,armv7-timer")) {
+			exynos4_mct_frc_start();
+			return;
+		}
+
 	mct_int_type = int_type;
 
 	/* This driver uses only one global timer interrupt */
@@ -594,7 +601,7 @@ static void __init mct_init_dt(struct device_node *np, unsigned int int_type)
 	for (i = MCT_L0_IRQ; i < nr_irqs; i++)
 		mct_irqs[i] = irq_of_parse_and_map(np, i);
 
-	exynos4_timer_resources(np, of_iomap(np, 0));
+	exynos4_timer_resources(np);
 	exynos4_clocksource_init();
 	exynos4_clockevent_init();
 }
-- 
2.4.4

[PATCH 2/2] ARM: dts: Use architected timer on Exynos 5410

[Pavel Fedin]

This switches the kernel to timer accesses using CP15, allowing to use
KVM virtualization.

Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
 arch/arm/boot/dts/exynos5410.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5410.dtsi b/arch/arm/boot/dts/exynos5410.dtsi
index d8de015..c14f100 100644
--- a/arch/arm/boot/dts/exynos5410.dtsi
+++ b/arch/arm/boot/dts/exynos5410.dtsi
@@ -138,6 +138,17 @@
 						<10 &gic 0 130 0>,
 						<11 &gic 0 131 0>;
 			};
+
+			arch_timer {
+				compatible = "arm,armv7-timer";
+				interrupt-parent = <&gic>;
+				interrupts = <1 13 0xf08>,
+						<1 14 0xf08>,
+						<1 11 0xf08>,
+						<1 10 0xf08>;
+				clock-frequency = <24000000>;
+			};
+
 		};
 
 		sysram at 02020000 {
-- 
2.4.4

[PATCH 2/2] ARM: dts: Use architected timer on Exynos 5410

[Chanwoo Choi]

Hi Pavel,

On 2015? 10? 06? 17:24, Pavel Fedin wrote:
> This switches the kernel to timer accesses using CP15, allowing to use
> KVM virtualization.
> 
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> ---
>  arch/arm/boot/dts/exynos5410.dtsi | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/exynos5410.dtsi b/arch/arm/boot/dts/exynos5410.dtsi
> index d8de015..c14f100 100644
> --- a/arch/arm/boot/dts/exynos5410.dtsi
> +++ b/arch/arm/boot/dts/exynos5410.dtsi
> @@ -138,6 +138,17 @@
>  						<10 &gic 0 130 0>,
>  						<11 &gic 0 131 0>;
>  			};
> +
> +			arch_timer {
> +				compatible = "arm,armv7-timer";
> +				interrupt-parent = <&gic>;

You don't need to add 'interrupt-parent' because exynos5410.dtsi already includes
it on top.

> +				interrupts = <1 13 0xf08>,
> +						<1 14 0xf08>,
> +						<1 11 0xf08>,
> +						<1 10 0xf08>;
> +				clock-frequency = <24000000>;
> +			};
> +
>  		};
>  
>  		sysram at 02020000 {
> 

Thanks,
Chanwoo Choi

[PATCH 2/2] ARM: dts: Use architected timer on Exynos 5410

[Pavel Fedin]

 Hello!

> > +
> > +			arch_timer {
> > +				compatible = "arm,armv7-timer";
> > +				interrupt-parent = <&gic>;
> 
> You don't need to add 'interrupt-parent' because exynos5410.dtsi already includes
> it on top.

 I do, because otherwise it tries to use interrupt-map, written for MCT (which is specified as interrupt-parent for its node), and looks like interrupt-map specifies totally different things.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

[PATCH 0/2] ARM: EXYNOS: Allow to use architected timer

[Arnd Bergmann]

On Tuesday 06 October 2015 11:23:58 Pavel Fedin wrote:
> Exynos platform officially uses "multi-core timer" AKA MCT. However, it
> also has a possibility to access the same timer using CP15 (AKA
> architected timer) registers.
> 
> In order to be able to use KVM, we need CP15 timer support. In its turn,
> MCT needs to be turned on before any accesses will work. This patchset
> allows to specify "CP15 compatibility mode" for the MCT by inserting
> armv7-timer node as a child for MCT device. In this case MCT driver will
> only enable the timer, leaving the rest for architected timer driver.

IIRC the use of the CP15 timer is not reliable on this platform.
I don't have a link ready, but this has come up a few times.

Kukjin Kim probably knows the details.

Regarding KVM usage, would it be possible to emulate the architected
timer from qemu instead?

	Arnd

[PATCH 0/2] ARM: EXYNOS: Allow to use architected timer

[Alexey Klimov]

(add linux-samsung-soc mail list to cc)

On Tue, Oct 6, 2015 at 11:37 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 06 October 2015 11:23:58 Pavel Fedin wrote:
>> Exynos platform officially uses "multi-core timer" AKA MCT. However, it
>> also has a possibility to access the same timer using CP15 (AKA
>> architected timer) registers.
>>
>> In order to be able to use KVM, we need CP15 timer support. In its turn,
>> MCT needs to be turned on before any accesses will work. This patchset
>> allows to specify "CP15 compatibility mode" for the MCT by inserting
>> armv7-timer node as a child for MCT device. In this case MCT driver will
>> only enable the timer, leaving the rest for architected timer driver.
>
> IIRC the use of the CP15 timer is not reliable on this platform.
> I don't have a link ready, but this has come up a few times.
>
> Kukjin Kim probably knows the details.

Please take a look: http://www.gossamer-threads.com/lists/linux/kernel/2225900
for some details. I don't know if it's related to 5410.

Best regards,
Alexey Klimov

[PATCH 0/2] ARM: EXYNOS: Allow to use architected timer

[Pavel Fedin]

 Hello!

> IIRC the use of the CP15 timer is not reliable on this platform.
> I don't have a link ready, but this has come up a few times.

 I know that Samsung SoC guys are not very keen on this, therefore i actually made this optional. If
you want, you could omit the second patch, and by default Exynos platform will use MCT. But if
someone wants to use KVM, it would be enough only to modify device tree in order to enable CP15.
Would this be a good compromise?

> Regarding KVM usage, would it be possible to emulate the architected
> timer from qemu instead?

 Unfortunately no. Accesses to virtual timer registers cannot be trapped. It is possible to trap
physical timer accesses, and trick the kernel into using it, but it's slow, and it is a different
story, which you can read here: http://www.spinics.net/lists/kvm/msg121714.html.
 So, i think that having an option to use CP15 would be a good solution. It is known to work, and i
use this configuration here on SMDK5410 without any problems.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

[PATCH 0/2] ARM: EXYNOS: Allow to use architected timer

[Arnd Bergmann]

On Tuesday 06 October 2015 11:56:33 Pavel Fedin wrote:
>  Hello!
> 
> > IIRC the use of the CP15 timer is not reliable on this platform.
> > I don't have a link ready, but this has come up a few times.
> 
>  I know that Samsung SoC guys are not very keen on this, therefore i actually made this optional. If
> you want, you could omit the second patch, and by default Exynos platform will use MCT. But if
> someone wants to use KVM, it would be enough only to modify device tree in order to enable CP15.
> Would this be a good compromise?
> 
> > Regarding KVM usage, would it be possible to emulate the architected
> > timer from qemu instead?
> 
>  Unfortunately no. Accesses to virtual timer registers cannot be trapped. It is possible to trap
> physical timer accesses, and trick the kernel into using it, but it's slow, and it is a different
> story, which you can read here: http://www.spinics.net/lists/kvm/msg121714.html.
>  So, i think that having an option to use CP15 would be a good solution.

My feeling is that is shouldn't be keyed off the presence of the device in DT
though. Maybe we can find a way that allows you to put the device into DT
but not have it used by default unless the user explicitly enables it, e.g. though
a kernel command line option like "force_broken_archtimer"?

Or maybe we could find a way to keep using MCT in the host but use the
arch timer in the guest only? At least that would not potentially break
anything that works today.

> It is known to work, and i use this configuration here on SMDK5410 without any problems.

For all I can tell, nobody every /saw/ the problem, we just know that it
hasn't passed verification (or something like that) and you shouldn't
use it in production.

	Arnd

[PATCH 0/2] ARM: EXYNOS: Allow to use architected timer

[Pavel Fedin]

 Hello!

> My feeling is that is shouldn't be keyed off the presence of the device in DT
> though. Maybe we can find a way that allows you to put the device into DT
> but not have it used by default unless the user explicitly enables it, e.g. though
> a kernel command line option like "force_broken_archtimer"?

 For this purpose we have status = "disabled" in the device tree. We can add CP15 timer node and
disable it. If the user needs it, the bootloader could just change "disabled" to "ok", and it's
done.
 The only problem here would be that CP15 timer on Exynos is actually tied to MCT. We need to enable
MCT, otherwise clocks don't tick. That's why i decided to make CP15 timer a subnode.
 My current patch lacks "status" property check, i could add it if you agree with this approach.

> Or maybe we could find a way to keep using MCT in the host but use the
> arch timer in the guest only?

 That's an interesting question, but, perhaps, it would require more changes to the code, and KVM
maintainers don't like these things, calling them "broken non-compliant hardware". So, i guess, we
could move from simple things to more complex ones.

> For all I can tell, nobody every /saw/ the problem, we just know that it
> hasn't passed verification (or something like that) and you shouldn't
> use it in production.

 Yes, and i agreed that this should be an option, not the default. 

 P.S. I cc'ed to linux-samsung-soc, but i'm not subscribed. Will it reach there?

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

[PATCH 0/2] ARM: EXYNOS: Allow to use architected timer

[Arnd Bergmann]

On Tuesday 06 October 2015 12:44:54 Pavel Fedin wrote:
>  Hello!
> 
> > My feeling is that is shouldn't be keyed off the presence of the device in DT
> > though. Maybe we can find a way that allows you to put the device into DT
> > but not have it used by default unless the user explicitly enables it, e.g. though
> > a kernel command line option like "force_broken_archtimer"?
> 
>  For this purpose we have status = "disabled" in the device tree. We can add CP15 timer node and
> disable it. If the user needs it, the bootloader could just change "disabled" to "ok", and it's
> done.
>  The only problem here would be that CP15 timer on Exynos is actually tied to MCT. We need to enable
> MCT, otherwise clocks don't tick. That's why i decided to make CP15 timer a subnode.
>  My current patch lacks "status" property check, i could add it if you agree with this approach.

It's not ideal, because the hardware is always there and always slightly
broken, so it's not really a property of the platform. I'd be happier with
something that is easier for users to change if they know what they are
doing and also makes it clear that it is slightly risky.

> > Or maybe we could find a way to keep using MCT in the host but use the
> > arch timer in the guest only?
> 
>  That's an interesting question, but, perhaps, it would require more changes to the code, and KVM
> maintainers don't like these things, calling them "broken non-compliant hardware". So, i guess, we
> could move from simple things to more complex ones.

It's probably worth trying to see how much complexity you need for that
approach, maybe it's not as hard in the end.

> > For all I can tell, nobody every /saw/ the problem, we just know that it
> > hasn't passed verification (or something like that) and you shouldn't
> > use it in production.
> 
>  Yes, and i agreed that this should be an option, not the default. 
> 
>  P.S. I cc'ed to linux-samsung-soc, but i'm not subscribed. Will it reach there?

I think it should, at least I didn't get an email back for my reply,
and I'm not subscribed either.

	Arnd

[PATCH 0/2] ARM: EXYNOS: Allow to use architected timer

[Kukjin Kim]

Pavel Fedin wrote:
> 
>  Hello!
> 
> > IIRC the use of the CP15 timer is not reliable on this platform.
> > I don't have a link ready, but this has come up a few times.
> 
>  I know that Samsung SoC guys are not very keen on this, therefore i actually made this optional.

No, it's not a good way and it's not an optional problem...I think, I already
commented many times about that...If we can arch timer on exynos stuff, why we
are still using MCT...? hmm...

> If
> you want, you could omit the second patch, and by default Exynos platform will use MCT. But if
> someone wants to use KVM, it would be enough only to modify device tree in order to enable CP15.
> Would this be a good compromise?
> 
No, because the MCT should be used by default _always_ and it is depending on hardware scheme...

> > Regarding KVM usage, would it be possible to emulate the architected
> > timer from qemu instead?
> 
>  Unfortunately no. Accesses to virtual timer registers cannot be trapped. It is possible to trap
> physical timer accesses, and trick the kernel into using it, but it's slow, and it is a different
> story, which you can read here: http://www.spinics.net/lists/kvm/msg121714.html.
>  So, i think that having an option to use CP15 would be a good solution. It is known to work, and i
> use this configuration here on SMDK5410 without any problems.
> 
Actually Exynos5410 has been designed differently so it is safe only on smdk5410 not others...

- Kukjin

[PATCH 0/2] ARM: EXYNOS: Allow to use architected timer

[Pavel Fedin]

 Hello!

> No, because the MCT should be used by default _always_ and it is depending on hardware
> scheme...

 You just said: "by default". My proposal follows this. What's wrong then?
 Or don't you just want users to be able to use KVM on Exynos?

> >  So, i think that having an option to use CP15 would be a good solution. It is known to work,
> and i
> > use this configuration here on SMDK5410 without any problems.
> >
> Actually Exynos5410 has been designed differently so it is safe only on smdk5410 not others...

 Odroid-XU users also report it working. So, i guess, it's Exynos-5410's property. Well, does not
matter after all...

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

[PATCH 0/2] ARM: EXYNOS: Allow to use architected timer

[Kukjin Kim]

Kukjin Kim wrote:
> 
> Pavel Fedin wrote:
> >
> >  Hello!
> >
> > > IIRC the use of the CP15 timer is not reliable on this platform.
> > > I don't have a link ready, but this has come up a few times.
> >
> >  I know that Samsung SoC guys are not very keen on this, therefore i actually made this optional.
> 
> No, it's not a good way and it's not an optional problem...I think, I already
> commented many times about that...If we can arch timer on exynos stuff, why we
> are still using MCT...? hmm...
> 
> > If
> > you want, you could omit the second patch, and by default Exynos platform will use MCT. But if
> > someone wants to use KVM, it would be enough only to modify device tree in order to enable CP15.
> > Would this be a good compromise?
> >
> No, because the MCT should be used by default _always_ and it is depending on hardware scheme...
> 
> > > Regarding KVM usage, would it be possible to emulate the architected
> > > timer from qemu instead?
> >
> >  Unfortunately no. Accesses to virtual timer registers cannot be trapped. It is possible to trap
> > physical timer accesses, and trick the kernel into using it, but it's slow, and it is a different
> > story, which you can read here: http://www.spinics.net/lists/kvm/msg121714.html.
> >  So, i think that having an option to use CP15 would be a good solution. It is known to work, and i
> > use this configuration here on SMDK5410 without any problems.
> >
> Actually Exynos5410 has been designed differently so it is safe only on smdk5410 not others...
> 
Oops, it should be 5440 not 5410. Could be "you see it looks work well", but it
is not guaranteed...unfortunately...

> - Kukjin