[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.