Xen devel - Dec 2005 - latest USB code including Xenidc documentation

Here again is the latest USB code split into the xenidc patch for
interdomain communication and the usb patch which depends on it.

The most significant change in this version of the code is the addition
of a chapter describing the Xenidc API to the interface document
docs/src/interface.tex.  I think there is now sufficient documentation
for people to have a look and start some discussion about whether a
high-level interdomain communication API like xenidc would be useful and
if so, whether the API presented would make a good starting point.

The USB code hasn''t changed much apart from some style cleanups (mainly
shortening some names to help with formatting) and it still passes the
usual sniff test of mounting a filesystem on a USB key.

The issues that I know about that remain with this code are as follows:

o - The USB protocol requires stalling the URB queue during error
conditions.  This isn''t done correctly yet which makes the code unsafe
for write access during error recovery.  I posted about this on the
usb-devel list and got some useful information.  The conclusion was that
a reasonable strategy on encountering an error would be to unlink all
the URBs outstanding for an endpoint in the backend and hold onto them
and queue any URBs received subsequently until the frontend driver had
had an opportunity to unlink any of the URBs that it wanted to unlink.
The remaining URBs would then be resubmitted.  I made a first attempt at
implementing this before discovering that the boundary conditions around
device resets and hotplug/removal were more subtle than I thought so I
reverted it.  I think I need to study the USB spec before I can attempt
this again.

o - An issue raised on the usb-devel mailing list was that there are a
few more commands which need to be implemented in the back end.  The
most difficult of which is "set configuration".  Set configuration
changes the interfaces a device presents and can''t currently be
performed by the usb backend driver which is binding to the interfaces
of a specific configuration that has already been set.  The solution
seems to be to change the Linux USB stack to allow binding to the device
rather than the interfaces.  This would make it possible to support set
configuration and would make for a cleaner separation between the
actions performed by the back and front ends.  Without the Linux
changes, the current implementation is limited to devices for which set
configuration is not required.  Devices with multiple configurations
probably won''t work properly.

o - The resource pools are currently only allocating the minimum
resources required to avoid deadlock so performance will be resource
constrained.  This can be fixed either by implementing a larger static
resource allocation or improving the buffer_resource_provider to
implement a dynamic shared resource pool.  Either is fairly easy.

o - You no longer need to specify swiotlb=force because xen has a small
swiotlb turned on by default.  If you explicitly turn the swiotlb off,
the USB code will still be broken.

o - The USB-specific part of the interdomain protocol is using polling
for device discovery which is inefficient.  I looked into fixing this
and it won''t be too hard to change the protocol so that the BE sends a
message to the FE to kick the FE into re-probing the BE.  This will make
the BE device state machine a bit more complicated.

o - Design documentation for the USB driver is required.

o - We might not want to keep the xenidc code.  There is now some
documentation for the xenidc API so you can have a look to see if you
think it is worth keeping.

o - Isochronous URBs are untested.

o - My test machine is USB 1.2 and I''ve not yet tested with USB 2.0.

o - Suspend and resume is untested.  Domain migration is untested.

o - I''ve only ever tried to build the code for X86 and never X86_64.

o - Automated tests integrated with the xm-test framework would be good.

o - Have now run some of the design issues by the USB mailing list.
Need to continue to do more of this.

Signed-off-by: Harry Butterworth <butterwo@uk.ibm.com>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Fri, Dec 16, 2005 at 04:51:07PM +0000, Harry Butterworth wrote:
> Here again is the latest USB code split into the xenidc patch for
> interdomain communication and the usb patch which depends on it.
Hi Harry,

Would it be possible to split this up into small incremental changes,
each of which could be reviewed and discussed independently? I''ve
tried to review both the XenIDC and the USB code, but my brain kept
returning -E2BIG. Splitting it on a per file basis doesn''t help
either, because reviewing one requires reading all of the rest.

There''s no arguing that splitting such a large piece of code into
small *incremental* changes is a pain; but I think it''s the only
realistic way to get review and acceptance, and I''d love to have USB
support in Xen. The process of splitting it up and making sure the
system keeps working also tends to substantially improve the code
quality.

Cheers,
Muli
-- 
Muli Ben-Yehuda
http://www.mulix.org | http://mulix.livejournal.com/

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

I can do the 17 patches I posted before.  They were smaller and
incremental in that the code would compile cleanly when patches 1-n were
applied for n = 1 to 17.

Is that what you are looking for?

Harry

On Fri, 2005-12-16 at 19:01 +0200, Muli Ben-Yehuda
wrote:
> On Fri, Dec 16, 2005 at 04:51:07PM +0000, Harry Butterworth wrote:
> 
> > Here again is the latest USB code split into the xenidc patch for
> > interdomain communication and the usb patch which depends on it.
> 
> Hi Harry,
> 
> Would it be possible to split this up into small incremental changes,
> each of which could be reviewed and discussed independently? I''ve
> tried to review both the XenIDC and the USB code, but my brain kept
> returning -E2BIG. Splitting it on a per file basis doesn''t help
> either, because reviewing one requires reading all of the rest.
> 
> There''s no arguing that splitting such a large piece of code into
> small *incremental* changes is a pain; but I think it''s the only
> realistic way to get review and acceptance, and I''d love to have
USB
> support in Xen. The process of splitting it up and making sure the
> system keeps working also tends to substantially improve the code
> quality.
> 
> Cheers,
> Muli

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Fri, Dec 16, 2005 at 05:23:51PM +0000, Harry Butterworth wrote:
> I can do the 17 patches I posted before.  They were smaller and
> incremental in that the code would compile cleanly when patches 1-n were
> applied for n = 1 to 17.
> 
> Is that what you are looking for?
I''m afraid not. What I would like to see (and I imagine others as
well) is a set of incremental patches, each of which is a good step
forward on its own. Continuing to compile cleanly is a prerequisite,
but not sufficient to be useful.

Do the USB drivers need all of the current XenIDC code to be
functional, or can you post a tiny USB frontend/backend driver that
uses the minimal ammount of XenIDC code (or none at all at first, that
would be even better)? If that''s possible, the next step would be to
add useful IDC and USB driver functionality one small incremental
patch at a time. That makes the review and submission process much
easier, since each patch can be discussed on its own merits and
accepted or reworked. The way the code stands right now, it''s an all
or nothing deal.

Cheers,
Muli
-- 
Muli Ben-Yehuda
http://www.mulix.org | http://mulix.livejournal.com/


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Fri, 2005-12-16 at 19:38 +0200, Muli Ben-Yehuda
wrote:
> On Fri, Dec 16, 2005 at 05:23:51PM +0000, Harry Butterworth wrote:
> 
> > I can do the 17 patches I posted before.  They were smaller and
> > incremental in that the code would compile cleanly when patches 1-n
were
> > applied for n = 1 to 17.
> > 
> > Is that what you are looking for?
> 
> I''m afraid not. What I would like to see (and I imagine others as
> well) is a set of incremental patches, each of which is a good step
> forward on its own. Continuing to compile cleanly is a prerequisite,
> but not sufficient to be useful.
The 17 patches are each fairly self contained and provided an
incremental bit of function each time which is useful in its own right.

So, for example by the time you get to patch 3 you have a framework for
supporting different types of local memory and different types of
interdomain transfers.
> Do the USB drivers need all of the current XenIDC code to be
> functional,
Yes.  I only implemented the bits of xenidc that were required to
support the USB driver.  If you remove any of it, the USB driver will
cease to function (or in one instance slow down :-).
>  or can you post a tiny USB frontend/backend driver that
> uses the minimal ammount of XenIDC code (or none at all at first, that
> would be even better)?
The code won''t work without some way to set up the comms channel and
get
the command blocks and data from the FE to the BE.  At the moment this
is xenidc which is expressed in a way that is reusable between different
drivers and extensible to support different kinds of data transfers.
The whole point of the xenidc patch is to demonstrate one kind of
interdomain communication infrastructure we could have if we wanted.

I could factor out the xenidc code and put all the comms code back into
the USB driver so it was like the block and net drivers and contained
its own copy of bespoke code for messaging, data transfer and set-up but
this would entirely defeat the point of the xenidc patch.

I''m happy to do that for the USB driver if that''s what people
decide
they want but I''m reluctant to do it without people having first
considered the idea of having a higher level interdomain communication
API and thought about xenidc as a possible option.
> If that''s possible, the next step would be to
> add useful IDC and USB driver functionality one small incremental
> patch at a time. That makes the review and submission process much
> easier, since each patch can be discussed on its own merits and
> accepted or reworked. The way the code stands right now, it''s an
all
> or nothing deal.
I think you are looking at it the wrong way.  The code isn''t really all
that important immediately.  The most important question is do we want
the split drivers to use the low level grant-table and event-channel
APIs directly and each have their own version of code to do the required
set-up and tear-down, manipulations for buffers bigger than individual
pages and all that cruft or do we want a higher-level API that is more
convenient to use, less coupled to the intricate implementation details
and therefore better for maintenance and IMHO generally a better way.

Xenidc, the API documentation and the working USB driver serve as an
example of the kind of infrastructure we could have.  Whether we want
the end goal or not should decide how I prepare the code for
integration.  If we don''t want the end goal then I can factor all the
xenidc stuff out.  If the end goal looks like a good thing, we can
discuss how to best incorporate it incrementally.

Did you read the API documentation?  Do you think it''s worthwhile to
try
to achieve a high level inter-domain communication API which allows
different split drivers to reuse the channel set-up, messaging,
interrupt handling, flow-control and bulk data transfer code?

Harry.
> 
> Cheers,
> Muli
-- 
Harry Butterworth <harry@hebutterworth.freeserve.co.uk>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Harry Butterworth wrote:
>I''m happy to do that for the USB driver if that''s what
people decide
>they want but I''m reluctant to do it without people having first
>considered the idea of having a higher level interdomain communication
>API and thought about xenidc as a possible option.
>  
>
I think Muli''s point (which I agree with) is that the question of a 
higher level interdomain communication API is orthagonal to the USB 
driver and that submitting the two at once makes it difficult to review 
because of sheer size.

In fact, what I would really like to see is a before and after with the 
USB driver (one version that uses the current mechanism and uses 
xenidc).  This would be a good way to evaluate how much complexity 
xenidc introduces/removes from the drivers.  xenidc is a big decision 
because it means yet another rewrite of all the device drivers (we''ve 
gone through how many in the past year already :-)).

Regards,

Anthony Liguori
>  
>
>>Cheers,
>>Muli
>>    
>>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Fri, 2005-12-16 at 14:13 -0600, Anthony Liguori
wrote:
> Harry Butterworth wrote:
> 
> >I''m happy to do that for the USB driver if that''s
what people decide
> >they want but I''m reluctant to do it without people having
first
> >considered the idea of having a higher level interdomain communication
> >API and thought about xenidc as a possible option.
> >  
> >
> I think Muli''s point (which I agree with) is that the question of
a
> higher level interdomain communication API is orthagonal to the USB 
> driver and that submitting the two at once makes it difficult to review 
> because of sheer size.
To construct a good API you really need at least one example of a client
to evaluate the API against and the patches are split into two (or 17
depending) with all the xenidc stuff in one and the usb stuff in the
other.
> In fact, what I would really like to see is a before and after with the 
> USB driver (one version that uses the current mechanism and uses 
> xenidc).  This would be a good way to evaluate how much complexity 
> xenidc introduces/removes from the drivers.  xenidc is a big decision 
> because it means yet another rewrite of all the device drivers
(we''ve
> gone through how many in the past year already :-)).
We could have saved most of those rewrites if we''d had some open
discussion about the requirements and the approach earlier on.

Xenidc can coexist with the current code so an immediate rewrite of the
other drivers isn''t a pre-req.

Now that the driver interface in the tree is more stable I can do an
example of the USB driver with all the xendic code merged into it and
factored out as much as possible for comparison.

I''m kind of surprised that it doesn''t seem to be possible to
have a
discussion at a higher level of abstraction than the code itself which
seems painfully inefficient to me but then I''m new to open source so I
expect it will all make sense eventually :-)

Thanks for your help.

Harry.
> 
> Regards,
> 
> Anthony Liguori
> 
> >  
> >
> >>Cheers,
> >>Muli
> >>    
> >>
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
> 
-- 
Harry Butterworth <harry@hebutterworth.freeserve.co.uk>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Fri, Dec 16, 2005 at 07:00:28PM +0000, Harry Butterworth wrote:
> Did you read the API documentation?
Not yet.
> Do you think it''s worthwhile to try
> to achieve a high level inter-domain communication API which allows
> different split drivers to reuse the channel set-up, messaging,
> interrupt handling, flow-control and bulk data transfer code?
Yes. But I''d like to see it developed and integrated piece by piece,
as a natural evolution of the existing code.

Cheers,
Muli
-- 
Muli Ben-Yehuda
http://www.mulix.org | http://mulix.livejournal.com/


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Sat, 2005-12-17 at 10:16 +0200, Muli Ben-Yehuda wrote:
> > Do you think it''s worthwhile to try
> > to achieve a high level inter-domain communication API which allows
> > different split drivers to reuse the channel set-up, messaging,
> > interrupt handling, flow-control and bulk data transfer code?
> 
> Yes. But I''d like to see it developed and integrated piece by
piece,
> as a natural evolution of the existing code.
If you churn the existing code by integrating a lot of incremental
changes across all the drivers it may introduce significant instability
and prevent other development from proceeding.  An alternative approach
would be to provide the infrastructure on the side and port the drivers
to it one by one.  That way the old and new versions of drivers could
coexist in the tree which would allow other developers to use the old,
stable set until the new ones were finished.

Just a thought.

-- 
Harry Butterworth <harry@hebutterworth.freeserve.co.uk>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel