* Re: [PATCH] ARM: virt: Relax arch timer version check during early boot [not found] <1579097798-106243-1-git-send-email-vladimir.murzin@arm.com> @ 2020-01-20 10:46 ` Vladimir Murzin 2020-01-20 11:14 ` Marc Zyngier 0 siblings, 1 reply; 4+ messages in thread From: Vladimir Murzin @ 2020-01-20 10:46 UTC (permalink / raw) To: linux-arm-kernel; +Cc: maz, kvmarm + Marc + kvmarm@lists.cs.columbia.edu On 1/15/20 2:16 PM, Vladimir Murzin wrote: > Updates to the Generic Timer architecture allow ID_PFR1.GenTimer to > have values other than 0 or 1. At the moment, Linux is quite strict in > the way it handles this field at early boot and will not configure > arch timer if it doesn't find the value 1. > > Since here use ubfx for arch timer version extraction (hyb-stub build > with -march=armv7-a, so it is safe) > > To help backports (even though the code was correct at the time of writing) > Fixes: 8ec58be9f3ff ("ARM: virt: arch_timers: enable access to physical timers") > Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> > --- > arch/arm/kernel/hyp-stub.S | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/kernel/hyp-stub.S b/arch/arm/kernel/hyp-stub.S > index ae50203..6607fa8 100644 > --- a/arch/arm/kernel/hyp-stub.S > +++ b/arch/arm/kernel/hyp-stub.S > @@ -146,10 +146,9 @@ ARM_BE8(orr r7, r7, #(1 << 25)) @ HSCTLR.EE > #if !defined(ZIMAGE) && defined(CONFIG_ARM_ARCH_TIMER) > @ make CNTP_* and CNTPCT accessible from PL1 > mrc p15, 0, r7, c0, c1, 1 @ ID_PFR1 > - lsr r7, #16 > - and r7, #0xf > - cmp r7, #1 > - bne 1f > + ubfx r7, r7, #16, #4 > + teq r7, #0 > + beq 1f > mrc p15, 4, r7, c14, c1, 0 @ CNTHCTL > orr r7, r7, #3 @ PL1PCEN | PL1PCTEN > mcr p15, 4, r7, c14, c1, 0 @ CNTHCTL > _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ARM: virt: Relax arch timer version check during early boot 2020-01-20 10:46 ` [PATCH] ARM: virt: Relax arch timer version check during early boot Vladimir Murzin @ 2020-01-20 11:14 ` Marc Zyngier 2020-01-20 11:34 ` Vladimir Murzin 0 siblings, 1 reply; 4+ messages in thread From: Marc Zyngier @ 2020-01-20 11:14 UTC (permalink / raw) To: Vladimir Murzin; +Cc: kvmarm, linux-arm-kernel Hi Vladimir, On 2020-01-20 11:46, Vladimir Murzin wrote: > + Marc > + kvmarm@lists.cs.columbia.edu > > On 1/15/20 2:16 PM, Vladimir Murzin wrote: >> Updates to the Generic Timer architecture allow ID_PFR1.GenTimer to >> have values other than 0 or 1. At the moment, Linux is quite strict in >> the way it handles this field at early boot and will not configure >> arch timer if it doesn't find the value 1. >> >> Since here use ubfx for arch timer version extraction (hyb-stub build >> with -march=armv7-a, so it is safe) >> >> To help backports (even though the code was correct at the time of >> writing) >> Fixes: 8ec58be9f3ff ("ARM: virt: arch_timers: enable access to >> physical timers") >> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> I'm not opposed to such a change, but it'd be good to document what other values are expected here, as the current (Rev E_a) ARM ARM only mentions values 0 and 1. Thanks, M. >> --- >> arch/arm/kernel/hyp-stub.S | 7 +++---- >> 1 file changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/arch/arm/kernel/hyp-stub.S b/arch/arm/kernel/hyp-stub.S >> index ae50203..6607fa8 100644 >> --- a/arch/arm/kernel/hyp-stub.S >> +++ b/arch/arm/kernel/hyp-stub.S >> @@ -146,10 +146,9 @@ ARM_BE8(orr r7, r7, #(1 << 25)) @ HSCTLR.EE >> #if !defined(ZIMAGE) && defined(CONFIG_ARM_ARCH_TIMER) >> @ make CNTP_* and CNTPCT accessible from PL1 >> mrc p15, 0, r7, c0, c1, 1 @ ID_PFR1 >> - lsr r7, #16 >> - and r7, #0xf >> - cmp r7, #1 >> - bne 1f >> + ubfx r7, r7, #16, #4 >> + teq r7, #0 >> + beq 1f >> mrc p15, 4, r7, c14, c1, 0 @ CNTHCTL >> orr r7, r7, #3 @ PL1PCEN | PL1PCTEN >> mcr p15, 4, r7, c14, c1, 0 @ CNTHCTL >> -- Jazz is not dead. It just smells funny... _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ARM: virt: Relax arch timer version check during early boot 2020-01-20 11:14 ` Marc Zyngier @ 2020-01-20 11:34 ` Vladimir Murzin 2020-01-20 11:56 ` Marc Zyngier 0 siblings, 1 reply; 4+ messages in thread From: Vladimir Murzin @ 2020-01-20 11:34 UTC (permalink / raw) To: Marc Zyngier; +Cc: kvmarm, linux-arm-kernel Hi Marc, On 1/20/20 11:14 AM, Marc Zyngier wrote: > Hi Vladimir, > > On 2020-01-20 11:46, Vladimir Murzin wrote: >> + Marc >> + kvmarm@lists.cs.columbia.edu >> >> On 1/15/20 2:16 PM, Vladimir Murzin wrote: >>> Updates to the Generic Timer architecture allow ID_PFR1.GenTimer to >>> have values other than 0 or 1. At the moment, Linux is quite strict in >>> the way it handles this field at early boot and will not configure >>> arch timer if it doesn't find the value 1. >>> >>> Since here use ubfx for arch timer version extraction (hyb-stub build >>> with -march=armv7-a, so it is safe) >>> >>> To help backports (even though the code was correct at the time of writing) >>> Fixes: 8ec58be9f3ff ("ARM: virt: arch_timers: enable access to physical timers") >>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> > > I'm not opposed to such a change, but it'd be good to document what other values > are expected here, as the current (Rev E_a) ARM ARM only mentions values 0 and 1. That true, ARM ARM doesn't mention it yet. OTOH, should we really care about exact values as soon it stays compatible? Cheers Vladimir > > Thanks, > > M. > >>> --- >>> arch/arm/kernel/hyp-stub.S | 7 +++---- >>> 1 file changed, 3 insertions(+), 4 deletions(-) >>> >>> diff --git a/arch/arm/kernel/hyp-stub.S b/arch/arm/kernel/hyp-stub.S >>> index ae50203..6607fa8 100644 >>> --- a/arch/arm/kernel/hyp-stub.S >>> +++ b/arch/arm/kernel/hyp-stub.S >>> @@ -146,10 +146,9 @@ ARM_BE8(orr r7, r7, #(1 << 25)) @ HSCTLR.EE >>> #if !defined(ZIMAGE) && defined(CONFIG_ARM_ARCH_TIMER) >>> @ make CNTP_* and CNTPCT accessible from PL1 >>> mrc p15, 0, r7, c0, c1, 1 @ ID_PFR1 >>> - lsr r7, #16 >>> - and r7, #0xf >>> - cmp r7, #1 >>> - bne 1f >>> + ubfx r7, r7, #16, #4 >>> + teq r7, #0 >>> + beq 1f >>> mrc p15, 4, r7, c14, c1, 0 @ CNTHCTL >>> orr r7, r7, #3 @ PL1PCEN | PL1PCTEN >>> mcr p15, 4, r7, c14, c1, 0 @ CNTHCTL >>> > _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ARM: virt: Relax arch timer version check during early boot 2020-01-20 11:34 ` Vladimir Murzin @ 2020-01-20 11:56 ` Marc Zyngier 0 siblings, 0 replies; 4+ messages in thread From: Marc Zyngier @ 2020-01-20 11:56 UTC (permalink / raw) To: Vladimir Murzin; +Cc: kvmarm, linux-arm-kernel On 2020-01-20 12:34, Vladimir Murzin wrote: > Hi Marc, > > On 1/20/20 11:14 AM, Marc Zyngier wrote: >> Hi Vladimir, >> >> On 2020-01-20 11:46, Vladimir Murzin wrote: >>> + Marc >>> + kvmarm@lists.cs.columbia.edu >>> >>> On 1/15/20 2:16 PM, Vladimir Murzin wrote: >>>> Updates to the Generic Timer architecture allow ID_PFR1.GenTimer to >>>> have values other than 0 or 1. At the moment, Linux is quite strict >>>> in >>>> the way it handles this field at early boot and will not configure >>>> arch timer if it doesn't find the value 1. >>>> >>>> Since here use ubfx for arch timer version extraction (hyb-stub >>>> build >>>> with -march=armv7-a, so it is safe) >>>> >>>> To help backports (even though the code was correct at the time of >>>> writing) >>>> Fixes: 8ec58be9f3ff ("ARM: virt: arch_timers: enable access to >>>> physical timers") >>>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> >> >> I'm not opposed to such a change, but it'd be good to document what >> other values >> are expected here, as the current (Rev E_a) ARM ARM only mentions >> values 0 and 1. > > That true, ARM ARM doesn't mention it yet. OTOH, should we really care > about exact values as soon it stays compatible? That's for you to say, really. But given that you hint at some changes, it'd be good to have at least a short sentence explaining that, for example, "upcoming revisions of the architecture will allow different ID_PFR1.GenTimer values while preserving backward compatibility". Other than that, feel free to add my Acked-by: Marc Zyngier <maz@kernel.org> Thanks, M. -- Jazz is not dead. It just smells funny... _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-01-20 11:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1579097798-106243-1-git-send-email-vladimir.murzin@arm.com>
2020-01-20 10:46 ` [PATCH] ARM: virt: Relax arch timer version check during early boot Vladimir Murzin
2020-01-20 11:14 ` Marc Zyngier
2020-01-20 11:34 ` Vladimir Murzin
2020-01-20 11:56 ` Marc Zyngier
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox