Re: [Xen-devel] [PATCH v2 3/4] xen/x86: Replace remaining mandatory barriers with SMP barriers

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.08.17 at 11:35, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 17/08/17 09:37, Jan Beulich wrote:
>>>>> On 16.08.17 at 13:22, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> --- a/xen/arch/x86/time.c
>>> +++ b/xen/arch/x86/time.c
>>> @@ -976,10 +976,10 @@ static void __update_vcpu_system_time(struct vcpu *v, 
>>> int force)
>>>  
>>>      /* 1. Update guest kernel version. */
>>>      _u.version = u->version = version_update_begin(u->version);
>>> -    wmb();
>>> +    smp_wmb();
>>>      /* 2. Update all other guest kernel fields. */
>>>      *u = _u;
>>> -    wmb();
>>> +    smp_wmb();
>>>      /* 3. Update guest kernel version. */
>>>      u->version = version_update_end(u->version);
>>>  
>>> @@ -1006,10 +1006,10 @@ bool update_secondary_system_time(struct vcpu *v,
>>>          update_guest_memory_policy(v, &policy);
>>>          return false;
>>>      }
>>> -    wmb();
>>> +    smp_wmb();
>>>      /* 2. Update all other userspace fields. */
>>>      __copy_to_guest(user_u, u, 1);
>>> -    wmb();
>>> +    smp_wmb();
>>>      /* 3. Update userspace version. */
>>>      u->version = version_update_end(u->version);
>>>      __copy_field_to_guest(user_u, u, version);
>> Same fore these.
> 
> Why?  The guest side of this protocol is just reads.

As always (and as you keep stressing) barriers make sense almost
exclusively when they're being used on both sides. This applies
here too. It's just that even a non-SMP consumer would need
barriers; that's along the lines of why Linux has gained virt_mb().

> Irrespective, how do you suggest I make things more clear?

Well, it was more a remark than a request for you to change
anything. I agree there's little point of adding a comment on the
hypervisor side of things.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel