From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony Liguori Subject: Re: USB Xen Summit status summary Date: Thu, 02 Feb 2006 08:51:17 -0600 Message-ID: <43E21C65.7020706@us.ibm.com> References: <43E0E196.8090502@comcast.net> <1138820403.8012.113.camel@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1138820403.8012.113.camel@localhost.localdomain> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Harry Butterworth Cc: Nivedita Singhvi , xen-devel List-Id: xen-devel@lists.xenproject.org Harry Butterworth wrote: >Would someone please explain why people thought the xenidc code is >unlikely to get accepted into mainline? > It's the sheer volume of code. The size of your xenidc patch is on par with early versions of the hypervisor in terms of SLOCs. Abstractions without a clear need for the abstraction is heavily frowned upon in Linux. It's just way too much code without a compelling justification for what it's needed. Regards, Anthony Liguori > 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@lists.xensource.com >>http://lists.xensource.com/xen-devel >> >> >> > > >_______________________________________________ >Xen-devel mailing list >Xen-devel@lists.xensource.com >http://lists.xensource.com/xen-devel > > >