Re: [PATCH 04/17 v5] xen/arm: vpl011: Add SBSA UART emulation in Xen

[Julien Grall <julien.grall@arm.com>]

Hi,

On 06/23/2017 07:28 PM, Stefano Stabellini wrote:
> On Fri, 23 Jun 2017, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 22/06/17 23:53, Stefano Stabellini wrote:
>>> On Thu, 22 Jun 2017, Bhupinder Thakur wrote:
>>>> +static void vpl011_write_data(struct domain *d, uint8_t data)
>>>> +{
>>>> +    unsigned long flags;
>>>> +    struct vpl011 *vpl011 = &d->arch.vpl011;
>>>> +    struct xencons_interface *intf = vpl011->ring_buf;
>>>> +    XENCONS_RING_IDX out_cons, out_prod;
>>>> +
>>>> +    VPL011_LOCK(d, flags);
>>>> +
>>>> +    out_cons = intf->out_cons;
>>>> +    out_prod = intf->out_prod;
>>>> +
>>>> +    smp_rmb();
>>>
>>> This should be
>>>         smp_mb();
>>
>> To speed up discussion, it would have been nice to give a bit more details why
>> you think smp_rmb() is not enough.
>>
>> In this case, I think smp_rmb() is fine because all the write we care depends
>> on out_cons and out_prod. So the processor cannot re-order it.
> 
> We discussed these barriers at length when I published the pvcalls and
> xen 9pfs protocols, see for example
> alpine.DEB.2.10.1612021318340.2777@sstabellini-ThinkPad-X260. Please
> refer to "Ring Usage" in docs/misc/9pfs.markdown and "Workflow" in
> docs/misc/pvcalls.markdown. I would like to keep them consistent across
> protocols (the console protocol works exactly like pvcalls and xen 9pfs
> in that respect).

None of the people involved in this series were CCed on this thread and 
looking at docs/misc/9pfs.markdown or docs/misc/pvcalls.markdown to 
check what barrier usage for all the PV protocols seem a bit odd...

Anyway, you should probably think of writing a common PV document to 
avoid similar question in the future.

Cheers,

-- 
Julien Grall

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