Re: usbback cleanup code

Re: usbback cleanup code

[Harry Butterworth]

I haven't done any more work on the USB code since the last patch I
posted to xen-devel.  There wasn't any feedback and it wasn't committed.
I think people were too busy with the release.

I have stopped working on USB.  I have done several versions now with no
success at getting it merged.  I think it will be easier to see what is
required once there are some examples of drivers that have been merged
with Linux.

On Sat, 2006-04-29 at 19:43 +0000, sanjay kumar wrote:
> Hi Harry,
> Do you know by what time the USB virtualization code will be commited
> in the xen-unstable tree?
> 
> Thanks,
> Sanjay
> 
> On 4/3/06, Harry Butterworth <harry@hebutterworth.freeserve.co.uk>
> wrote: 
>         The code is supposed to work with isochronous devices but it's
>         untested
>         so probably doesn't. 
>         
>         Harry.
>         
>         
> 
> 
> 
> -- 
> ----------------------
> PhD Student, Georgia Tech
> http://www.cc.gatech.edu/~ksanjay/

Re: Re: usbback cleanup code

[M.A. Williamson]

Harry,

I was able to do a little review of the patch a while back but never had to 
time finish looking through it properly. It looked much closer to 
mergeable, but there still seemed to be quite a lot of abstraction code. I 
think in general, folks were hoping to see a minimum amount of abstraction 
code with the USB driver instead using the driver APIs correctly.

If you don't want to do any more work on it, then maybe it would make a 
good project for somebody. Since the existing code is known to work, and 
has indentified a number of interesting corner cases this shouldn't be too 
hard. If anyone's interested, it'd require you to develop a knowledge of 
the USB protocols, the Linux USB stack, and the Xen driver APIs. The task 
would be a combination of refactoring existing code to satisfy merge 
review, restoring isochronous support (this is still broken, right? Should 
be possible to implement some of the original code to get it up and running 
again), optimising performance and testing with real devices.

Cheers,
Mark

On May 1 2006, Harry Butterworth wrote:

>I haven't done any more work on the USB code since the last patch I
>posted to xen-devel.  There wasn't any feedback and it wasn't committed.
>I think people were too busy with the release.
>
>I have stopped working on USB.  I have done several versions now with no
>success at getting it merged.  I think it will be easier to see what is
>required once there are some examples of drivers that have been merged
>with Linux.
>
>On Sat, 2006-04-29 at 19:43 +0000, sanjay kumar wrote:
>> Hi Harry,
>> Do you know by what time the USB virtualization code will be commited
>> in the xen-unstable tree?
>> 
>> Thanks,
>> Sanjay
>> 
>> On 4/3/06, Harry Butterworth <harry@hebutterworth.freeserve.co.uk>
>> wrote: 
>>         The code is supposed to work with isochronous devices but it's
>>         untested
>>         so probably doesn't. 
>>         
>>         Harry.
>>         
>>         
>> 
>> 
>> 
>> -- 
>> ----------------------
>> PhD Student, Georgia Tech
>> http://www.cc.gatech.edu/~ksanjay/
>
>
>_______________________________________________
>Xen-devel mailing list
>Xen-devel@lists.xensource.com
>http://lists.xensource.com/xen-devel
>

Re: Re: usbback cleanup code

[Harry Butterworth]

On Mon, 2006-05-01 at 15:04 +0100, M.A. Williamson wrote:
> Harry,
> 
> I was able to do a little review of the patch a while back but never had to 
> time finish looking through it properly. It looked much closer to 
> mergeable, but there still seemed to be quite a lot of abstraction code. I 
> think in general, folks were hoping to see a minimum amount of abstraction 
> code with the USB driver instead using the driver APIs correctly.

As far as I'm aware, the USB code is using the driver API correctly
(except possibly for any bugs or where the API may have changed since
the last patch I released).

I think we have a fairly fundamental disconnect about abstraction.  For
me, abstraction is a necessary part of good software engineering.  Just
as I assume you wouldn't write machine code where you could use assembly
and wouldn't write assembly where you could write C, I wouldn't write
code at a low level of abstraction where it was possible to use a higher
level of abstraction.  Abstraction is useful to manage complexity and
useful to write software which is easier to reason about and easier to
modify.

> If you don't want to do any more work on it, then maybe it would make a 
> good project for somebody.

If anyone wants to pick it up, they are more than welcome but I think it
might be worthwhile to wait until some Xen drivers have been
successfully merged upstream with Linux since I suspect that there may
be some more significant churn in the xenbus/xenstore area before this
happens.

> Since the existing code is known to work, and 
> has indentified a number of interesting corner cases this shouldn't be too 
> hard. If anyone's interested, it'd require you to develop a knowledge of 
> the USB protocols, the Linux USB stack, and the Xen driver APIs. The task 
> would be a combination of refactoring existing code to satisfy merge 
> review, restoring isochronous support (this is still broken, right?

Isochronous is implemented but untested as I couldn't get the
isochronous devices I bought for testing working under native Linux.

>  Should 
> be possible to implement some of the original code to get it up and running 
> again), optimising performance and testing with real devices.

The most difficult remaining work is to fix the protocol to correctly
stall URBs during error recovery.  I was involved in some discussion
about this on the USB mailing list and there was a proposal for a
solution but it is fairly tricky.  Stalling URBs is required when there
is a queue of URBs and an URB fails.  If the URBs are not stalled then
they may be submitted to the device out-of-order which is a
data-integrity exposure.

Also I would expect the Linux USB stack to have changed again.

Harry.

> 
> Cheers,
> Mark
> 
> On May 1 2006, Harry Butterworth wrote:
> 
> >I haven't done any more work on the USB code since the last patch I
> >posted to xen-devel.  There wasn't any feedback and it wasn't committed.
> >I think people were too busy with the release.
> >
> >I have stopped working on USB.  I have done several versions now with no
> >success at getting it merged.  I think it will be easier to see what is
> >required once there are some examples of drivers that have been merged
> >with Linux.
> >
> >On Sat, 2006-04-29 at 19:43 +0000, sanjay kumar wrote:
> >> Hi Harry,
> >> Do you know by what time the USB virtualization code will be commited
> >> in the xen-unstable tree?
> >> 
> >> Thanks,
> >> Sanjay
> >> 
> >> On 4/3/06, Harry Butterworth <harry@hebutterworth.freeserve.co.uk>
> >> wrote: 
> >>         The code is supposed to work with isochronous devices but it's
> >>         untested
> >>         so probably doesn't. 
> >>         
> >>         Harry.
> >>         
> >>         
> >> 
> >> 
> >> 
> >> -- 
> >> ----------------------
> >> PhD Student, Georgia Tech
> >> http://www.cc.gatech.edu/~ksanjay/
> >
> >
> >_______________________________________________
> >Xen-devel mailing list
> >Xen-devel@lists.xensource.com
> >http://lists.xensource.com/xen-devel
> >
> 

Re: Re: usbback cleanup code

[Mark Williamson]

> > I was able to do a little review of the patch a while back but never had
> > to time finish looking through it properly. It looked much closer to
> > mergeable, but there still seemed to be quite a lot of abstraction code.
> > I think in general, folks were hoping to see a minimum amount of
> > abstraction code with the USB driver instead using the driver APIs
> > correctly.
>
> As far as I'm aware, the USB code is using the driver API correctly
> (except possibly for any bugs or where the API may have changed since
> the last patch I released).

Sorry, didn't mean to imply it wasn't correctly using it now.  I meant to 
say "directly", which is not at all the same thing.

> I think we have a fairly fundamental disconnect about abstraction.  For
> me, abstraction is a necessary part of good software engineering.  Just
> as I assume you wouldn't write machine code where you could use assembly
> and wouldn't write assembly where you could write C, I wouldn't write
> code at a low level of abstraction where it was possible to use a higher
> level of abstraction.  Abstraction is useful to manage complexity and
> useful to write software which is easier to reason about and easier to
> modify.

Quite.  But it can be a problem where there's just one client, going through 
many layers of abstractions.

There were a lot of files added by your patch which appeared to be utility 
code / abstractions.  This is fine in general, but the other drivers seem to 
get away with much less of this kind of thing without suffering unduly in 
terms of complexity.  I didn't have time to study the code in detail, but I 
wasn't convinced they were all strictly necessary.

> > If you don't want to do any more work on it, then maybe it would make a
> > good project for somebody.
>
> If anyone wants to pick it up, they are more than welcome but I think it
> might be worthwhile to wait until some Xen drivers have been
> successfully merged upstream with Linux since I suspect that there may
> be some more significant churn in the xenbus/xenstore area before this
> happens.

Maybe, but I suspect upstream merge is still quite a long way off.

Personally, I've found that the Xenbus APIs are now sufficiently simple to 
work with that it's very little work to establish a shared memory page (I 
hacked up one very quickly for DCSS), after which you don't have to worry 
about them anylonger.  I don't think keeping up with the control plane is 
prohibitive now, although it was at one stage.

> Isochronous is implemented but untested as I couldn't get the
> isochronous devices I bought for testing working under native Linux.

OK.

> The most difficult remaining work is to fix the protocol to correctly
> stall URBs during error recovery.  I was involved in some discussion
> about this on the USB mailing list and there was a proposal for a
> solution but it is fairly tricky.  Stalling URBs is required when there
> is a queue of URBs and an URB fails.  If the URBs are not stalled then
> they may be submitted to the device out-of-order which is a
> data-integrity exposure.

Any reason not just to fail all the URBs on the queue?  It's not the ideal 
response, but I wouldn't see a need to handle error recovery fully initially, 
although it'd be nice in the long run.

> Also I would expect the Linux USB stack to have changed again.

2.6's APIs do change fairly flexibly, but I don't remember there being any 
major changes to the USB stack for some time now.

Cheers,
Mark

> Harry.
>
> > Cheers,
> > Mark
> >
> > On May 1 2006, Harry Butterworth wrote:
> > >I haven't done any more work on the USB code since the last patch I
> > >posted to xen-devel.  There wasn't any feedback and it wasn't committed.
> > >I think people were too busy with the release.
> > >
> > >I have stopped working on USB.  I have done several versions now with no
> > >success at getting it merged.  I think it will be easier to see what is
> > >required once there are some examples of drivers that have been merged
> > >with Linux.
> > >
> > >On Sat, 2006-04-29 at 19:43 +0000, sanjay kumar wrote:
> > >> Hi Harry,
> > >> Do you know by what time the USB virtualization code will be commited
> > >> in the xen-unstable tree?
> > >>
> > >> Thanks,
> > >> Sanjay
> > >>
> > >> On 4/3/06, Harry Butterworth <harry@hebutterworth.freeserve.co.uk>
> > >> wrote:
> > >>         The code is supposed to work with isochronous devices but it's
> > >>         untested
> > >>         so probably doesn't.
> > >>
> > >>         Harry.
> > >>
> > >>
> > >>
> > >>
> > >>
> > >> --
> > >> ----------------------
> > >> PhD Student, Georgia Tech
> > >> http://www.cc.gatech.edu/~ksanjay/
> > >
> > >_______________________________________________
> > >Xen-devel mailing list
> > >Xen-devel@lists.xensource.com
> > >http://lists.xensource.com/xen-devel

-- 
Dave: Just a question. What use is a unicyle with no seat?  And no pedals!
Mark: To answer a question with a question: What use is a skateboard?
Dave: Skateboards have wheels.
Mark: My wheel has a wheel!

Re: Re: usbback cleanup code

[Anthony Liguori]

FWIW, I took a look at USB over IP.  It looks pretty reasonable to me 
(plus, it's already in -mm).  At this point, I'm convinced we in the 
very least want to share code (even if we don't use IP as the actual 
transport).  It already handles all of the nasty protocol marshaling 
stuff.  No reason to have two bits of code doing the same thing.

Regards,

Anthony Liguori

Mark Williamson wrote:
>>> I was able to do a little review of the patch a while back but never had
>>> to time finish looking through it properly. It looked much closer to
>>> mergeable, but there still seemed to be quite a lot of abstraction code.
>>> I think in general, folks were hoping to see a minimum amount of
>>> abstraction code with the USB driver instead using the driver APIs
>>> correctly.
>>>       
>> As far as I'm aware, the USB code is using the driver API correctly
>> (except possibly for any bugs or where the API may have changed since
>> the last patch I released).
>>     
>
> Sorry, didn't mean to imply it wasn't correctly using it now.  I meant to 
> say "directly", which is not at all the same thing.
>
>   
>> I think we have a fairly fundamental disconnect about abstraction.  For
>> me, abstraction is a necessary part of good software engineering.  Just
>> as I assume you wouldn't write machine code where you could use assembly
>> and wouldn't write assembly where you could write C, I wouldn't write
>> code at a low level of abstraction where it was possible to use a higher
>> level of abstraction.  Abstraction is useful to manage complexity and
>> useful to write software which is easier to reason about and easier to
>> modify.
>>     
>
> Quite.  But it can be a problem where there's just one client, going through 
> many layers of abstractions.
>
> There were a lot of files added by your patch which appeared to be utility 
> code / abstractions.  This is fine in general, but the other drivers seem to 
> get away with much less of this kind of thing without suffering unduly in 
> terms of complexity.  I didn't have time to study the code in detail, but I 
> wasn't convinced they were all strictly necessary.
>
>   
>>> If you don't want to do any more work on it, then maybe it would make a
>>> good project for somebody.
>>>       
>> If anyone wants to pick it up, they are more than welcome but I think it
>> might be worthwhile to wait until some Xen drivers have been
>> successfully merged upstream with Linux since I suspect that there may
>> be some more significant churn in the xenbus/xenstore area before this
>> happens.
>>     
>
> Maybe, but I suspect upstream merge is still quite a long way off.
>
> Personally, I've found that the Xenbus APIs are now sufficiently simple to 
> work with that it's very little work to establish a shared memory page (I 
> hacked up one very quickly for DCSS), after which you don't have to worry 
> about them anylonger.  I don't think keeping up with the control plane is 
> prohibitive now, although it was at one stage.
>
>   
>> Isochronous is implemented but untested as I couldn't get the
>> isochronous devices I bought for testing working under native Linux.
>>     
>
> OK.
>
>   
>> The most difficult remaining work is to fix the protocol to correctly
>> stall URBs during error recovery.  I was involved in some discussion
>> about this on the USB mailing list and there was a proposal for a
>> solution but it is fairly tricky.  Stalling URBs is required when there
>> is a queue of URBs and an URB fails.  If the URBs are not stalled then
>> they may be submitted to the device out-of-order which is a
>> data-integrity exposure.
>>     
>
> Any reason not just to fail all the URBs on the queue?  It's not the ideal 
> response, but I wouldn't see a need to handle error recovery fully initially, 
> although it'd be nice in the long run.
>
>   
>> Also I would expect the Linux USB stack to have changed again.
>>     
>
> 2.6's APIs do change fairly flexibly, but I don't remember there being any 
> major changes to the USB stack for some time now.
>
> Cheers,
> Mark
>
>   
>> Harry.
>>
>>     
>>> Cheers,
>>> Mark
>>>
>>> On May 1 2006, Harry Butterworth wrote:
>>>       
>>>> I haven't done any more work on the USB code since the last patch I
>>>> posted to xen-devel.  There wasn't any feedback and it wasn't committed.
>>>> I think people were too busy with the release.
>>>>
>>>> I have stopped working on USB.  I have done several versions now with no
>>>> success at getting it merged.  I think it will be easier to see what is
>>>> required once there are some examples of drivers that have been merged
>>>> with Linux.
>>>>
>>>> On Sat, 2006-04-29 at 19:43 +0000, sanjay kumar wrote:
>>>>         
>>>>> Hi Harry,
>>>>> Do you know by what time the USB virtualization code will be commited
>>>>> in the xen-unstable tree?
>>>>>
>>>>> Thanks,
>>>>> Sanjay
>>>>>
>>>>> On 4/3/06, Harry Butterworth <harry@hebutterworth.freeserve.co.uk>
>>>>> wrote:
>>>>>         The code is supposed to work with isochronous devices but it's
>>>>>         untested
>>>>>         so probably doesn't.
>>>>>
>>>>>         Harry.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> ----------------------
>>>>> PhD Student, Georgia Tech
>>>>> http://www.cc.gatech.edu/~ksanjay/
>>>>>           
>>>> _______________________________________________
>>>> Xen-devel mailing list
>>>> Xen-devel@lists.xensource.com
>>>> http://lists.xensource.com/xen-devel
>>>>         
>
>   

Re: Re: usbback cleanup code

[Mark Williamson]

> FWIW, I took a look at USB over IP.  It looks pretty reasonable to me
> (plus, it's already in -mm).  At this point, I'm convinced we in the
> very least want to share code (even if we don't use IP as the actual
> transport).  It already handles all of the nasty protocol marshaling
> stuff.  No reason to have two bits of code doing the same thing.

My personal first reaction was that having been designed for a TCP network it 
wouldn't necessarily be a good fit for the shared memory / transient mappings 
model that Xen includes.  However, it seems likely there is infrastructure 
stuff stuff in there that we can leverage, *if* the code handles more special 
cases than the existing Xen USB driver does.

If not, it's arguably not so much of a win for us but Linux should probably 
aim to have only have one core for remote USB - whichever one is more 
complete.

I can certainly imagine that if the "backend" logic is reasonably 
sophisticated, we might want to generalise it into a remote USB provider 
library, and a frontend for Xen and another for IP.  That could be quite 
nice.

When USBoIP was first on the scene it didn't support isochronous (Xen USB for 
Linux 2.4 did), but I understand that it's advanced quite a lot since then.

Any chance of an overview of the basic structure of USBoIP?  What special 
cases does it handle?  How does it deal with protocol marshalling?  It'd be 
nice to have an idea how good a fit the current codebase is.

Cheers,
Mark

> Regards,
>
> Anthony Liguori
>
> Mark Williamson wrote:
> >>> I was able to do a little review of the patch a while back but never
> >>> had to time finish looking through it properly. It looked much closer
> >>> to mergeable, but there still seemed to be quite a lot of abstraction
> >>> code. I think in general, folks were hoping to see a minimum amount of
> >>> abstraction code with the USB driver instead using the driver APIs
> >>> correctly.
> >>
> >> As far as I'm aware, the USB code is using the driver API correctly
> >> (except possibly for any bugs or where the API may have changed since
> >> the last patch I released).
> >
> > Sorry, didn't mean to imply it wasn't correctly using it now.  I meant to
> > say "directly", which is not at all the same thing.
> >
> >> I think we have a fairly fundamental disconnect about abstraction.  For
> >> me, abstraction is a necessary part of good software engineering.  Just
> >> as I assume you wouldn't write machine code where you could use assembly
> >> and wouldn't write assembly where you could write C, I wouldn't write
> >> code at a low level of abstraction where it was possible to use a higher
> >> level of abstraction.  Abstraction is useful to manage complexity and
> >> useful to write software which is easier to reason about and easier to
> >> modify.
> >
> > Quite.  But it can be a problem where there's just one client, going
> > through many layers of abstractions.
> >
> > There were a lot of files added by your patch which appeared to be
> > utility code / abstractions.  This is fine in general, but the other
> > drivers seem to get away with much less of this kind of thing without
> > suffering unduly in terms of complexity.  I didn't have time to study the
> > code in detail, but I wasn't convinced they were all strictly necessary.
> >
> >>> If you don't want to do any more work on it, then maybe it would make a
> >>> good project for somebody.
> >>
> >> If anyone wants to pick it up, they are more than welcome but I think it
> >> might be worthwhile to wait until some Xen drivers have been
> >> successfully merged upstream with Linux since I suspect that there may
> >> be some more significant churn in the xenbus/xenstore area before this
> >> happens.
> >
> > Maybe, but I suspect upstream merge is still quite a long way off.
> >
> > Personally, I've found that the Xenbus APIs are now sufficiently simple
> > to work with that it's very little work to establish a shared memory page
> > (I hacked up one very quickly for DCSS), after which you don't have to
> > worry about them anylonger.  I don't think keeping up with the control
> > plane is prohibitive now, although it was at one stage.
> >
> >> Isochronous is implemented but untested as I couldn't get the
> >> isochronous devices I bought for testing working under native Linux.
> >
> > OK.
> >
> >> The most difficult remaining work is to fix the protocol to correctly
> >> stall URBs during error recovery.  I was involved in some discussion
> >> about this on the USB mailing list and there was a proposal for a
> >> solution but it is fairly tricky.  Stalling URBs is required when there
> >> is a queue of URBs and an URB fails.  If the URBs are not stalled then
> >> they may be submitted to the device out-of-order which is a
> >> data-integrity exposure.
> >
> > Any reason not just to fail all the URBs on the queue?  It's not the
> > ideal response, but I wouldn't see a need to handle error recovery fully
> > initially, although it'd be nice in the long run.
> >
> >> Also I would expect the Linux USB stack to have changed again.
> >
> > 2.6's APIs do change fairly flexibly, but I don't remember there being
> > any major changes to the USB stack for some time now.
> >
> > Cheers,
> > Mark
> >
> >> Harry.
> >>
> >>> Cheers,
> >>> Mark
> >>>
> >>> On May 1 2006, Harry Butterworth wrote:
> >>>> I haven't done any more work on the USB code since the last patch I
> >>>> posted to xen-devel.  There wasn't any feedback and it wasn't
> >>>> committed. I think people were too busy with the release.
> >>>>
> >>>> I have stopped working on USB.  I have done several versions now with
> >>>> no success at getting it merged.  I think it will be easier to see
> >>>> what is required once there are some examples of drivers that have
> >>>> been merged with Linux.
> >>>>
> >>>> On Sat, 2006-04-29 at 19:43 +0000, sanjay kumar wrote:
> >>>>> Hi Harry,
> >>>>> Do you know by what time the USB virtualization code will be commited
> >>>>> in the xen-unstable tree?
> >>>>>
> >>>>> Thanks,
> >>>>> Sanjay
> >>>>>
> >>>>> On 4/3/06, Harry Butterworth <harry@hebutterworth.freeserve.co.uk>
> >>>>> wrote:
> >>>>>         The code is supposed to work with isochronous devices but
> >>>>> it's untested
> >>>>>         so probably doesn't.
> >>>>>
> >>>>>         Harry.
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>> --
> >>>>> ----------------------
> >>>>> PhD Student, Georgia Tech
> >>>>> http://www.cc.gatech.edu/~ksanjay/
> >>>>
> >>>> _______________________________________________
> >>>> Xen-devel mailing list
> >>>> Xen-devel@lists.xensource.com
> >>>> http://lists.xensource.com/xen-devel

Re: Re: usbback cleanup code

[Anthony Liguori]

Mark Williamson wrote:
>> FWIW, I took a look at USB over IP.  It looks pretty reasonable to me
>> (plus, it's already in -mm).  At this point, I'm convinced we in the
>> very least want to share code (even if we don't use IP as the actual
>> transport).  It already handles all of the nasty protocol marshaling
>> stuff.  No reason to have two bits of code doing the same thing.
>>     
>
> My personal first reaction was that having been designed for a TCP network it 
> wouldn't necessarily be a good fit for the shared memory / transient mappings 
> model that Xen includes.  However, it seems likely there is infrastructure 
> stuff stuff in there that we can leverage, *if* the code handles more special 
> cases than the existing Xen USB driver does.
>   

I had similar feelings (which I believe were previously valid).  After 
reading some of the docs/paper on it and then spending some time with 
the source code, I have somewhat mixed feelings.

On the one hand, it's very close to being generic enough for us to use.  
There are some understandable assumption the code makes though about it 
operating on a socket.  With a small bit of refactoring, we ought to 
make it generic enough to handle a transport other than sockets.  
Moreover, I think one would end up with a generally useful abstraction 
for frontend/backend streaming.

> If not, it's arguably not so much of a win for us but Linux should probably 
> aim to have only have one core for remote USB - whichever one is more 
> complete.
>   

 From what I could gather, USBIP is more complete than any of our 
efforts so far.

> I can certainly imagine that if the "backend" logic is reasonably 
> sophisticated, we might want to generalise it into a remote USB provider 
> library, and a frontend for Xen and another for IP.  That could be quite 
> nice.
>   

Much of the backend for USBIP is in userspace which is +1 for me.

> When USBoIP was first on the scene it didn't support isochronous (Xen USB for 
> Linux 2.4 did), but I understand that it's advanced quite a lot since then.
>   

Yes, it definitely supports isochronous devices.

> Any chance of an overview of the basic structure of USBoIP?  What special 
> cases does it handle?  How does it deal with protocol marshalling?  It'd be 
> nice to have an idea how good a fit the current codebase is.
>   

I'll likely not do justice to the actual implementation so instead I'll 
point you to a concise paper that describes it :-)  It's split in a very 
similar way to the typical front-end/back-end Xen device.

I looked into this as it seems like a very clever way to deal with 
things like sound and absolute input devices.  We could emulate HID 
devices within dom0 userspace and it keeps us from having to bother at 
all with implementing frontend/backend drivers for this whole class of 
devices.

http://www.citeulike.org/user/aliguori/article/441674

Regards,

Anthony Liguori

> Cheers,
> Mark
>
>   
>> Regards,
>>
>> Anthony Liguori
>>
>> Mark Williamson wrote:
>>     
>>>>> I was able to do a little review of the patch a while back but never
>>>>> had to time finish looking through it properly. It looked much closer
>>>>> to mergeable, but there still seemed to be quite a lot of abstraction
>>>>> code. I think in general, folks were hoping to see a minimum amount of
>>>>> abstraction code with the USB driver instead using the driver APIs
>>>>> correctly.
>>>>>           
>>>> As far as I'm aware, the USB code is using the driver API correctly
>>>> (except possibly for any bugs or where the API may have changed since
>>>> the last patch I released).
>>>>         
>>> Sorry, didn't mean to imply it wasn't correctly using it now.  I meant to
>>> say "directly", which is not at all the same thing.
>>>
>>>       
>>>> I think we have a fairly fundamental disconnect about abstraction.  For
>>>> me, abstraction is a necessary part of good software engineering.  Just
>>>> as I assume you wouldn't write machine code where you could use assembly
>>>> and wouldn't write assembly where you could write C, I wouldn't write
>>>> code at a low level of abstraction where it was possible to use a higher
>>>> level of abstraction.  Abstraction is useful to manage complexity and
>>>> useful to write software which is easier to reason about and easier to
>>>> modify.
>>>>         
>>> Quite.  But it can be a problem where there's just one client, going
>>> through many layers of abstractions.
>>>
>>> There were a lot of files added by your patch which appeared to be
>>> utility code / abstractions.  This is fine in general, but the other
>>> drivers seem to get away with much less of this kind of thing without
>>> suffering unduly in terms of complexity.  I didn't have time to study the
>>> code in detail, but I wasn't convinced they were all strictly necessary.
>>>
>>>       
>>>>> If you don't want to do any more work on it, then maybe it would make a
>>>>> good project for somebody.
>>>>>           
>>>> If anyone wants to pick it up, they are more than welcome but I think it
>>>> might be worthwhile to wait until some Xen drivers have been
>>>> successfully merged upstream with Linux since I suspect that there may
>>>> be some more significant churn in the xenbus/xenstore area before this
>>>> happens.
>>>>         
>>> Maybe, but I suspect upstream merge is still quite a long way off.
>>>
>>> Personally, I've found that the Xenbus APIs are now sufficiently simple
>>> to work with that it's very little work to establish a shared memory page
>>> (I hacked up one very quickly for DCSS), after which you don't have to
>>> worry about them anylonger.  I don't think keeping up with the control
>>> plane is prohibitive now, although it was at one stage.
>>>
>>>       
>>>> Isochronous is implemented but untested as I couldn't get the
>>>> isochronous devices I bought for testing working under native Linux.
>>>>         
>>> OK.
>>>
>>>       
>>>> The most difficult remaining work is to fix the protocol to correctly
>>>> stall URBs during error recovery.  I was involved in some discussion
>>>> about this on the USB mailing list and there was a proposal for a
>>>> solution but it is fairly tricky.  Stalling URBs is required when there
>>>> is a queue of URBs and an URB fails.  If the URBs are not stalled then
>>>> they may be submitted to the device out-of-order which is a
>>>> data-integrity exposure.
>>>>         
>>> Any reason not just to fail all the URBs on the queue?  It's not the
>>> ideal response, but I wouldn't see a need to handle error recovery fully
>>> initially, although it'd be nice in the long run.
>>>
>>>       
>>>> Also I would expect the Linux USB stack to have changed again.
>>>>         
>>> 2.6's APIs do change fairly flexibly, but I don't remember there being
>>> any major changes to the USB stack for some time now.
>>>
>>> Cheers,
>>> Mark
>>>
>>>       
>>>> Harry.
>>>>
>>>>         
>>>>> Cheers,
>>>>> Mark
>>>>>
>>>>> On May 1 2006, Harry Butterworth wrote:
>>>>>           
>>>>>> I haven't done any more work on the USB code since the last patch I
>>>>>> posted to xen-devel.  There wasn't any feedback and it wasn't
>>>>>> committed. I think people were too busy with the release.
>>>>>>
>>>>>> I have stopped working on USB.  I have done several versions now with
>>>>>> no success at getting it merged.  I think it will be easier to see
>>>>>> what is required once there are some examples of drivers that have
>>>>>> been merged with Linux.
>>>>>>
>>>>>> On Sat, 2006-04-29 at 19:43 +0000, sanjay kumar wrote:
>>>>>>             
>>>>>>> Hi Harry,
>>>>>>> Do you know by what time the USB virtualization code will be commited
>>>>>>> in the xen-unstable tree?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Sanjay
>>>>>>>
>>>>>>> On 4/3/06, Harry Butterworth <harry@hebutterworth.freeserve.co.uk>
>>>>>>> wrote:
>>>>>>>         The code is supposed to work with isochronous devices but
>>>>>>> it's untested
>>>>>>>         so probably doesn't.
>>>>>>>
>>>>>>>         Harry.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> ----------------------
>>>>>>> PhD Student, Georgia Tech
>>>>>>> http://www.cc.gatech.edu/~ksanjay/
>>>>>>>               
>>>>>> _______________________________________________
>>>>>> Xen-devel mailing list
>>>>>> Xen-devel@lists.xensource.com
>>>>>> http://lists.xensource.com/xen-devel
>>>>>>             

Re: Re: usbback cleanup code

[Harry Butterworth]

On Mon, 2006-05-01 at 20:32 +0100, Mark Williamson wrote:

> There were a lot of files added by your patch which appeared to be utility 
> code / abstractions.  This is fine in general, but the other drivers seem to 
> get away with much less of this kind of thing without suffering unduly in 
> terms of complexity.

I don't think the other drivers are expressed in a way that allows the
reader to see that they are obviously correct.  I found them fairly
difficult to read and I think they could be improved with some
additional internal structure.

> I didn't have time to study the code in detail, but I 
> wasn't convinced they were all strictly necessary.

This feedback isn't specific enough to be useful for me to improve the
patches to your liking.

> > The most difficult remaining work is to fix the protocol to correctly
> > stall URBs during error recovery.  I was involved in some discussion
> > about this on the USB mailing list and there was a proposal for a
> > solution but it is fairly tricky.  Stalling URBs is required when there
> > is a queue of URBs and an URB fails.  If the URBs are not stalled then
> > they may be submitted to the device out-of-order which is a
> > data-integrity exposure.
> 
> Any reason not just to fail all the URBs on the queue?

There was some discussion about this on the USB mailing list.
Apparently the URBs on the control endpoint can have more than one
source (presumably the USB stack and the USB driver) and failure for one
client shouldn't impact another client.

Harry.

Re: Re: usbback cleanup code

[Mark Williamson]

> I don't think the other drivers are expressed in a way that allows the
> reader to see that they are obviously correct.  I found them fairly
> difficult to read and I think they could be improved with some
> additional internal structure.

Agreed, up to a point.  Certainly Xenbus interfacing used to feel a little 
like voodoo, but I do believe it has improved.  It can probably get better 
still.  The data-path part of the other drivers generally seemed reasonably 
readable to me, although I admit it did take a bit of head scratching to 
understand why sometimes.

What I do like about those drivers is that because the code volume is small 
and they have very few files, it's quite easy to see how things fit together, 
if not clearly understand all the nuances.

> > I didn't have time to study the code in detail, but I
> > wasn't convinced they were all strictly necessary.
>
> This feedback isn't specific enough to be useful for me to improve the
> patches to your liking.

No, sorry about that.

I might be able to find time to look through the patch in more detail to see 
what specifically I think needs doing.  If I gave more specific feedback, 
would you be willing to take another pass?  Doing so ought to move us a step 
forwards, at least.

> > Any reason not just to fail all the URBs on the queue?
>
> There was some discussion about this on the USB mailing list.
> Apparently the URBs on the control endpoint can have more than one
> source (presumably the USB stack and the USB driver) and failure for one
> client shouldn't impact another client.

Good point!  It's interesting...  Kudos for extracting this arcane wisdom from 
the USB developers  ;-)  It makes sense...  the USB stack can send generic 
enumeration, address setting, etc messages to the device but it's also 
perfectly valid for the client driver to use control messages - possibly 
*only* control messages, e.g. for a HID device.

I'd suggest that failing them all is fine for now, and then look at improving 
it later.  I don't imagine it's going to be a common scenario, and would 
suggest that we're better having an in-tree driver that can be improved as we 
go.

Cheers,
Mark

-- 
Dave: Just a question. What use is a unicyle with no seat?  And no pedals!
Mark: To answer a question with a question: What use is a skateboard?
Dave: Skateboards have wheels.
Mark: My wheel has a wheel!

Re: Re: usbback cleanup code

[Harry Butterworth]

On Tue, 2006-05-02 at 12:31 +0100, Mark Williamson wrote:
> I might be able to find time to look through the patch in more detail to see 
> what specifically I think needs doing.  If I gave more specific feedback, 
> would you be willing to take another pass?  Doing so ought to move us a step 
> forwards, at least.

If my team lead asks me to, I will do another pass.  Your feedback will
be useful in any case.

> I'd suggest that failing them all is fine for now, and then look at improving 
> it later.  I don't imagine it's going to be a common scenario, and would 
> suggest that we're better having an in-tree driver that can be improved as we 
> go.

The current driver was working well enough to go into the tree.  The
protocol issues are just a theoretical exposure, I hadn't actually seen
them occurring.

> 
> Cheers,
> Mark
> 

Re: Re: usbback cleanup code

[Harry Butterworth]

On Tue, 2006-05-02 at 12:31 +0100, Mark Williamson wrote:

> I might be able to find time to look through the patch in more detail to see 
> what specifically I think needs doing.  If I gave more specific feedback, 
> would you be willing to take another pass?  Doing so ought to move us a step 
> forwards, at least.

OK.  If you come back with a specific list of things to fix for
inclusion I am allowed to do another pass.

Harry.