From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: [PATCH] xen/HVM: atomically access pointers in bufioreq handling Date: Thu, 18 Jun 2015 14:18:39 +0100 Message-ID: <5582E14F0200007800086A37__34084.985117852$1434633638$gmane$org@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=__Part45719F3F.1__=" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Z5ZiL-0008Lm-Li for xen-devel@lists.xenproject.org; Thu, 18 Jun 2015 13:18:41 +0000 List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: qemu-devel@nongnu.org Cc: xen-devel , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org This is a MIME message. If you are reading this text, you may want to consider changing to a mail reader or gateway that understands how to properly handle MIME multipart messages. --=__Part45719F3F.1__= Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Content-Disposition: inline The number of slots per page being 511 (i.e. not a power of two) means that the (32-bit) read and write indexes going beyond 2^32 will likely disturb operation. The hypervisor side gets I/O req server creation extended so we can indicate that we're using suitable atomic accesses where needed (not all accesses to the two pointers really need to be atomic), allowing it to atomically canonicalize both pointers when both have gone through at least one cycle. The Xen side counterpart (which is not a functional prereq to this change, albeit a build one) can be found at e.g. http://lists.xenproject.org/archives/html/xen-devel/2015-06/msg02996.html= =20 Signed-off-by: Jan Beulich --- a/xen-hvm.c +++ b/xen-hvm.c @@ -981,19 +981,30 @@ static void handle_ioreq(XenIOState *sta =20 static int handle_buffered_iopage(XenIOState *state) { + buffered_iopage_t *buf_page =3D state->buffered_io_page; buf_ioreq_t *buf_req =3D NULL; ioreq_t req; int qw; =20 - if (!state->buffered_io_page) { + if (!buf_page) { return 0; } =20 memset(&req, 0x00, sizeof(req)); =20 - while (state->buffered_io_page->read_pointer !=3D state->buffered_io_p= age->write_pointer) { - buf_req =3D &state->buffered_io_page->buf_ioreq[ - state->buffered_io_page->read_pointer % IOREQ_BUFFER_SLOT_NUM]= ; + for (;;) { + uint32_t rdptr =3D buf_page->read_pointer, wrptr; + + xen_rmb(); + wrptr =3D buf_page->write_pointer; + xen_rmb(); + if (rdptr !=3D buf_page->read_pointer) { + continue; + } + if (rdptr =3D=3D wrptr) { + break; + } + buf_req =3D &buf_page->buf_ioreq[rdptr % IOREQ_BUFFER_SLOT_NUM]; req.size =3D 1UL << buf_req->size; req.count =3D 1; req.addr =3D buf_req->addr; @@ -1005,15 +1016,14 @@ static int handle_buffered_iopage(XenIOS req.data_is_ptr =3D 0; qw =3D (req.size =3D=3D 8); if (qw) { - buf_req =3D &state->buffered_io_page->buf_ioreq[ - (state->buffered_io_page->read_pointer + 1) % IOREQ_BUFFER= _SLOT_NUM]; + buf_req =3D &buf_page->buf_ioreq[(rdptr + 1) % + IOREQ_BUFFER_SLOT_NUM]; req.data |=3D ((uint64_t)buf_req->data) << 32; } =20 handle_ioreq(state, &req); =20 - xen_mb(); - state->buffered_io_page->read_pointer +=3D qw ? 2 : 1; + atomic_add(&buf_page->read_pointer, qw + 1); } =20 return req.count; --- a/include/hw/xen/xen_common.h +++ b/include/hw/xen/xen_common.h @@ -370,7 +370,8 @@ static inline void xen_unmap_pcidev(XenX static inline int xen_create_ioreq_server(XenXC xc, domid_t dom, ioservid_t *ioservid) { - int rc =3D xc_hvm_create_ioreq_server(xc, dom, 1, ioservid); + int rc =3D xc_hvm_create_ioreq_server(xc, dom, HVM_IOREQSRV_BUFIOREQ_A= TOMIC, + ioservid); =20 if (rc =3D=3D 0) { trace_xen_ioreq_server_create(*ioservid); --=__Part45719F3F.1__= Content-Type: text/plain; name="qemu-bufioreq-atomic-add.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="qemu-bufioreq-atomic-add.patch" xen/HVM: atomically access pointers in bufioreq handling=0A=0AThe number = of slots per page being 511 (i.e. not a power of two) means=0Athat the = (32-bit) read and write indexes going beyond 2^32 will likely=0Adisturb = operation. The hypervisor side gets I/O req server creation=0Aextended so = we can indicate that we're using suitable atomic accesses=0Awhere needed = (not all accesses to the two pointers really need to be=0Aatomic), = allowing it to atomically canonicalize both pointers when both=0Ahave gone = through at least one cycle.=0A=0AThe Xen side counterpart (which is not a = functional prereq to this=0Achange, albeit a build one) can be found at = e.g.=0Ahttp://lists.xenproject.org/archives/html/xen-devel/2015-06/msg02996= .html=0A=0ASigned-off-by: Jan Beulich =0A=0A--- = a/xen-hvm.c=0A+++ b/xen-hvm.c=0A@@ -981,19 +981,30 @@ static void = handle_ioreq(XenIOState *sta=0A =0A static int handle_buffered_iopage(XenIO= State *state)=0A {=0A+ buffered_iopage_t *buf_page =3D state->buffered_i= o_page;=0A buf_ioreq_t *buf_req =3D NULL;=0A ioreq_t req;=0A = int qw;=0A =0A- if (!state->buffered_io_page) {=0A+ if (!buf_page) = {=0A return 0;=0A }=0A =0A memset(&req, 0x00, sizeof(req));= =0A =0A- while (state->buffered_io_page->read_pointer !=3D state->buffer= ed_io_page->write_pointer) {=0A- buf_req =3D &state->buffered_io_pag= e->buf_ioreq[=0A- state->buffered_io_page->read_pointer % = IOREQ_BUFFER_SLOT_NUM];=0A+ for (;;) {=0A+ uint32_t rdptr =3D = buf_page->read_pointer, wrptr;=0A+=0A+ xen_rmb();=0A+ wrptr = =3D buf_page->write_pointer;=0A+ xen_rmb();=0A+ if (rdptr = !=3D buf_page->read_pointer) {=0A+ continue;=0A+ }=0A+ = if (rdptr =3D=3D wrptr) {=0A+ break;=0A+ }=0A+ = buf_req =3D &buf_page->buf_ioreq[rdptr % IOREQ_BUFFER_SLOT_NUM];=0A = req.size =3D 1UL << buf_req->size;=0A req.count =3D 1;=0A = req.addr =3D buf_req->addr;=0A@@ -1005,15 +1016,14 @@ static int = handle_buffered_iopage(XenIOS=0A req.data_is_ptr =3D 0;=0A = qw =3D (req.size =3D=3D 8);=0A if (qw) {=0A- buf_req = =3D &state->buffered_io_page->buf_ioreq[=0A- (state->buffere= d_io_page->read_pointer + 1) % IOREQ_BUFFER_SLOT_NUM];=0A+ = buf_req =3D &buf_page->buf_ioreq[(rdptr + 1) %=0A+ = IOREQ_BUFFER_SLOT_NUM];=0A req.data |=3D = ((uint64_t)buf_req->data) << 32;=0A }=0A =0A handle_ioreq(s= tate, &req);=0A =0A- xen_mb();=0A- state->buffered_io_page->r= ead_pointer +=3D qw ? 2 : 1;=0A+ atomic_add(&buf_page->read_pointer,= qw + 1);=0A }=0A =0A return req.count;=0A--- a/include/hw/xen/xen_= common.h=0A+++ b/include/hw/xen/xen_common.h=0A@@ -370,7 +370,8 @@ static = inline void xen_unmap_pcidev(XenX=0A static inline int xen_create_ioreq_ser= ver(XenXC xc, domid_t dom,=0A = ioservid_t *ioservid)=0A {=0A- int rc =3D xc_hvm_create_ioreq_server(xc,= dom, 1, ioservid);=0A+ int rc =3D xc_hvm_create_ioreq_server(xc, dom, = HVM_IOREQSRV_BUFIOREQ_ATOMIC,=0A+ = ioservid);=0A =0A if (rc =3D=3D 0) {=0A trace_xen_ioreq_server_= create(*ioservid);=0A --=__Part45719F3F.1__= Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --=__Part45719F3F.1__=--