[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
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). > > > + /* > > > + * 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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |