Re: [Qemu-devel] [PATCH v2 2/2] xen: add pvUSB backend

[Anthony PERARD <anthony.perard@citrix.com>]

On Wed, May 04, 2016 at 10:25:03AM +0200, Juergen Gross wrote:
> On 03/05/16 17:06, Anthony PERARD wrote:
> > On Thu, Mar 10, 2016 at 04:19:30PM +0100, Juergen Gross wrote:
> >> +static void usbback_bh(void *opaque)
> >> +{
> >> +    struct usbback_info *usbif;
> >> +    struct usbif_urb_back_ring *urb_ring;
> >> +    struct usbback_req *usbback_req;
> >> +    RING_IDX rc, rp;
> >> +    unsigned int more_to_do;
> >> +
> >> +    usbif = opaque;
> >> +    if (usbif->ring_error) {
> >> +        return;
> >> +    }
> >> +
> >> +    urb_ring = &usbif->urb_ring;
> >> +    rc = urb_ring->req_cons;
> >> +    rp = urb_ring->sring->req_prod;
> > 
> > Maybe use atomic_read() here to avoid req_prod been read more than once.
> 
> Hmm. This isn't done in the other backends.
> 
> TBH: what would happen if req_prod would be read multiple times? In the
> worst case we would see a new request from the guest which we would have
> missed without the atomic_read().

If the guest is misbehaving, it maybe could provoke QEMU to handle more
request. I'm not sure.

For this use of atomic_read, I'm mostly refering to XSA-155[1] and a
conversation[2].

[1] http://xenbits.xen.org/xsa/advisory-155.html
[2] <570CFA45.7070504@citrix.com>
    http://lists.xen.org/archives/html/xen-devel/2016-04/msg01696.html


> >> +    xen_rmb(); /* Ensure we see queued requests up to 'rp'. */
> >> +

-- 
Anthony PERARD