* [PATCH v3 2/2] clocksource: arm_arch_timer: Correct fault programming of CNTKCTL_EL1.EVNTI
@ 2020-12-04 7:31 ` Keqian Zhu
0 siblings, 0 replies; 20+ messages in thread
From: Keqian Zhu @ 2020-12-04 7:31 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel, kvmarm, kvm
Cc: Suzuki K Poulose, Marc Zyngier, Keqian Zhu, Daniel Lezcano,
Sean Christopherson, Alexios Zavras, Mark Brown, James Morse,
Catalin Marinas, wanghaibin.wang, Thomas Gleixner, Will Deacon,
Andrew Morton, Julien Thierry
ARM virtual counter supports event stream, it can only trigger an event
when the trigger bit (the value of CNTKCTL_EL1.EVNTI) of CNTVCT_EL0 changes,
so the actual period of event stream is 2^(cntkctl_evnti + 1). For example,
when the trigger bit is 0, then virtual counter trigger an event for every
two cycles.
Fixes: 037f637767a8 ("drivers: clocksource: add support for ARM architected timer event stream")
Suggested-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
---
drivers/clocksource/arm_arch_timer.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 777d38cb39b0..d0177824c518 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -822,15 +822,24 @@ static void arch_timer_evtstrm_enable(int divider)
static void arch_timer_configure_evtstream(void)
{
- int evt_stream_div, pos;
+ int evt_stream_div, lsb;
+
+ /*
+ * As the event stream can at most be generated at half the frequency
+ * of the counter, use half the frequency when computing the divider.
+ */
+ evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ / 2;
+
+ /*
+ * Find the closest power of two to the divisor. If the adjacent bit
+ * of lsb (last set bit, starts from 0) is set, then we use (lsb + 1).
+ */
+ lsb = fls(evt_stream_div) - 1;
+ if (lsb > 0 && (evt_stream_div & BIT(lsb - 1)))
+ lsb++;
- /* Find the closest power of two to the divisor */
- evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
- pos = fls(evt_stream_div);
- if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
- pos--;
/* enable event stream */
- arch_timer_evtstrm_enable(min(pos, 15));
+ arch_timer_evtstrm_enable(max(0, min(lsb, 15)));
}
static void arch_counter_set_user_access(void)
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v3 2/2] clocksource: arm_arch_timer: Correct fault programming of CNTKCTL_EL1.EVNTI
@ 2020-12-04 7:31 ` Keqian Zhu
0 siblings, 0 replies; 20+ messages in thread
From: Keqian Zhu @ 2020-12-04 7:31 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel, kvmarm, kvm
Cc: Marc Zyngier, Daniel Lezcano, Sean Christopherson,
Alexios Zavras, Mark Brown, Catalin Marinas, Thomas Gleixner,
Will Deacon, Andrew Morton
ARM virtual counter supports event stream, it can only trigger an event
when the trigger bit (the value of CNTKCTL_EL1.EVNTI) of CNTVCT_EL0 changes,
so the actual period of event stream is 2^(cntkctl_evnti + 1). For example,
when the trigger bit is 0, then virtual counter trigger an event for every
two cycles.
Fixes: 037f637767a8 ("drivers: clocksource: add support for ARM architected timer event stream")
Suggested-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
---
drivers/clocksource/arm_arch_timer.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 777d38cb39b0..d0177824c518 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -822,15 +822,24 @@ static void arch_timer_evtstrm_enable(int divider)
static void arch_timer_configure_evtstream(void)
{
- int evt_stream_div, pos;
+ int evt_stream_div, lsb;
+
+ /*
+ * As the event stream can at most be generated at half the frequency
+ * of the counter, use half the frequency when computing the divider.
+ */
+ evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ / 2;
+
+ /*
+ * Find the closest power of two to the divisor. If the adjacent bit
+ * of lsb (last set bit, starts from 0) is set, then we use (lsb + 1).
+ */
+ lsb = fls(evt_stream_div) - 1;
+ if (lsb > 0 && (evt_stream_div & BIT(lsb - 1)))
+ lsb++;
- /* Find the closest power of two to the divisor */
- evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
- pos = fls(evt_stream_div);
- if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
- pos--;
/* enable event stream */
- arch_timer_evtstrm_enable(min(pos, 15));
+ arch_timer_evtstrm_enable(max(0, min(lsb, 15)));
}
static void arch_counter_set_user_access(void)
--
2.23.0
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v3 2/2] clocksource: arm_arch_timer: Correct fault programming of CNTKCTL_EL1.EVNTI
2020-12-04 7:31 ` Keqian Zhu
(?)
@ 2020-12-05 11:15 ` Daniel Lezcano
-1 siblings, 0 replies; 20+ messages in thread
From: Daniel Lezcano @ 2020-12-05 11:15 UTC (permalink / raw)
To: Keqian Zhu, linux-kernel, linux-arm-kernel, kvmarm, kvm
Cc: Marc Zyngier, Thomas Gleixner, Catalin Marinas, Will Deacon,
James Morse, Suzuki K Poulose, Sean Christopherson,
Julien Thierry, Mark Brown, Andrew Morton, Alexios Zavras,
wanghaibin.wang
Hi Marc,
are you fine with this patch ?
On 04/12/2020 08:31, Keqian Zhu wrote:
> ARM virtual counter supports event stream, it can only trigger an event
> when the trigger bit (the value of CNTKCTL_EL1.EVNTI) of CNTVCT_EL0 changes,
> so the actual period of event stream is 2^(cntkctl_evnti + 1). For example,
> when the trigger bit is 0, then virtual counter trigger an event for every
> two cycles.
>
> Fixes: 037f637767a8 ("drivers: clocksource: add support for ARM architected timer event stream")
> Suggested-by: Marc Zyngier <maz@kernel.org>
> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
> ---
> drivers/clocksource/arm_arch_timer.c | 23 ++++++++++++++++-------
> 1 file changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 777d38cb39b0..d0177824c518 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -822,15 +822,24 @@ static void arch_timer_evtstrm_enable(int divider)
>
> static void arch_timer_configure_evtstream(void)
> {
> - int evt_stream_div, pos;
> + int evt_stream_div, lsb;
> +
> + /*
> + * As the event stream can at most be generated at half the frequency
> + * of the counter, use half the frequency when computing the divider.
> + */
> + evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ / 2;
> +
> + /*
> + * Find the closest power of two to the divisor. If the adjacent bit
> + * of lsb (last set bit, starts from 0) is set, then we use (lsb + 1).
> + */
> + lsb = fls(evt_stream_div) - 1;
> + if (lsb > 0 && (evt_stream_div & BIT(lsb - 1)))
> + lsb++;
>
> - /* Find the closest power of two to the divisor */
> - evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
> - pos = fls(evt_stream_div);
> - if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
> - pos--;
> /* enable event stream */
> - arch_timer_evtstrm_enable(min(pos, 15));
> + arch_timer_evtstrm_enable(max(0, min(lsb, 15)));
> }
>
> static void arch_counter_set_user_access(void)
>
--
<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] 20+ messages in thread* Re: [PATCH v3 2/2] clocksource: arm_arch_timer: Correct fault programming of CNTKCTL_EL1.EVNTI
@ 2020-12-05 11:15 ` Daniel Lezcano
0 siblings, 0 replies; 20+ messages in thread
From: Daniel Lezcano @ 2020-12-05 11:15 UTC (permalink / raw)
To: Keqian Zhu, linux-kernel, linux-arm-kernel, kvmarm, kvm
Cc: Suzuki K Poulose, Marc Zyngier, Sean Christopherson,
Alexios Zavras, Mark Brown, James Morse, Catalin Marinas,
wanghaibin.wang, Thomas Gleixner, Will Deacon, Andrew Morton,
Julien Thierry
Hi Marc,
are you fine with this patch ?
On 04/12/2020 08:31, Keqian Zhu wrote:
> ARM virtual counter supports event stream, it can only trigger an event
> when the trigger bit (the value of CNTKCTL_EL1.EVNTI) of CNTVCT_EL0 changes,
> so the actual period of event stream is 2^(cntkctl_evnti + 1). For example,
> when the trigger bit is 0, then virtual counter trigger an event for every
> two cycles.
>
> Fixes: 037f637767a8 ("drivers: clocksource: add support for ARM architected timer event stream")
> Suggested-by: Marc Zyngier <maz@kernel.org>
> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
> ---
> drivers/clocksource/arm_arch_timer.c | 23 ++++++++++++++++-------
> 1 file changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 777d38cb39b0..d0177824c518 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -822,15 +822,24 @@ static void arch_timer_evtstrm_enable(int divider)
>
> static void arch_timer_configure_evtstream(void)
> {
> - int evt_stream_div, pos;
> + int evt_stream_div, lsb;
> +
> + /*
> + * As the event stream can at most be generated at half the frequency
> + * of the counter, use half the frequency when computing the divider.
> + */
> + evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ / 2;
> +
> + /*
> + * Find the closest power of two to the divisor. If the adjacent bit
> + * of lsb (last set bit, starts from 0) is set, then we use (lsb + 1).
> + */
> + lsb = fls(evt_stream_div) - 1;
> + if (lsb > 0 && (evt_stream_div & BIT(lsb - 1)))
> + lsb++;
>
> - /* Find the closest power of two to the divisor */
> - evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
> - pos = fls(evt_stream_div);
> - if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
> - pos--;
> /* enable event stream */
> - arch_timer_evtstrm_enable(min(pos, 15));
> + arch_timer_evtstrm_enable(max(0, min(lsb, 15)));
> }
>
> static void arch_counter_set_user_access(void)
>
--
<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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v3 2/2] clocksource: arm_arch_timer: Correct fault programming of CNTKCTL_EL1.EVNTI
@ 2020-12-05 11:15 ` Daniel Lezcano
0 siblings, 0 replies; 20+ messages in thread
From: Daniel Lezcano @ 2020-12-05 11:15 UTC (permalink / raw)
To: Keqian Zhu, linux-kernel, linux-arm-kernel, kvmarm, kvm
Cc: Marc Zyngier, Sean Christopherson, Alexios Zavras, Mark Brown,
Catalin Marinas, Thomas Gleixner, Will Deacon, Andrew Morton
Hi Marc,
are you fine with this patch ?
On 04/12/2020 08:31, Keqian Zhu wrote:
> ARM virtual counter supports event stream, it can only trigger an event
> when the trigger bit (the value of CNTKCTL_EL1.EVNTI) of CNTVCT_EL0 changes,
> so the actual period of event stream is 2^(cntkctl_evnti + 1). For example,
> when the trigger bit is 0, then virtual counter trigger an event for every
> two cycles.
>
> Fixes: 037f637767a8 ("drivers: clocksource: add support for ARM architected timer event stream")
> Suggested-by: Marc Zyngier <maz@kernel.org>
> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
> ---
> drivers/clocksource/arm_arch_timer.c | 23 ++++++++++++++++-------
> 1 file changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 777d38cb39b0..d0177824c518 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -822,15 +822,24 @@ static void arch_timer_evtstrm_enable(int divider)
>
> static void arch_timer_configure_evtstream(void)
> {
> - int evt_stream_div, pos;
> + int evt_stream_div, lsb;
> +
> + /*
> + * As the event stream can at most be generated at half the frequency
> + * of the counter, use half the frequency when computing the divider.
> + */
> + evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ / 2;
> +
> + /*
> + * Find the closest power of two to the divisor. If the adjacent bit
> + * of lsb (last set bit, starts from 0) is set, then we use (lsb + 1).
> + */
> + lsb = fls(evt_stream_div) - 1;
> + if (lsb > 0 && (evt_stream_div & BIT(lsb - 1)))
> + lsb++;
>
> - /* Find the closest power of two to the divisor */
> - evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
> - pos = fls(evt_stream_div);
> - if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
> - pos--;
> /* enable event stream */
> - arch_timer_evtstrm_enable(min(pos, 15));
> + arch_timer_evtstrm_enable(max(0, min(lsb, 15)));
> }
>
> static void arch_counter_set_user_access(void)
>
--
<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
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v3 2/2] clocksource: arm_arch_timer: Correct fault programming of CNTKCTL_EL1.EVNTI
2020-12-05 11:15 ` Daniel Lezcano
(?)
@ 2020-12-05 18:22 ` Marc Zyngier
-1 siblings, 0 replies; 20+ messages in thread
From: Marc Zyngier @ 2020-12-05 18:22 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Keqian Zhu, linux-kernel, linux-arm-kernel, kvmarm, kvm,
Thomas Gleixner, Catalin Marinas, Will Deacon, James Morse,
Suzuki K Poulose, Sean Christopherson, Julien Thierry,
Mark Brown, Andrew Morton, Alexios Zavras, wanghaibin.wang
Hi Daniel,
On 2020-12-05 11:15, Daniel Lezcano wrote:
> Hi Marc,
>
> are you fine with this patch ?
I am, although there still isn't any justification for the pos/lsb
rework in the commit message (and calling that variable lsb is somewhat
confusing). If you are going to apply it, please consider adding
the additional comment below.
>
>
> On 04/12/2020 08:31, Keqian Zhu wrote:
>> ARM virtual counter supports event stream, it can only trigger an
>> event
>> when the trigger bit (the value of CNTKCTL_EL1.EVNTI) of CNTVCT_EL0
>> changes,
>> so the actual period of event stream is 2^(cntkctl_evnti + 1). For
>> example,
>> when the trigger bit is 0, then virtual counter trigger an event for
>> every
>> two cycles.
"While we're at it, rework the way we compute the trigger bit position
by
making it more obvious that when bits [n:n-1] are both set (with n
being
the most significant bit), we pick bit (n + 1)."
With that:
Acked-by: Marc Zyngier <maz@kernel.org>
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v3 2/2] clocksource: arm_arch_timer: Correct fault programming of CNTKCTL_EL1.EVNTI
@ 2020-12-05 18:22 ` Marc Zyngier
0 siblings, 0 replies; 20+ messages in thread
From: Marc Zyngier @ 2020-12-05 18:22 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Andrew Morton, kvm, Suzuki K Poulose, Catalin Marinas,
linux-kernel, Sean Christopherson, Alexios Zavras, Will Deacon,
Mark Brown, James Morse, Julien Thierry, wanghaibin.wang,
Thomas Gleixner, Keqian Zhu, kvmarm, linux-arm-kernel
Hi Daniel,
On 2020-12-05 11:15, Daniel Lezcano wrote:
> Hi Marc,
>
> are you fine with this patch ?
I am, although there still isn't any justification for the pos/lsb
rework in the commit message (and calling that variable lsb is somewhat
confusing). If you are going to apply it, please consider adding
the additional comment below.
>
>
> On 04/12/2020 08:31, Keqian Zhu wrote:
>> ARM virtual counter supports event stream, it can only trigger an
>> event
>> when the trigger bit (the value of CNTKCTL_EL1.EVNTI) of CNTVCT_EL0
>> changes,
>> so the actual period of event stream is 2^(cntkctl_evnti + 1). For
>> example,
>> when the trigger bit is 0, then virtual counter trigger an event for
>> every
>> two cycles.
"While we're at it, rework the way we compute the trigger bit position
by
making it more obvious that when bits [n:n-1] are both set (with n
being
the most significant bit), we pick bit (n + 1)."
With that:
Acked-by: Marc Zyngier <maz@kernel.org>
Thanks,
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v3 2/2] clocksource: arm_arch_timer: Correct fault programming of CNTKCTL_EL1.EVNTI
@ 2020-12-05 18:22 ` Marc Zyngier
0 siblings, 0 replies; 20+ messages in thread
From: Marc Zyngier @ 2020-12-05 18:22 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Andrew Morton, kvm, Catalin Marinas, linux-kernel,
Sean Christopherson, Alexios Zavras, Will Deacon, Mark Brown,
Thomas Gleixner, kvmarm, linux-arm-kernel
Hi Daniel,
On 2020-12-05 11:15, Daniel Lezcano wrote:
> Hi Marc,
>
> are you fine with this patch ?
I am, although there still isn't any justification for the pos/lsb
rework in the commit message (and calling that variable lsb is somewhat
confusing). If you are going to apply it, please consider adding
the additional comment below.
>
>
> On 04/12/2020 08:31, Keqian Zhu wrote:
>> ARM virtual counter supports event stream, it can only trigger an
>> event
>> when the trigger bit (the value of CNTKCTL_EL1.EVNTI) of CNTVCT_EL0
>> changes,
>> so the actual period of event stream is 2^(cntkctl_evnti + 1). For
>> example,
>> when the trigger bit is 0, then virtual counter trigger an event for
>> every
>> two cycles.
"While we're at it, rework the way we compute the trigger bit position
by
making it more obvious that when bits [n:n-1] are both set (with n
being
the most significant bit), we pick bit (n + 1)."
With that:
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] 20+ messages in thread* Re: [PATCH v3 2/2] clocksource: arm_arch_timer: Correct fault programming of CNTKCTL_EL1.EVNTI
2020-12-05 18:22 ` Marc Zyngier
(?)
@ 2020-12-05 18:30 ` Daniel Lezcano
-1 siblings, 0 replies; 20+ messages in thread
From: Daniel Lezcano @ 2020-12-05 18:30 UTC (permalink / raw)
To: Marc Zyngier
Cc: Keqian Zhu, linux-kernel, linux-arm-kernel, kvmarm, kvm,
Thomas Gleixner, Catalin Marinas, Will Deacon, James Morse,
Suzuki K Poulose, Sean Christopherson, Julien Thierry,
Mark Brown, Andrew Morton, Alexios Zavras, wanghaibin.wang
Hi Marc,
On 05/12/2020 19:22, Marc Zyngier wrote:
> Hi Daniel,
>
> On 2020-12-05 11:15, Daniel Lezcano wrote:
>> Hi Marc,
>>
>> are you fine with this patch ?
>
> I am, although there still isn't any justification for the pos/lsb
> rework in the commit message (and calling that variable lsb is somewhat
> confusing). If you are going to apply it, please consider adding
> the additional comment below.
Ok, I will do that.
Thanks for the additional comment
-- Daniel
>> On 04/12/2020 08:31, Keqian Zhu wrote:
>>> ARM virtual counter supports event stream, it can only trigger an event
>>> when the trigger bit (the value of CNTKCTL_EL1.EVNTI) of CNTVCT_EL0
>>> changes,
>>> so the actual period of event stream is 2^(cntkctl_evnti + 1). For
>>> example,
>>> when the trigger bit is 0, then virtual counter trigger an event for
>>> every
>>> two cycles.
>
> "While we're at it, rework the way we compute the trigger bit position by
> making it more obvious that when bits [n:n-1] are both set (with n being
> the most significant bit), we pick bit (n + 1)."
>
> With that:
>
> Acked-by: Marc Zyngier <maz@kernel.org>
>
> Thanks,
>
> M.
--
<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] 20+ messages in thread
* Re: [PATCH v3 2/2] clocksource: arm_arch_timer: Correct fault programming of CNTKCTL_EL1.EVNTI
@ 2020-12-05 18:30 ` Daniel Lezcano
0 siblings, 0 replies; 20+ messages in thread
From: Daniel Lezcano @ 2020-12-05 18:30 UTC (permalink / raw)
To: Marc Zyngier
Cc: Andrew Morton, kvm, Suzuki K Poulose, Catalin Marinas,
linux-kernel, Sean Christopherson, Alexios Zavras, Will Deacon,
Mark Brown, James Morse, Julien Thierry, wanghaibin.wang,
Thomas Gleixner, Keqian Zhu, kvmarm, linux-arm-kernel
Hi Marc,
On 05/12/2020 19:22, Marc Zyngier wrote:
> Hi Daniel,
>
> On 2020-12-05 11:15, Daniel Lezcano wrote:
>> Hi Marc,
>>
>> are you fine with this patch ?
>
> I am, although there still isn't any justification for the pos/lsb
> rework in the commit message (and calling that variable lsb is somewhat
> confusing). If you are going to apply it, please consider adding
> the additional comment below.
Ok, I will do that.
Thanks for the additional comment
-- Daniel
>> On 04/12/2020 08:31, Keqian Zhu wrote:
>>> ARM virtual counter supports event stream, it can only trigger an event
>>> when the trigger bit (the value of CNTKCTL_EL1.EVNTI) of CNTVCT_EL0
>>> changes,
>>> so the actual period of event stream is 2^(cntkctl_evnti + 1). For
>>> example,
>>> when the trigger bit is 0, then virtual counter trigger an event for
>>> every
>>> two cycles.
>
> "While we're at it, rework the way we compute the trigger bit position by
> making it more obvious that when bits [n:n-1] are both set (with n being
> the most significant bit), we pick bit (n + 1)."
>
> With that:
>
> Acked-by: Marc Zyngier <maz@kernel.org>
>
> Thanks,
>
> M.
--
<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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 2/2] clocksource: arm_arch_timer: Correct fault programming of CNTKCTL_EL1.EVNTI
@ 2020-12-05 18:30 ` Daniel Lezcano
0 siblings, 0 replies; 20+ messages in thread
From: Daniel Lezcano @ 2020-12-05 18:30 UTC (permalink / raw)
To: Marc Zyngier
Cc: Andrew Morton, kvm, Catalin Marinas, linux-kernel,
Sean Christopherson, Alexios Zavras, Will Deacon, Mark Brown,
Thomas Gleixner, kvmarm, linux-arm-kernel
Hi Marc,
On 05/12/2020 19:22, Marc Zyngier wrote:
> Hi Daniel,
>
> On 2020-12-05 11:15, Daniel Lezcano wrote:
>> Hi Marc,
>>
>> are you fine with this patch ?
>
> I am, although there still isn't any justification for the pos/lsb
> rework in the commit message (and calling that variable lsb is somewhat
> confusing). If you are going to apply it, please consider adding
> the additional comment below.
Ok, I will do that.
Thanks for the additional comment
-- Daniel
>> On 04/12/2020 08:31, Keqian Zhu wrote:
>>> ARM virtual counter supports event stream, it can only trigger an event
>>> when the trigger bit (the value of CNTKCTL_EL1.EVNTI) of CNTVCT_EL0
>>> changes,
>>> so the actual period of event stream is 2^(cntkctl_evnti + 1). For
>>> example,
>>> when the trigger bit is 0, then virtual counter trigger an event for
>>> every
>>> two cycles.
>
> "While we're at it, rework the way we compute the trigger bit position by
> making it more obvious that when bits [n:n-1] are both set (with n being
> the most significant bit), we pick bit (n + 1)."
>
> With that:
>
> Acked-by: Marc Zyngier <maz@kernel.org>
>
> Thanks,
>
> M.
--
<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
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 20+ messages in thread
* [tip: timers/core] clocksource/drivers/arm_arch_timer: Correct fault programming of CNTKCTL_EL1.EVNTI
2020-12-04 7:31 ` Keqian Zhu
` (2 preceding siblings ...)
(?)
@ 2020-12-12 12:58 ` tip-bot2 for Keqian Zhu
-1 siblings, 0 replies; 20+ messages in thread
From: tip-bot2 for Keqian Zhu @ 2020-12-12 12:58 UTC (permalink / raw)
To: linux-tip-commits
Cc: Marc Zyngier, Keqian Zhu, Daniel Lezcano, x86, linux-kernel
The following commit has been merged into the timers/core branch of tip:
Commit-ID: 8b7770b877d187bfdae1eaf587bd2b792479a31c
Gitweb: https://git.kernel.org/tip/8b7770b877d187bfdae1eaf587bd2b792479a31c
Author: Keqian Zhu <zhukeqian1@huawei.com>
AuthorDate: Fri, 04 Dec 2020 15:31:26 +08:00
Committer: Daniel Lezcano <daniel.lezcano@linaro.org>
CommitterDate: Sat, 05 Dec 2020 19:34:04 +01:00
clocksource/drivers/arm_arch_timer: Correct fault programming of CNTKCTL_EL1.EVNTI
ARM virtual counter supports event stream, it can only trigger an event
when the trigger bit (the value of CNTKCTL_EL1.EVNTI) of CNTVCT_EL0 changes,
so the actual period of event stream is 2^(cntkctl_evnti + 1). For example,
when the trigger bit is 0, then virtual counter trigger an event for every
two cycles.
While we're at it, rework the way we compute the trigger bit position
by making it more obvious that when bits [n:n-1] are both set (with n
being the most significant bit), we pick bit (n + 1).
Fixes: 037f637767a8 ("drivers: clocksource: add support for ARM architected timer event stream")
Suggested-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
Acked-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Link: https://lore.kernel.org/r/20201204073126.6920-3-zhukeqian1@huawei.com
---
drivers/clocksource/arm_arch_timer.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 777d38c..d017782 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -822,15 +822,24 @@ static void arch_timer_evtstrm_enable(int divider)
static void arch_timer_configure_evtstream(void)
{
- int evt_stream_div, pos;
+ int evt_stream_div, lsb;
+
+ /*
+ * As the event stream can at most be generated at half the frequency
+ * of the counter, use half the frequency when computing the divider.
+ */
+ evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ / 2;
+
+ /*
+ * Find the closest power of two to the divisor. If the adjacent bit
+ * of lsb (last set bit, starts from 0) is set, then we use (lsb + 1).
+ */
+ lsb = fls(evt_stream_div) - 1;
+ if (lsb > 0 && (evt_stream_div & BIT(lsb - 1)))
+ lsb++;
- /* Find the closest power of two to the divisor */
- evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
- pos = fls(evt_stream_div);
- if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
- pos--;
/* enable event stream */
- arch_timer_evtstrm_enable(min(pos, 15));
+ arch_timer_evtstrm_enable(max(0, min(lsb, 15)));
}
static void arch_counter_set_user_access(void)
^ permalink raw reply related [flat|nested] 20+ messages in thread