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

Re: [Xen-devel] [PATCH 03/14 v4] xen/arm: vpl011: Add pl011 uart emulation in Xen



On 13/06/2017 09:58, Bhupinder Thakur wrote:
Hi Julien,

Hi Bhupinder,

+static uint8_t vpl011_read_data(struct domain *d)
+{
+    unsigned long flags;
+    uint8_t data = 0;
+    struct vpl011 *vpl011 = &d->arch.vpl011;
+    struct xencons_interface *intf = vpl011->ring_buf;
+    XENCONS_RING_IDX in_cons = intf->in_cons;
+    XENCONS_RING_IDX in_prod = intf->in_prod;


After reading the indexes, we always need barriers. In this case:

  smp_rmb();


Well, there are already barrier with the spinlock. However, I am bit concern
about reading those index without the lock taken. You can have concurrent
call to vpl011_read_data happening and therefore the indexes may have
changed when the lock will be taken.

Is there a possibility of concurrent access since this function is
executed as part of
trap handling which will serialize access to this function?

There are no locking in the common trap handling. The emulation should take care of the locking because multiple vCPU can concurrently access the MMIO region.

[....]

I am pretty sure that the PV console protocol requires us to notify the
other end even on reads. We need to add a notify_via_xen_event_channel
here, I think.


I would agree here. On the previous version, I asked Bhupinder to explain
why it is necessary and he said: "On second thoughs, notification is not
required".

I understand that xenconsole is currently using the event notification
as an indication to read
data from the ring buffer. For writing data, it keeps checking
periodically if there is space in the
ring buffer. If there is space then it writes more data.

You should not assume that xenconsoled is the only backend console. One could decide to implement its own.

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