Re: [Xen-devel] [PATCH for-next 7/7] x86: implement Hyper-V clock source

[Wei Liu <wl@xxxxxxx>]

On Wed, Dec 18, 2019 at 02:24:33PM +0100, Jan Beulich wrote:
> On 18.12.2019 14:18, Wei Liu wrote:
> > On Wed, Dec 18, 2019 at 01:51:54PM +0100, Jan Beulich wrote:
> >> On 18.12.2019 13:38, Wei Liu wrote:
> >>> On Tue, Dec 10, 2019 at 05:59:04PM +0100, Jan Beulich wrote:
> >>>> On 25.10.2019 11:16, Wei Liu wrote:
> >>>>> +static inline uint64_t read_hyperv_timer(void)
> >>>>> +{
> >>>>> +    uint64_t scale, offset, ret, tsc;
> >>>>> +    uint32_t seq;
> >>>>> +    struct ms_hyperv_tsc_page *tsc_page = &hyperv_tsc_page;
> >>>>> +
> >>>>> +    do {
> >>>>> +        seq = tsc_page->tsc_sequence;
> >>>>> +
> >>>>> +        /* Seq 0 is special. It means the TSC enlightenment is not
> >>>>> +         * available at the moment. The reference time can only be
> >>>>> +         * obtained from the Reference Counter MSR.
> >>>>> +         */
> >>>>> +        if ( seq == 0 )
> >>>>> +        {
> >>>>> +            rdmsrl(HV_X64_MSR_TIME_REF_COUNT, ret);
> >>>>> +            return ret;
> >>>>> +        }
> >>>>> +
> >>>>> +        smp_rmb();
> >>>>> +
> >>>>> +        tsc = rdtsc_ordered();
> >>>>
> >>>> This already includes at least a read fence.
> >>>
> >>> OK. rdtsc() should be enough here.
> >>
> >> Are you sure? My comment was rather towards the dropping of smp_rmb()
> >> (maybe replacing by a comment).
> > 
> > I do mean to keep smp_rmb() before it. Is that not enough?
> 
> With
> 
> #define smp_rmb()       barrier()
> 
> it isn't - it's merely a compiler barrier, but for the ordering
> you want you need a fence.

Ah, I see. Thank you.

Wei.

> 
> Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel