Ian Jackson
2009-Mar-30 15:02 UTC
[Xen-devel] [PATCH] usbfront: do not assume sequentially mapped pages
xenhcd_gnttab_map in usbfront-q.c looks up the mfn of the start of the
usb transfer buffer. But the buffer may span several pages, and the
current code simply increments the obtained mfn. Needless to say this
is an unwarranted assumption. It causes large transfers to be
corrupted and/or to overwrite other parts of memory.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
diff -r 87c84f7dd850 drivers/xen/usbfront/usbfront-q.c
--- a/drivers/xen/usbfront/usbfront-q.c Fri Mar 20 09:00:58 2009 +0000
+++ b/drivers/xen/usbfront/usbfront-q.c Fri Mar 27 17:53:12 2009 +0100
@@ -106,12 +106,15 @@ static inline void xenhcd_gnttab_map(str
unsigned int bytes;
int i;
- page = virt_to_page(addr);
- buffer_pfn = page_to_phys(page) >> PAGE_SHIFT;
- offset = offset_in_page(addr);
len = length;
for(i = 0;i < nr_pages;i++){
+ BUG_ON(!len);
+
+ page = virt_to_page(addr);
+ buffer_pfn = page_to_phys(page) >> PAGE_SHIFT;
+ offset = offset_in_page(addr);
+
bytes = PAGE_SIZE - offset;
if(bytes > len)
bytes = len;
@@ -123,9 +126,8 @@ static inline void xenhcd_gnttab_map(str
seg[i].offset = (uint16_t)offset;
seg[i].length = (uint16_t)bytes;
- buffer_pfn++;
+ addr += bytes;
len -= bytes;
- offset = 0;
}
}
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Noboru Iwamatsu
2009-Apr-01 06:05 UTC
Re: [Xen-devel] [PATCH] usbfront: do not assume sequentially mapped pages
Hi, Oh, I had a misunderstanding about the continuity of buffer pages. This caused webcam''s image to corrupt. PVUSB applied to this change can work well with the following webcams. * Creative Labs: WebCam 3 USB (with ov511 driver) * Creative Labs: WebCam NX pro (with gspca driver) Now, Webcam works! Thanks, Noboru Ian Jackson wrote:> xenhcd_gnttab_map in usbfront-q.c looks up the mfn of the start of the > usb transfer buffer. But the buffer may span several pages, and the > current code simply increments the obtained mfn. Needless to say this > is an unwarranted assumption. It causes large transfers to be > corrupted and/or to overwrite other parts of memory. > > Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> > > diff -r 87c84f7dd850 drivers/xen/usbfront/usbfront-q.c > --- a/drivers/xen/usbfront/usbfront-q.c Fri Mar 20 09:00:58 2009 +0000 > +++ b/drivers/xen/usbfront/usbfront-q.c Fri Mar 27 17:53:12 2009 +0100 > @@ -106,12 +106,15 @@ static inline void xenhcd_gnttab_map(str > unsigned int bytes; > int i; > > - page = virt_to_page(addr); > - buffer_pfn = page_to_phys(page) >> PAGE_SHIFT; > - offset = offset_in_page(addr); > len = length; > > for(i = 0;i < nr_pages;i++){ > + BUG_ON(!len); > + > + page = virt_to_page(addr); > + buffer_pfn = page_to_phys(page) >> PAGE_SHIFT; > + offset = offset_in_page(addr); > + > bytes = PAGE_SIZE - offset; > if(bytes > len) > bytes = len; > @@ -123,9 +126,8 @@ static inline void xenhcd_gnttab_map(str > seg[i].offset = (uint16_t)offset; > seg[i].length = (uint16_t)bytes; > > - buffer_pfn++; > + addr += bytes; > len -= bytes; > - offset = 0; > } > } > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2009-Apr-01 16:00 UTC
Re: [Xen-devel] [PATCH] usbfront: do not assume sequentially mapped pages
Noboru Iwamatsu writes ("Re: [Xen-devel] [PATCH] usbfront: do not assume
sequentially mapped pages"):> Oh, I had a misunderstanding about the continuity of buffer pages.
> This caused webcam''s image to corrupt.
>
> PVUSB applied to this change can work well with the following webcams.
That''s great. I''ve been trying it out with a Logitech
Quickcam Sphere
AF (with the Linux uvc driver from v4l) and there still seem to be
problems, which look like they''re related to the v4l driver''s
attempts
to do direct access to dma pages and/or mmap pages into client process
address space.
> * Creative Labs: WebCam 3 USB (with ov511 driver)
> * Creative Labs: WebCam NX pro (with gspca driver)
You might like to try a v4l uvc driver and see if it works for you.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Noboru Iwamatsu
2009-Apr-06 09:03 UTC
Re: [Xen-devel] [PATCH] usbfront: do not assume sequentially mapped pages
Ian, > You might like to try a v4l uvc driver and see if it works for you. Sorry for the late reply. I''ve been trying USB2.0 UVC webcam too, but I have not succeeded yet. I dumped urbs and saw no errors in isoc-urbs. My testing environment is the following, UVC driver: I checked out the recent driver from http://linuxtv.org/hg/~pinchartl/uvcvideo/ Webcam: * Logitech QuickCam S5500 (046d:09a1) * Logitech QuickCam E3500 (046d:09a4) App: * luvcview (ver. 20070512) Results: luvcview outputs "Ignoring empty buffer ..." and exits with errors. Regards, Noboru _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2009-Apr-07 16:40 UTC
Re: [Xen-devel] [PATCH] usbfront: do not assume sequentially mapped pages
Noboru Iwamatsu writes ("Re: [Xen-devel] [PATCH] usbfront: do not assume
sequentially mapped pages"):> Ian,
> > You might like to try a v4l uvc driver and see if it works for you.
>
> Sorry for the late reply.
>
> I''ve been trying USB2.0 UVC webcam too, but I have not succeeded
yet.
> I dumped urbs and saw no errors in isoc-urbs.
I didn''t see errors. I ran uvccapture (as in Debian lenny), which
seems to be the most simple test program available, and it segfaults
while accessing what is supposed to be a buffer full of video data
provided by mmapping the kernel''s UVC driver.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Noboru Iwamatsu
2009-Apr-08 02:15 UTC
Re: [Xen-devel] [PATCH] usbfront: do not assume sequentially mapped pages
Ian, I tried out uvccapture-0.5, and I''m certain that we have the same problem. When uvccapture had started memcpy-ing from the v4l2_buffer, the v4l2_buffer was not filled yet. (in line 294 of v4l2uvc.c, vd->buf.bytesused is less than HEADERFRAME1) I''m uncertain whether this problem results from the isoc-transfer timing or v4l relating. Regards, Noboru Ian Jackson wrote:> Noboru Iwamatsu writes ("Re: [Xen-devel] [PATCH] usbfront: do not assume sequentially mapped pages"): >> Ian, >> > You might like to try a v4l uvc driver and see if it works for you. >> >> Sorry for the late reply. >> >> I''ve been trying USB2.0 UVC webcam too, but I have not succeeded yet. >> I dumped urbs and saw no errors in isoc-urbs. > > I didn''t see errors. I ran uvccapture (as in Debian lenny), which > seems to be the most simple test program available, and it segfaults > while accessing what is supposed to be a buffer full of video data > provided by mmapping the kernel''s UVC driver. > > Ian._______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel