Re: [Qemu-devel] [PATCH 1/3] xen: fix quad word bufioreq handling

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 23 November 2016 09:24
> To: qemu-devel@nongnu.org
> Cc: Anthony Perard <anthony.perard@citrix.com>; Paul Durrant
> <Paul.Durrant@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; xen-
> devel <xen-devel@lists.xenproject.org>
> Subject: [PATCH 1/3] xen: fix quad word bufioreq handling
> 
> We should not consume the second slot if it didn't get written yet.
> Normal writers - i.e. Xen - would not update write_pointer between the
> two writes, but the page may get fiddled with by the guest itself, and
> we're better off entering an infinite loop in that case.
> 

Xen would never put QEMU in this situation and the guest can't actually modify the page whilst it's in use, since activation of the IOREQ server removes the page from the guest's p2m so the premise of the patch is not correct.

  Paul

> Reported-by: yanghongke <yanghongke@huawei.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> TBD: Alternatively we could call e.g. hw_error() instead.
> 
> --- a/xen-hvm.c
> +++ b/xen-hvm.c
> @@ -1021,6 +1021,9 @@ static int handle_buffered_iopage(XenIOS
>          xen_rmb();
>          qw = (req.size == 8);
>          if (qw) {
> +            if (rdptr + 1 == wrptr) {
> +                break;
> +            }
>              buf_req = &buf_page->buf_ioreq[(rdptr + 1) %
>                                             IOREQ_BUFFER_SLOT_NUM];
>              req.data |= ((uint64_t)buf_req->data) << 32;
> 
>