Re: [Xen-devel] USB Xen Summit status summary

[Harry Butterworth <harry@xxxxxxxxxxxxxxxxxxxxxxxxxxxxx>]

On Wed, 2006-02-01 at 08:28 -0800, Nivedita Singhvi wrote:
> Since Harry Butterworth, who's been working on the USB
> virtualization couldn't attend the Xen Summit, I sat in
> for him and am providing this summary:
> 
> Here were the options under consideration:
> 
> 1. Xen includes current patch Harry had put out, which
>     includes his IDC API.
> 2. Harry puts out a simpler USB driver without his IDC
>     API, written directly to the current Xen bus/store API,
>     and reducing to only features deemed needed for Xen,
>     see if that will be accepted into tree.
> 3. Examine USBoverIP patches (currently in -mm tree)
>     and see if those provide all the functionality we
>     need.
> 4. Throw away everything and have someone else rewrite
>     from scratch.
> 
> There was a brief discussion at the Client (Graphics,
> USB...) session on USB. Ewan and several community folks
> were present. Opinions expressed:
> 
> - Harry's IDC code and current code will not make it
>    into tree as is [consensus]
> - IDC piece very unlikely to be accepted into Linux mainline,
>    hence should not go into Xen tree

Would someone please explain why people thought the xenidc code is
unlikely to get accepted into mainline?  It seems to me that Linux has a
common network stack for the different network card drivers and a common
USB stack for the different USB devices---I don't see why Xen shouldn't
have a common inter-domain communication stack for the different virtual
xen devices.  Is it that there are style issues remaining?  Or perhaps
it's the problem of better integrating the code with the current Xen
infrastructure.

I did spend a week or so trying to work out what the best way to
integrate was and came to the conclusion that the best bet (with minimum
impact to ongoing development) was to leave the existing xen drivers and
API in place and write a new set of drivers to the new API on the side
making the choice of old or new driver selectable in the Kconfig.  After
the new drivers were stable it would be possible to remove the old ones
and then remove the old API and coalesce the infrastructure code.  The
other approach of attempting to make atomic incremental changes to the
existing API and all the drivers is just going to destabilise the tree
and impact out-of-tree development too much.

> - API code is orthogonal to USB driver piece, should be
>    a seperate patch/discussion [consensus]

I needed a stable API to code the driver to which is why I did both
together.  Now the native Xen API is more stable I'm doing a version of
the USB driver with the xenidc code factored out.

> - Best option is (2), rewrite code to leaner, simpler
>    USB driver with minimal functionality, and get that into
>    tree

Factoring out the xenidc code just means duplicating the function it
provided in the usbback and usbfront directories, replacing the xenidc
channel code with a version constructed using the ring macros and making
use of Ewan's state change info to drive connect/disconnect.

Whilst the sum of xenidc and the USB driver code will be reduced I'm not
sure that the USB driver itself is actually going to get that much
smaller with the low-level comms code put back into it.

> - Noone in session had looked at USBoverIp patches
> - There were some good ideas in the IDC API that needed
>    to be discussed/incorporated in Xen
> 
> Other input/questions received:
> - Need to get USB community input

I have had some input on the design which has been helpful. I posted a
code snippet too which led to a discussion about changing Linux to make
it possible to bind to devices rather than interfaces.

> - What were the issues that were left? Are they resolved?
>    If so, what's the current working state of the patch?

No, I have wasted too much time on the Xen APIs to have got around to
the remaining issues yet.  In particular, a correct solution to the
problem of stalling and retrying URBs during error recovery looks fairly
tricky.  Also changes are required in Linux to allow binding to a USB
device rather than an interface so as to allow the correct handling of
devices with multiple configurations.

Having said this, I don't think the remaining USB issues should be the
thing that stops the driver from going into the tree.  My intention was
always just to get the basic function working and then let it get
improved afterwards.  The problem has been syncing up with the xen
driver infrastructure.

The current state of the patch is that when I last posted it it was
working for my test USB devices (USB key, keyboard) against the then
current unstable tree.  Since then the unstable tree has moved on---the
xenbus API has changed slightly, all the header files have been moved
and the Kconfig file has been split up---the version of the driver that
I'm factoring xenidc out of compiles but is currently untested.

> - Keir: rewrite to a simpler driver without the IDC API
>    as the xenbus/store stuff is pretty baked into Xen now,

The xenbus/xenstore APIs are pretty bad from the device driver side.  It
would be a shame to have to live with them indefinitely.

In particular, I think that the following issues need to be addressed:

1) The RING macros and interrupt handlers for inter-domain messages are
too difficult to use. The interrupt handler code to service the ring
queue correctly is pretty subtle and it's totally unnecessary to have
this functionality duplicated in each driver.

2) The low-level memory manipulation in drivers is unnecessary.  Linux
has DMA APIs to do DMA.  Xen should have equivalent high-level APIs for
inter-domain bulk data transport.  I pointed this out at the first Xen
summit and Christian claimed it was impossible because each driver
required specific performance optimisations but I think the xenidc code
demonstrates that it is actually quite easy to achieve without having to
sacrifice performance.

3) Marshalling parameters from xenstore.  This wasn't addressed by
xenidc but it's quite clear that the driver-side xenstore API is not a
good match for the C programming language.  On device create, it would
be much easier to get parameters in a struct.  If it's necessary to
register an XML definition of the struct expected that would be OK.

4) Sequencing operations using xenstore. This is a real problem because
of the level-sensitive nature of watches on the store.  From the
driver's perspective if there's going to be a hot configuration change,
the easiest interface to deal with is receiving the reconfiguration
request and all the parameters in one operation in a struct.  Again,
having to marshall the parameters individually and then work out which
ones have changed and what configuration request that implies is just
too painful.

5) The xenbus otherend_changed state machine API.  This API is largely a
result of having half of the communication channel implementation in
xenbus and the other half in the drivers.  Xenidc demonstrates a better
channel implementation and a better driver API here.

6) I'm not sure that the function in the xenbus state machine which
drives a clean shutdown from the back-end closing is useful because the
FE can just flush if it wants to do a clean shutdown.  On the other
hand, for multi-pathing device drivers it's more normal for FE's to have
to deal cleanly with unclean termination of a channel BE which isn't
catered for in the xenbus state machine.

7) Exposing the implementation details of everything in xenstore isn't
good from the point of view of creating a well defined API for third
parties.  I get the impression this is being addressed by using XML-RPC
to define a public interface to xend which I think is probably a good
thing.

>    might want to do some cleanups in this area.
> - Ian: look at USBoverIP, tried it and it seems to
>    work, but not sure if that's the right solution
> 
> Current Issues/Design questions:
> - Harry's code supports back/front module load/unload
>    (useful during development, if nothing else).

Without working driver domains I was finding module unload of the
back-end useful because it saves a 5 min reboot.  Driver domains are not
quite as useful as module unload because you have to reboot both the FE
and BE domains whereas with module unload you can leave the FE set up.

> - Harry's code is not written to Ewan's last common
>    code pullout API
> - What other code functionality can be dropped in order
>    to make it smaller?

A fair amount of code comes from doing pre-allocation of resources and
resource management.  This means that once the module is initialised it
has all the resources required to guarantee progress is made with normal
operation.  If I drop all the resource management stuff and just do
allocations on the I/O path and allow I/O failures due to ENOMEM then I
can make the code simpler.

I think the current bounded approach is better though.

> 
> [All misrepresentations and errors are mine, I'm operating
>   from memory and on occasion what I heard over the crowd noise :)]
> 
> Hope that initiates the necessary conversation on this...

Thanks Niv, very useful.

> 
> thanks,
> Nivedita
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
> 


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