[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 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.

+    /*
+     * It is expected that the ring is not full when this function is called
+     * as the guest is expected to write to the data register only when the
+     * TXFF flag is not set.
+     * In case the guest does write even when the TXFF flag is set then the
+     * data will be silently dropped.
+     */
+    if ( xencons_queued(out_prod, out_cons, sizeof(intf->out)) !=
+         sizeof (intf->out) )
+    {
+        intf->out[xencons_mask(out_prod, sizeof(intf->out))] = data;
+        out_prod += 1;
+        smp_wmb();
+        intf->out_prod = out_prod;
+    }
+    else
+        gprintk(XENLOG_ERR, "vpl011: Unexpected OUT ring buffer full\n");
+
+    if ( xencons_queued(out_prod, out_cons, sizeof(intf->out)) ==
+         sizeof (intf->out) )
+    {
+        vpl011->uartfr |= TXFF;
+        vpl011->uartris &= ~TXI;
+    }
+
+    vpl011->uartfr |= BUSY;
+
+    vpl011->uartfr &= ~TXFE;
+
+    vpl011_update_interrupt_status(d);
+
+    VPL011_UNLOCK(d, flags);
+
+    /*
+     * Send an event to console backend to indicate that there is
+     * data in the OUT ring buffer.
+     */
+    notify_via_xen_event_channel(d, vpl011->evtchn);
+}

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®.