diff mbox series

[PRE-4.12] xen/arm: time: cycles_t should be an uint64_t and not unsigned long

Message ID 20190620174706.16657-1-julien.grall@arm.com (mailing list archive)
State New, archived
Headers show
Series [PRE-4.12] xen/arm: time: cycles_t should be an uint64_t and not unsigned long | expand

Commit Message

Julien Grall June 20, 2019, 5:47 p.m. UTC
Since commit ca73ac8e7d "xen/arm: Add an isb() before reading CNTPCT_EL0
to prevent re-ordering", get_cycles() is now returning the number of
cycles and used in more callers.

While the counter registers is always 64-bit, get_cycles() will only
reutrn a 32-bit on Arm32 and therefore truncate the value. This will
result to weird behavior by both Xen and the Guest as the timer will not
be setup correctly.

This could be resolved by switch cycles_t from unsigned long to
unsigned int.

Signed-off-by: Julien Grall <julien.grall@arm.com>

---
    This is only targeting xen 4.11 and earlier. Xen 4.12 and later have
    a correct definition of cycles_t thanks to da3d55ae67 "console:
    avoid printing no or null time stamps".

    This will hopefully unblock osstest on staging-4.10 and
    staging-4.11. This patch should be backported up to Xen 4.8.
---
 xen/include/asm-arm/time.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Andrew Cooper June 20, 2019, 5:50 p.m. UTC | #1
On 20/06/2019 18:47, Julien Grall wrote:
> Since commit ca73ac8e7d "xen/arm: Add an isb() before reading CNTPCT_EL0
> to prevent re-ordering", get_cycles() is now returning the number of
> cycles and used in more callers.
>
> While the counter registers is always 64-bit, get_cycles() will only
> reutrn a 32-bit on Arm32 and therefore truncate the value. This will

"return 32 bits"

~Andrew
Stefano Stabellini June 20, 2019, 11:56 p.m. UTC | #2
On Thu, 20 Jun 2019, Julien Grall wrote:
> Since commit ca73ac8e7d "xen/arm: Add an isb() before reading CNTPCT_EL0
> to prevent re-ordering", get_cycles() is now returning the number of
> cycles and used in more callers.
> 
> While the counter registers is always 64-bit, get_cycles() will only
> reutrn a 32-bit on Arm32 and therefore truncate the value. This will
> result to weird behavior by both Xen and the Guest as the timer will not
> be setup correctly.
> 
> This could be resolved by switch cycles_t from unsigned long to
> unsigned int.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

I'll commit adding a reference to
da3d55ae67225798c2ad8f42af2f432f6f2b2214 in the commit message.


> ---
>     This is only targeting xen 4.11 and earlier. Xen 4.12 and later have
>     a correct definition of cycles_t thanks to da3d55ae67 "console:
>     avoid printing no or null time stamps".
> 
>     This will hopefully unblock osstest on staging-4.10 and
>     staging-4.11. This patch should be backported up to Xen 4.8.
> ---
>  xen/include/asm-arm/time.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/include/asm-arm/time.h b/xen/include/asm-arm/time.h
> index ca30406669..bd7dc86d78 100644
> --- a/xen/include/asm-arm/time.h
> +++ b/xen/include/asm-arm/time.h
> @@ -7,7 +7,7 @@
>      DT_MATCH_COMPATIBLE("arm,armv7-timer"), \
>      DT_MATCH_COMPATIBLE("arm,armv8-timer")
>  
> -typedef unsigned long cycles_t;
> +typedef uint64_t cycles_t;
>  
>  static inline cycles_t get_cycles (void)
>  {
> -- 
> 2.11.0
>
Julien Grall June 21, 2019, 9:20 a.m. UTC | #3
Hi,

On 20/06/2019 18:50, Andrew Cooper wrote:
> On 20/06/2019 18:47, Julien Grall wrote:
>> Since commit ca73ac8e7d "xen/arm: Add an isb() before reading CNTPCT_EL0
>> to prevent re-ordering", get_cycles() is now returning the number of
>> cycles and used in more callers.
>>
>> While the counter registers is always 64-bit, get_cycles() will only
>> reutrn a 32-bit on Arm32 and therefore truncate the value. This will
> 
> "return 32 bits"

Yes, sorry Stefano committed without this change.

Cheers,
diff mbox series

Patch

diff --git a/xen/include/asm-arm/time.h b/xen/include/asm-arm/time.h
index ca30406669..bd7dc86d78 100644
--- a/xen/include/asm-arm/time.h
+++ b/xen/include/asm-arm/time.h
@@ -7,7 +7,7 @@ 
     DT_MATCH_COMPATIBLE("arm,armv7-timer"), \
     DT_MATCH_COMPATIBLE("arm,armv8-timer")
 
-typedef unsigned long cycles_t;
+typedef uint64_t cycles_t;
 
 static inline cycles_t get_cycles (void)
 {