From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: [PATCH v2 2/2] xen: add pvUSB backend Date: Fri, 6 May 2016 06:57:44 +0200 Message-ID: <572C2448.7010806@suse.com> References: <1457623170-30896-1-git-send-email-jgross@suse.com> <1457623170-30896-3-git-send-email-jgross@suse.com> <20160503150614.GG1885@perard.uk.xensource.com> <5729B1DF.1070108@suse.com> <20160505101355.GJ1885@perard.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20160505101355.GJ1885@perard.uk.xensource.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org Sender: "Qemu-devel" To: Anthony PERARD Cc: xen-devel@lists.xensource.com, kraxel@redhat.com, qemu-devel@nongnu.org, stefano.stabellini@eu.citrix.com, konrad.wilk@oracle.com List-Id: xen-devel@lists.xenproject.org On 05/05/16 12:13, Anthony PERARD wrote: > 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. I don't think this would add any risk to dom0. A misbehaving guest writing arbitrary values to ->req_prod could influence qemu activity in just the same way regardless whether atomic_read() is used on qemu side or not. The only difference would be that with atomic_read() the additional qemu activity would be delayed until the next invocation of the function. > For this use of atomic_read, I'm mostly refering to XSA-155[1] and a > conversation[2]. The main problem with XSA-155 was modification of the request's contents by the guest after verification by the backend happened. This is not related to reading the producer's ring index. I should use RING_COPY_REQUEST(), however. Juergen