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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

Jan

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