diff mbox

[3/3] ARM: EXYNOS5250: Register architected timers

Message ID 1363222742-15220-4-git-send-email-agraf@suse.de (mailing list archive)
State New, archived
Headers show

Commit Message

Alexander Graf March 14, 2013, 12:59 a.m. UTC
When running on an exynos 5250 SoC, we don't initialize the architected
timers. The chip however supports architected timers.

When we don't initialize them, KVM will try to access them and run into
NULL pointer dereferences attempting to do so.

This patch is really more of a hack than a real fix, but does get me
working with KVM on Arndale.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/arm/mach-exynos/mct.c |    4 ++++
 1 file changed, 4 insertions(+)

Comments

Christoffer Dall March 14, 2013, 3:26 a.m. UTC | #1
On Wed, Mar 13, 2013 at 5:59 PM, Alexander Graf <agraf@suse.de> wrote:
> When running on an exynos 5250 SoC, we don't initialize the architected
> timers. The chip however supports architected timers.
>
> When we don't initialize them, KVM will try to access them and run into
> NULL pointer dereferences attempting to do so.
>
> This patch is really more of a hack than a real fix, but does get me
> working with KVM on Arndale.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/arm/mach-exynos/mct.c |    4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm/mach-exynos/mct.c b/arch/arm/mach-exynos/mct.c
> index c9d6650..eefb8af 100644
> --- a/arch/arm/mach-exynos/mct.c
> +++ b/arch/arm/mach-exynos/mct.c
> @@ -482,4 +482,8 @@ void __init exynos4_timer_init(void)
>         exynos4_timer_resources();
>         exynos4_clocksource_init();
>         exynos4_clockevent_init();
> +
> +       if (soc_is_exynos5250()) {
> +               arch_timer_of_register();
> +       }
>  }

I did something similar a while back:
https://github.com/columbia/linux-kvm-arm/commit/2a368f711893e8fb5fe5cf9e237a7631277f3fd1

But I'm not sure how exactly is the right way for exynos. The other
patches look good to me (you can find some very similar work around
that very commit in my tree).

-Christoffer
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov March 14, 2013, 7:07 p.m. UTC | #2
Hello.

On 14-03-2013 4:59, Alexander Graf wrote:

> When running on an exynos 5250 SoC, we don't initialize the architected
> timers. The chip however supports architected timers.

> When we don't initialize them, KVM will try to access them and run into
> NULL pointer dereferences attempting to do so.

> This patch is really more of a hack than a real fix, but does get me
> working with KVM on Arndale.

> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>   arch/arm/mach-exynos/mct.c |    4 ++++
>   1 file changed, 4 insertions(+)

> diff --git a/arch/arm/mach-exynos/mct.c b/arch/arm/mach-exynos/mct.c
> index c9d6650..eefb8af 100644
> --- a/arch/arm/mach-exynos/mct.c
> +++ b/arch/arm/mach-exynos/mct.c
> @@ -482,4 +482,8 @@ void __init exynos4_timer_init(void)
>   	exynos4_timer_resources();
>   	exynos4_clocksource_init();
>   	exynos4_clockevent_init();
> +
> +	if (soc_is_exynos5250()) {
> +		arch_timer_of_register();
> +	}

    {} not needed here. scripts/checkpatch.pl should probabl;y warn about it.

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Graf March 21, 2013, 3:40 p.m. UTC | #3
On 14.03.2013, at 20:07, Sergei Shtylyov wrote:

> Hello.
> 
> On 14-03-2013 4:59, Alexander Graf wrote:
> 
>> When running on an exynos 5250 SoC, we don't initialize the architected
>> timers. The chip however supports architected timers.
> 
>> When we don't initialize them, KVM will try to access them and run into
>> NULL pointer dereferences attempting to do so.
> 
>> This patch is really more of a hack than a real fix, but does get me
>> working with KVM on Arndale.
> 
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>>  arch/arm/mach-exynos/mct.c |    4 ++++
>>  1 file changed, 4 insertions(+)
> 
>> diff --git a/arch/arm/mach-exynos/mct.c b/arch/arm/mach-exynos/mct.c
>> index c9d6650..eefb8af 100644
>> --- a/arch/arm/mach-exynos/mct.c
>> +++ b/arch/arm/mach-exynos/mct.c
>> @@ -482,4 +482,8 @@ void __init exynos4_timer_init(void)
>>  	exynos4_timer_resources();
>>  	exynos4_clocksource_init();
>>  	exynos4_clockevent_init();
>> +
>> +	if (soc_is_exynos5250()) {
>> +		arch_timer_of_register();
>> +	}
> 
>   {} not needed here. scripts/checkpatch.pl should probabl;y warn about it.

Yeah, I'd leave it to whoever wants to apply this patch to remove the braces :). IMHO it's not worth it to respin just for this.


Alex

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kim Kukjin April 2, 2013, 10:44 a.m. UTC | #4
Alexander Graf wrote:
> 
> When running on an exynos 5250 SoC, we don't initialize the architected
> timers. The chip however supports architected timers.
> 
Yes, exynos5250 can support, mct(multi core timer) is used though.

> When we don't initialize them, KVM will try to access them and run into
> NULL pointer dereferences attempting to do so.
> 
Yes, right.

> This patch is really more of a hack than a real fix, but does get me
> working with KVM on Arndale.
> 
Hmm, if you think, this is _really_ a hack, you need to add some comments
about that for clearance, and since the mct.c file has been moved into
drivers/clocksource/, this should be re-worked.

BTW, I discussed about this with Thomas and Giridhar just now, we reached
this 3rd patch could be dropped because the correct way is to add a dts
node for arch timer which patch 2nd is already doing after 3.9-rc1 because
of CLOCKSOURCE_OF_DECLARE macro.

So if you' OK above, let me know so that I can take only 1st and 2nd
patches to support KVM on exynos5250.

Thanks.

- Kukjin

> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/arm/mach-exynos/mct.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/mach-exynos/mct.c b/arch/arm/mach-exynos/mct.c
> index c9d6650..eefb8af 100644
> --- a/arch/arm/mach-exynos/mct.c
> +++ b/arch/arm/mach-exynos/mct.c
> @@ -482,4 +482,8 @@ void __init exynos4_timer_init(void)
>  	exynos4_timer_resources();
>  	exynos4_clocksource_init();
>  	exynos4_clockevent_init();
> +
> +	if (soc_is_exynos5250()) {
> +		arch_timer_of_register();
> +	}
>  }
> --
> 1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Graf April 2, 2013, 11:45 a.m. UTC | #5
On 04/02/2013 12:44 PM, Kukjin Kim wrote:
> Alexander Graf wrote:
>> When running on an exynos 5250 SoC, we don't initialize the architected
>> timers. The chip however supports architected timers.
>>
> Yes, exynos5250 can support, mct(multi core timer) is used though.
>
>> When we don't initialize them, KVM will try to access them and run into
>> NULL pointer dereferences attempting to do so.
>>
> Yes, right.
>
>> This patch is really more of a hack than a real fix, but does get me
>> working with KVM on Arndale.
>>
> Hmm, if you think, this is _really_ a hack, you need to add some comments
> about that for clearance, and since the mct.c file has been moved into
> drivers/clocksource/, this should be re-worked.
>
> BTW, I discussed about this with Thomas and Giridhar just now, we reached
> this 3rd patch could be dropped because the correct way is to add a dts
> node for arch timer which patch 2nd is already doing after 3.9-rc1 because
> of CLOCKSOURCE_OF_DECLARE macro.
>
> So if you' OK above, let me know so that I can take only 1st and 2nd
> patches to support KVM on exynos5250.

I'd say go ahead and take them and I'll verify whether things work on
your tree :).

What's the git repo of your branch?


Alex

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kim Kukjin April 3, 2013, 4:56 a.m. UTC | #6
Alexander Graf wrote:
> 
> On 04/02/2013 12:44 PM, Kukjin Kim wrote:
> > Alexander Graf wrote:
> >> When running on an exynos 5250 SoC, we don't initialize the architected
> >> timers. The chip however supports architected timers.
> >>
> > Yes, exynos5250 can support, mct(multi core timer) is used though.
> >
> >> When we don't initialize them, KVM will try to access them and run into
> >> NULL pointer dereferences attempting to do so.
> >>
> > Yes, right.
> >
> >> This patch is really more of a hack than a real fix, but does get me
> >> working with KVM on Arndale.
> >>
> > Hmm, if you think, this is _really_ a hack, you need to add some
> comments
> > about that for clearance, and since the mct.c file has been moved into
> > drivers/clocksource/, this should be re-worked.
> >
> > BTW, I discussed about this with Thomas and Giridhar just now, we
> reached
> > this 3rd patch could be dropped because the correct way is to add a dts
> > node for arch timer which patch 2nd is already doing after 3.9-rc1
> because
> > of CLOCKSOURCE_OF_DECLARE macro.
> >
> > So if you' OK above, let me know so that I can take only 1st and 2nd
> > patches to support KVM on exynos5250.
> 
> I'd say go ahead and take them and I'll verify whether things work on
> your tree :).
> 
OK, I will.

> What's the git repo of your branch?
> 
You can test with my for-next branch but this series can be seen tomorrow
night(KST) in my public tree.

Any problems, please let me know.

Thanks.

- Kukjin

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/mach-exynos/mct.c b/arch/arm/mach-exynos/mct.c
index c9d6650..eefb8af 100644
--- a/arch/arm/mach-exynos/mct.c
+++ b/arch/arm/mach-exynos/mct.c
@@ -482,4 +482,8 @@  void __init exynos4_timer_init(void)
 	exynos4_timer_resources();
 	exynos4_clocksource_init();
 	exynos4_clockevent_init();
+
+	if (soc_is_exynos5250()) {
+		arch_timer_of_register();
+	}
 }