* 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;
as well as URLs for NNTP newsgroup(s).