| Message ID | 55B0ACFE020000780009457A@prv-mh.provo.novell.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 23 Jul 2015, Jan Beulich wrote: > 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, 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) went in already (commit b7007bc6f9). > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > --- unstable.orig/qemu/upstream/include/hw/xen/xen_common.h 2015-03-31 15:58:04.000000000 +0200 > +++ unstable/qemu/upstream/include/hw/xen/xen_common.h 2015-06-15 08:24:28.000000000 +0200 > @@ -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 = xc_hvm_create_ioreq_server(xc, dom, 1, ioservid); > + int rc = xc_hvm_create_ioreq_server(xc, dom, HVM_IOREQSRV_BUFIOREQ_ATOMIC, > + ioservid); > I am sorry that I noticed this just now: HVM_IOREQSRV_BUFIOREQ_ATOMIC is not defined on older versions of Xen and QEMU still needs to build against them. This change breaks compilation against Xen 4.5 for example. Please define HVM_IOREQSRV_BUFIOREQ_ATOMIC in include/hw/xen/xen_common.h when Xen < 4.6. You might have to introduce a test in configure to detect Xen 4.6, like the others at configure:1869.
Il 23/07/2015 08:59, Jan Beulich ha scritto: > 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, 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) went in already (commit b7007bc6f9). If I understand good a xen 4.6 is needed to build qemu with this patch, if yes probably is good add also a change to configure of require xen >=4.6, instead qemu will fail to build if xen version if lower. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > --- > v2: Adjust description. > > --- a/xen-hvm.c > +++ b/xen-hvm.c > @@ -957,19 +957,30 @@ static void handle_ioreq(XenIOState *sta > > static int handle_buffered_iopage(XenIOState *state) > { > + buffered_iopage_t *buf_page = state->buffered_io_page; > buf_ioreq_t *buf_req = NULL; > ioreq_t req; > int qw; > > - if (!state->buffered_io_page) { > + if (!buf_page) { > return 0; > } > > memset(&req, 0x00, sizeof(req)); > > - while (state->buffered_io_page->read_pointer != state->buffered_io_page->write_pointer) { > - buf_req = &state->buffered_io_page->buf_ioreq[ > - state->buffered_io_page->read_pointer % IOREQ_BUFFER_SLOT_NUM]; > + for (;;) { > + uint32_t rdptr = buf_page->read_pointer, wrptr; > + > + xen_rmb(); > + wrptr = buf_page->write_pointer; > + xen_rmb(); > + if (rdptr != buf_page->read_pointer) { > + continue; > + } > + if (rdptr == wrptr) { > + break; > + } > + buf_req = &buf_page->buf_ioreq[rdptr % IOREQ_BUFFER_SLOT_NUM]; > req.size = 1UL << buf_req->size; > req.count = 1; > req.addr = buf_req->addr; > @@ -981,15 +992,14 @@ static int handle_buffered_iopage(XenIOS > req.data_is_ptr = 0; > qw = (req.size == 8); > if (qw) { > - buf_req = &state->buffered_io_page->buf_ioreq[ > - (state->buffered_io_page->read_pointer + 1) % IOREQ_BUFFER_SLOT_NUM]; > + buf_req = &buf_page->buf_ioreq[(rdptr + 1) % > + IOREQ_BUFFER_SLOT_NUM]; > req.data |= ((uint64_t)buf_req->data) << 32; > } > > handle_ioreq(state, &req); > > - xen_mb(); > - state->buffered_io_page->read_pointer += qw ? 2 : 1; > + atomic_add(&buf_page->read_pointer, qw + 1); > } > > return req.count; > --- unstable.orig/qemu/upstream/include/hw/xen/xen_common.h 2015-03-31 15:58:04.000000000 +0200 > +++ unstable/qemu/upstream/include/hw/xen/xen_common.h 2015-06-15 08:24:28.000000000 +0200 > @@ -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 = xc_hvm_create_ioreq_server(xc, dom, 1, ioservid); > + int rc = xc_hvm_create_ioreq_server(xc, dom, HVM_IOREQSRV_BUFIOREQ_ATOMIC, > + ioservid); > > if (rc == 0) { > trace_xen_ioreq_server_create(*ioservid); >
--- a/xen-hvm.c +++ b/xen-hvm.c @@ -957,19 +957,30 @@ static void handle_ioreq(XenIOState *sta static int handle_buffered_iopage(XenIOState *state) { + buffered_iopage_t *buf_page = state->buffered_io_page; buf_ioreq_t *buf_req = NULL; ioreq_t req; int qw; - if (!state->buffered_io_page) { + if (!buf_page) { return 0; } memset(&req, 0x00, sizeof(req)); - while (state->buffered_io_page->read_pointer != state->buffered_io_page->write_pointer) { - buf_req = &state->buffered_io_page->buf_ioreq[ - state->buffered_io_page->read_pointer % IOREQ_BUFFER_SLOT_NUM]; + for (;;) { + uint32_t rdptr = buf_page->read_pointer, wrptr; + + xen_rmb(); + wrptr = buf_page->write_pointer; + xen_rmb(); + if (rdptr != buf_page->read_pointer) { + continue; + } + if (rdptr == wrptr) { + break; + } + buf_req = &buf_page->buf_ioreq[rdptr % IOREQ_BUFFER_SLOT_NUM]; req.size = 1UL << buf_req->size; req.count = 1; req.addr = buf_req->addr; @@ -981,15 +992,14 @@ static int handle_buffered_iopage(XenIOS req.data_is_ptr = 0; qw = (req.size == 8); if (qw) { - buf_req = &state->buffered_io_page->buf_ioreq[ - (state->buffered_io_page->read_pointer + 1) % IOREQ_BUFFER_SLOT_NUM]; + buf_req = &buf_page->buf_ioreq[(rdptr + 1) % + IOREQ_BUFFER_SLOT_NUM]; req.data |= ((uint64_t)buf_req->data) << 32; } handle_ioreq(state, &req); - xen_mb(); - state->buffered_io_page->read_pointer += qw ? 2 : 1; + atomic_add(&buf_page->read_pointer, qw + 1); } return req.count; --- unstable.orig/qemu/upstream/include/hw/xen/xen_common.h 2015-03-31 15:58:04.000000000 +0200 +++ unstable/qemu/upstream/include/hw/xen/xen_common.h 2015-06-15 08:24:28.000000000 +0200 @@ -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 = xc_hvm_create_ioreq_server(xc, dom, 1, ioservid); + int rc = xc_hvm_create_ioreq_server(xc, dom, HVM_IOREQSRV_BUFIOREQ_ATOMIC, + ioservid); if (rc == 0) { trace_xen_ioreq_server_create(*ioservid);