* [PATCH] clocksource: arch_timer: Fix arm64 platforms not booting
@ 2014-12-28 14:20 Christoffer Dall
2014-12-28 21:46 ` Marc Zyngier
0 siblings, 1 reply; 6+ messages in thread
From: Christoffer Dall @ 2014-12-28 14:20 UTC (permalink / raw)
To: linux-arm-kernel
Commit 0b46b8a718c6e ("clocksource: arch_timer: Fix code to...") fixes
timer issues with certain ARMv7 platforms, but unfortunately breaks
arm64 platforms with hyp mode (EL2) enabled.
The commit only sets arch_timer_use_virtual to false under CONFIG_ARM,
but forgets that the config variable is also set in other code paths
(actually, right underneath the check in the patch) with detrimental
consequences as we've now introduced a direct early call to BUG() on
practically all arm64 platforms.
One could argue that this code could be refactored to use different
variables for checking which *timer* to use and which *counter* to use,
which seems to be the desired difference between ARM and arm64 in this
case, but this approach fixes an urgent issue for now.
Cc: Sonny Rao <sonnyrao@chromium.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Olof Johansson <olof@lixom.net>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
This was apparently already discovered by Yingjoe Chen in this thread
https://lkml.org/lkml/2014/11/24/41 and Catalin recommended a similar
fix.
drivers/clocksource/arm_arch_timer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 6a79fc4..095c177 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -462,7 +462,7 @@ static void __init arch_counter_register(unsigned type)
/* Register the CP15 based counter if we have one */
if (type & ARCH_CP15_TIMER) {
- if (arch_timer_use_virtual)
+ if (IS_ENABLED(CONFIG_ARM64) || arch_timer_use_virtual)
arch_timer_read_counter = arch_counter_get_cntvct;
else
arch_timer_read_counter = arch_counter_get_cntpct;
--
2.1.2.330.g565301e.dirty
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] clocksource: arch_timer: Fix arm64 platforms not booting
2014-12-28 14:20 [PATCH] clocksource: arch_timer: Fix arm64 platforms not booting Christoffer Dall
@ 2014-12-28 21:46 ` Marc Zyngier
2014-12-29 8:35 ` Christoffer Dall
0 siblings, 1 reply; 6+ messages in thread
From: Marc Zyngier @ 2014-12-28 21:46 UTC (permalink / raw)
To: linux-arm-kernel
On 2014-12-28 14:20, Christoffer Dall wrote:
> Commit 0b46b8a718c6e ("clocksource: arch_timer: Fix code to...")
> fixes
> timer issues with certain ARMv7 platforms, but unfortunately breaks
> arm64 platforms with hyp mode (EL2) enabled.
>
> The commit only sets arch_timer_use_virtual to false under
> CONFIG_ARM,
> but forgets that the config variable is also set in other code paths
> (actually, right underneath the check in the patch) with detrimental
> consequences as we've now introduced a direct early call to BUG() on
> practically all arm64 platforms.
>
> One could argue that this code could be refactored to use different
> variables for checking which *timer* to use and which *counter* to
> use,
> which seems to be the desired difference between ARM and arm64 in
> this
> case, but this approach fixes an urgent issue for now.
>
> Cc: Sonny Rao <sonnyrao@chromium.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Olof Johansson <olof@lixom.net>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> ---
> This was apparently already discovered by Yingjoe Chen in this thread
> https://lkml.org/lkml/2014/11/24/41 and Catalin recommended a similar
> fix.
I'm increasingly worried about the time it takes to get such critical
fixes into the tree (arm64 is *dead* without it).
What is holding this patch which, as far as I remember, has been posted
by Catalin almost three weeks ago?
Daniel?
M.
--
Fast, cheap, reliable. Pick two.
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] clocksource: arch_timer: Fix arm64 platforms not booting
2014-12-28 21:46 ` Marc Zyngier
@ 2014-12-29 8:35 ` Christoffer Dall
2014-12-29 19:13 ` Mark Rutland
0 siblings, 1 reply; 6+ messages in thread
From: Christoffer Dall @ 2014-12-29 8:35 UTC (permalink / raw)
To: linux-arm-kernel
On Sun, Dec 28, 2014 at 09:46:20PM +0000, Marc Zyngier wrote:
> On 2014-12-28 14:20, Christoffer Dall wrote:
> >Commit 0b46b8a718c6e ("clocksource: arch_timer: Fix code to...")
> >fixes
> >timer issues with certain ARMv7 platforms, but unfortunately breaks
> >arm64 platforms with hyp mode (EL2) enabled.
> >
> >The commit only sets arch_timer_use_virtual to false under
> >CONFIG_ARM,
> >but forgets that the config variable is also set in other code paths
> >(actually, right underneath the check in the patch) with detrimental
> >consequences as we've now introduced a direct early call to BUG() on
> >practically all arm64 platforms.
> >
> >One could argue that this code could be refactored to use different
> >variables for checking which *timer* to use and which *counter* to
> >use,
> >which seems to be the desired difference between ARM and arm64 in
> >this
> >case, but this approach fixes an urgent issue for now.
> >
> >Cc: Sonny Rao <sonnyrao@chromium.org>
> >Cc: Catalin Marinas <catalin.marinas@arm.com>
> >Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> >Cc: Olof Johansson <olof@lixom.net>
> >Cc: Mark Rutland <mark.rutland@arm.com>
> >Cc: Catalin Marinas <catalin.marinas@arm.com>
> >Cc: Marc Zyngier <marc.zyngier@arm.com>
> >Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> >Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> >---
> >This was apparently already discovered by Yingjoe Chen in this thread
> >https://lkml.org/lkml/2014/11/24/41 and Catalin recommended a similar
> >fix.
>
> I'm increasingly worried about the time it takes to get such critical
> fixes into the tree (arm64 is *dead* without it).
>
> What is holding this patch which, as far as I remember, has been posted
> by Catalin almost three weeks ago?
>
I didn't find that since I didn't think I'd have to go back that long on
lakml for something that breaks boot of an entire architecture. Sorry
for the confusion of a second patch, but fwiw, I now spent another few
hours bisecting this, so I would really like to see this fix go into
mainline ASAP as well to save others the trouble.
Thanks,
-Christoffer
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] clocksource: arch_timer: Fix arm64 platforms not booting
2014-12-29 8:35 ` Christoffer Dall
@ 2014-12-29 19:13 ` Mark Rutland
2014-12-29 20:54 ` Daniel Lezcano
0 siblings, 1 reply; 6+ messages in thread
From: Mark Rutland @ 2014-12-29 19:13 UTC (permalink / raw)
To: linux-arm-kernel
[Adding Arnd]
On Mon, Dec 29, 2014 at 08:35:09AM +0000, Christoffer Dall wrote:
> On Sun, Dec 28, 2014 at 09:46:20PM +0000, Marc Zyngier wrote:
> > On 2014-12-28 14:20, Christoffer Dall wrote:
> > >Commit 0b46b8a718c6e ("clocksource: arch_timer: Fix code to...")
> > >fixes
> > >timer issues with certain ARMv7 platforms, but unfortunately breaks
> > >arm64 platforms with hyp mode (EL2) enabled.
> > >
> > >The commit only sets arch_timer_use_virtual to false under
> > >CONFIG_ARM,
> > >but forgets that the config variable is also set in other code paths
> > >(actually, right underneath the check in the patch) with detrimental
> > >consequences as we've now introduced a direct early call to BUG() on
> > >practically all arm64 platforms.
> > >
> > >One could argue that this code could be refactored to use different
> > >variables for checking which *timer* to use and which *counter* to
> > >use,
> > >which seems to be the desired difference between ARM and arm64 in
> > >this
> > >case, but this approach fixes an urgent issue for now.
> > >
> > >Cc: Sonny Rao <sonnyrao@chromium.org>
> > >Cc: Catalin Marinas <catalin.marinas@arm.com>
> > >Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> > >Cc: Olof Johansson <olof@lixom.net>
> > >Cc: Mark Rutland <mark.rutland@arm.com>
> > >Cc: Catalin Marinas <catalin.marinas@arm.com>
> > >Cc: Marc Zyngier <marc.zyngier@arm.com>
> > >Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> > >Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> > >---
> > >This was apparently already discovered by Yingjoe Chen in this thread
> > >https://lkml.org/lkml/2014/11/24/41 and Catalin recommended a similar
> > >fix.
> >
> > I'm increasingly worried about the time it takes to get such critical
> > fixes into the tree (arm64 is *dead* without it).
> >
> > What is holding this patch which, as far as I remember, has been posted
> > by Catalin almost three weeks ago?
> >
> I didn't find that since I didn't think I'd have to go back that long on
> lakml for something that breaks boot of an entire architecture. Sorry
> for the confusion of a second patch, but fwiw, I now spent another few
> hours bisecting this, so I would really like to see this fix go into
> mainline ASAP as well to save others the trouble.
Last I knew, Arnd was going to take the fix [1], which has been in
arm-soc's for-next and fixes branches for almost two weeks now. It
didn't make it into the last pull req due to some confusion over who was
going to take it.
Arnd, what's the plan for getting this into mainline ASAP?
Mark.
[1] https://git.kernel.org/cgit/linux/kernel/git/arm/arm-soc.git/commit/?h=fixes&id=d6ad36913083d683aad4e02e53580c995f1a6ede
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] clocksource: arch_timer: Fix arm64 platforms not booting
2014-12-29 19:13 ` Mark Rutland
@ 2014-12-29 20:54 ` Daniel Lezcano
2014-12-30 15:59 ` Arnd Bergmann
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Lezcano @ 2014-12-29 20:54 UTC (permalink / raw)
To: linux-arm-kernel
On 12/29/2014 08:13 PM, Mark Rutland wrote:
> [Adding Arnd]
>
> On Mon, Dec 29, 2014 at 08:35:09AM +0000, Christoffer Dall wrote:
>> On Sun, Dec 28, 2014 at 09:46:20PM +0000, Marc Zyngier wrote:
>>> On 2014-12-28 14:20, Christoffer Dall wrote:
>>>> Commit 0b46b8a718c6e ("clocksource: arch_timer: Fix code to...")
>>>> fixes
>>>> timer issues with certain ARMv7 platforms, but unfortunately breaks
>>>> arm64 platforms with hyp mode (EL2) enabled.
>>>>
>>>> The commit only sets arch_timer_use_virtual to false under
>>>> CONFIG_ARM,
>>>> but forgets that the config variable is also set in other code paths
>>>> (actually, right underneath the check in the patch) with detrimental
>>>> consequences as we've now introduced a direct early call to BUG() on
>>>> practically all arm64 platforms.
>>>>
>>>> One could argue that this code could be refactored to use different
>>>> variables for checking which *timer* to use and which *counter* to
>>>> use,
>>>> which seems to be the desired difference between ARM and arm64 in
>>>> this
>>>> case, but this approach fixes an urgent issue for now.
>>>>
>>>> Cc: Sonny Rao <sonnyrao@chromium.org>
>>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>>>> Cc: Olof Johansson <olof@lixom.net>
>>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>>> Cc: Marc Zyngier <marc.zyngier@arm.com>
>>>> Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
>>>> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
>>>> ---
>>>> This was apparently already discovered by Yingjoe Chen in this thread
>>>> https://lkml.org/lkml/2014/11/24/41 and Catalin recommended a similar
>>>> fix.
>>>
>>> I'm increasingly worried about the time it takes to get such critical
>>> fixes into the tree (arm64 is *dead* without it).
>>>
>>> What is holding this patch which, as far as I remember, has been posted
>>> by Catalin almost three weeks ago?
>>>
>> I didn't find that since I didn't think I'd have to go back that long on
>> lakml for something that breaks boot of an entire architecture. Sorry
>> for the confusion of a second patch, but fwiw, I now spent another few
>> hours bisecting this, so I would really like to see this fix go into
>> mainline ASAP as well to save others the trouble.
>
> Last I knew, Arnd was going to take the fix [1], which has been in
> arm-soc's for-next and fixes branches for almost two weeks now. It
> didn't make it into the last pull req due to some confusion over who was
> going to take it.
I asked arm-soc team to take the fix because it was depending on a patch
which was already in their tree [1].
Merry Christmas
-- Daniel
[1] http://www.spinics.net/lists/arm-kernel/msg385704.html
> Arnd, what's the plan for getting this into mainline ASAP?
>
> Mark.
>
> [1] https://git.kernel.org/cgit/linux/kernel/git/arm/arm-soc.git/commit/?h=fixes&id=d6ad36913083d683aad4e02e53580c995f1a6ede
>
--
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] clocksource: arch_timer: Fix arm64 platforms not booting
2014-12-29 20:54 ` Daniel Lezcano
@ 2014-12-30 15:59 ` Arnd Bergmann
0 siblings, 0 replies; 6+ messages in thread
From: Arnd Bergmann @ 2014-12-30 15:59 UTC (permalink / raw)
To: linux-arm-kernel
On Monday 29 December 2014 21:54:33 Daniel Lezcano wrote:
> On 12/29/2014 08:13 PM, Mark Rutland wrote:
> > On Mon, Dec 29, 2014 at 08:35:09AM +0000, Christoffer Dall wrote:
> >> On Sun, Dec 28, 2014 at 09:46:20PM +0000, Marc Zyngier wrote:
> >>> What is holding this patch which, as far as I remember, has been posted
> >>> by Catalin almost three weeks ago?
> >>>
> >> I didn't find that since I didn't think I'd have to go back that long on
> >> lakml for something that breaks boot of an entire architecture. Sorry
> >> for the confusion of a second patch, but fwiw, I now spent another few
> >> hours bisecting this, so I would really like to see this fix go into
> >> mainline ASAP as well to save others the trouble.
> >
> > Last I knew, Arnd was going to take the fix [1], which has been in
> > arm-soc's for-next and fixes branches for almost two weeks now. It
> > didn't make it into the last pull req due to some confusion over who was
> > going to take it.
>
> I asked arm-soc team to take the fix because it was depending on a patch
> which was already in their tree [1].
Sorry for dropping the ball on this, I've send out a pull request to Linus
now.
Arnd
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-12-30 15:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-28 14:20 [PATCH] clocksource: arch_timer: Fix arm64 platforms not booting Christoffer Dall
2014-12-28 21:46 ` Marc Zyngier
2014-12-29 8:35 ` Christoffer Dall
2014-12-29 19:13 ` Mark Rutland
2014-12-29 20:54 ` Daniel Lezcano
2014-12-30 15:59 ` Arnd Bergmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).