lkml.org 
[lkml]   [2016]   [Aug]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC] arm64: Enforce gettimeofday vdso structure read ordering
On 2016-08-11 11:54, Will Deacon wrote:
> On Thu, Aug 11, 2016 at 11:37:44AM -0400, Christopher Covington wrote:
>> From: Brent DeGraaf <bdegraaf@codeaurora.org>
>>
>> Prior gettimeofday code register read code is not architecturally
>> correct as there is no control flow gating logic enforced
>> immediately prior to the required isb. Introduce explicit
>> control-flow logic prior to register read in all cases so that
>> the mrs read will always be done after all vdso data elements are
>> read, and read all data elements within the protection logic
>> provided by the sequence counter.
>
> -ENOPARSE
>
> Can you explain the problem that you're fixing here, please?
>
>> Signed-off-by: Brent DeGraaf <bdegraaf@codeaurora.org>
>> ---
>> arch/arm64/include/asm/vdso_datapage.h | 4 +-
>> arch/arm64/kernel/vdso.c | 11 ++--
>> arch/arm64/kernel/vdso/gettimeofday.S | 106
>> +++++++++++++++------------------
>> 3 files changed, 56 insertions(+), 65 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/vdso_datapage.h
>> b/arch/arm64/include/asm/vdso_datapage.h
>> index 2b9a637..49a0a51 100644
>> --- a/arch/arm64/include/asm/vdso_datapage.h
>> +++ b/arch/arm64/include/asm/vdso_datapage.h
>> @@ -21,6 +21,8 @@
>> #ifndef __ASSEMBLY__
>>
>> struct vdso_data {
>> + __u32 tb_seq_count; /* Timebase sequence counter */
>> + __u32 use_syscall;
>> __u64 cs_cycle_last; /* Timebase at clocksource init */
>> __u64 raw_time_sec; /* Raw time */
>> __u64 raw_time_nsec;
>> @@ -30,14 +32,12 @@ struct vdso_data {
>> __u64 xtime_coarse_nsec;
>> __u64 wtm_clock_sec; /* Wall to monotonic time */
>> __u64 wtm_clock_nsec;
>> - __u32 tb_seq_count; /* Timebase sequence counter */
>> /* cs_* members must be adjacent and in this order (ldp accesses) */
>> __u32 cs_mono_mult; /* NTP-adjusted clocksource multiplier */
>> __u32 cs_shift; /* Clocksource shift (mono = raw) */
>> __u32 cs_raw_mult; /* Raw clocksource multiplier */
>> __u32 tz_minuteswest; /* Whacky timezone stuff */
>> __u32 tz_dsttime;
>> - __u32 use_syscall;
>> };
>>
>> #endif /* !__ASSEMBLY__ */
>> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
>> index 076312b..7751667 100644
>> --- a/arch/arm64/kernel/vdso.c
>> +++ b/arch/arm64/kernel/vdso.c
>> @@ -201,10 +201,12 @@ up_fail:
>> */
>> void update_vsyscall(struct timekeeper *tk)
>> {
>> + register u32 tmp;
>> u32 use_syscall = strcmp(tk->tkr_mono.clock->name,
>> "arch_sys_counter");
>>
>> - ++vdso_data->tb_seq_count;
>> - smp_wmb();
>> + tmp = smp_load_acquire(&vdso_data->tb_seq_count);
>> + ++tmp;
>> + smp_store_release(&vdso_data->tb_seq_count, tmp);
>
> This looks busted -- the writes to vdso_data can be reordered before
> the
> update of tb_seq_count.
>
> /confused
>
> Will

The ldar/strl contain implicit barriers that will prevent the reorder,
similar to the way they allowed removal of the explicit barriers in the
spinlock code.

Thank you,
Brent

\
 
 \ /
  Last update: 2016-08-11 20:21    [W:0.064 / U:0.700 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site