[Qemu-devel] State of EHCI emulation for QEMU

[Qemu-devel] State of EHCI emulation for QEMU

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1801 bytes --]

Hi all,

as you may know, there is an experimental git repository at

	git://git.kiszka.org/qemu.git ehci

that contains (primarily) David's work on an EHCI model for QEMU,
infrequently merges with latest git by me. As both David and I are short
on time ATM to drive this towards mainline integration, I'd like to
motivate some more contributions.

But first of all a summary of its state:
 - works OK for mass storage device pass-through
 - gives quite decent performance this way (I've once installed a
   complete Linux distro onto an USB disk this way)
 - reported to fail when passing through certain other host device types
   (e.g. the iPhone)
 - lacks integration with UHCI (currently, only EHCI is provided by
   that tree, breaking USB 1.1 devices)

David furthermore provided the following to-dos:
 - lacks periodic frames support
 - lacks isochronous traffic support
 - lacks split transactions support
 - NAK/reload support is kludgy at best
 - throttle interrupt rate based on OS settings

Regarding integration with UHCI: David once posted an RFC patch that
basically establishes the foundation for fixed device-to-controller
assignment [1][2]. Most real implementations do dynamic hand-over
between EHCI and UHCI, however, the spec allows static assignment as
well (which simplifies the implementation in QEMU).

Would be really nice to see this upstream rather sooner than later. I
think the major blocking point is just the companion controller
integration, then we would already be able to use it with USB disks/sticks.

So, if anyone is interested in picking up some open task, every
contribution is welcome!

Jan

[1] http://thread.gmane.org/gmane.comp.emulators.qemu/77130
[2] http://thread.gmane.org/gmane.comp.emulators.qemu/76072


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

Re: [Qemu-devel] State of EHCI emulation for QEMU

[Gerd Hoffmann]

   Hi,

> as you may know, there is an experimental git repository at
>
> 	git://git.kiszka.org/qemu.git ehci

Looking into this right now as I'm busy with various usb issues anyway.
Current state:
   http://cgit.freedesktop.org/spice/qemu/log/?h=usb.1

>   - lacks integration with UHCI (currently, only EHCI is provided by
>     that tree, breaking USB 1.1 devices)

Sort-of fixed (see docs/usb2.txt).

> David furthermore provided the following to-dos:
>   - lacks periodic frames support
>   - lacks isochronous traffic support

--verbose please.  There seems to be some code for that?

>   - lacks split transactions support

That would need a USB 2.0 HUB emulation to be useful.
We lack that one too ;)

>   - NAK/reload support is kludgy at best

--verbose please.

cheers,
   Gerd

Re: [Qemu-devel] State of EHCI emulation for QEMU

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1524 bytes --]

Am 04.12.2010 01:18, Gerd Hoffmann wrote:
>   Hi,
> 
>> as you may know, there is an experimental git repository at
>>
>>     git://git.kiszka.org/qemu.git ehci
> 
> Looking into this right now as I'm busy with various usb issues anyway.
> Current state:
>   http://cgit.freedesktop.org/spice/qemu/log/?h=usb.1
> 
>>   - lacks integration with UHCI (currently, only EHCI is provided by
>>     that tree, breaking USB 1.1 devices)
> 
> Sort-of fixed (see docs/usb2.txt).

Very nice. Some remarks:

- command line must look like this:
        ...
        -drive if=none,id=usbstick,file=/path/to/image   \
        -device usb-storage,bus=ehci.0,drive=usbstick
  ie. register driver device referencing it, and there was a typo id->if

- long-term, "-usb" should become a shorthand for "-device
  usb-uhci,id=uhci -device usb-ehci,id=ehci" (at least for x86)

Unfortunately, msd is not yet magically healed:

  non queue head request in async schedule
  processing error - resetting ehci HC

> 
>> David furthermore provided the following to-dos:
>>   - lacks periodic frames support
>>   - lacks isochronous traffic support
> 
> --verbose please.  There seems to be some code for that?
> 
>>   - lacks split transactions support
> 
> That would need a USB 2.0 HUB emulation to be useful.
> We lack that one too ;)
> 
>>   - NAK/reload support is kludgy at best
> 
> --verbose please.
> 

Those are questions for David.

Jan

PS: Your tree lacks conversion of usb-musb.c.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

Re: [Qemu-devel] State of EHCI emulation for QEMU

[David S. Ahern]

On 12/04/10 02:05, Jan Kiszka wrote:
> Am 04.12.2010 01:18, Gerd Hoffmann wrote:
>>   Hi,
>>
>>> as you may know, there is an experimental git repository at
>>>
>>>     git://git.kiszka.org/qemu.git ehci
>>
>> Looking into this right now as I'm busy with various usb issues anyway.
>> Current state:
>>   http://cgit.freedesktop.org/spice/qemu/log/?h=usb.1
>>

It appears that the import of the ehci code to spice has completely lost
the development history and code contributions - from the original
version by Mark Burkley through the work I've done on it. Would you mind
pulling in the patch history instead of just the final code?

I'll respond in --verbose mode for the other questions later this
weekend when I get more time.

David

Re: [Qemu-devel] State of EHCI emulation for QEMU

[Gerd Hoffmann]

   Hi,

> It appears that the import of the ehci code to spice has completely lost
> the development history and code contributions - from the original
> version by Mark Burkley through the work I've done on it. Would you mind
> pulling in the patch history instead of just the final code?

I've first tried to rebase the ehci branch to latest master exactly to 
keep the history.  Was quite messy with lots of conflicts though, so I 
gave up.  For review & upstream merge having the whole history isn't 
that helpful anyway.

cheers,
   Gerd

Re: [Qemu-devel] State of EHCI emulation for QEMU

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 836 bytes --]

Am 08.12.2010 09:26, Gerd Hoffmann wrote:
>   Hi,
> 
>> It appears that the import of the ehci code to spice has completely lost
>> the development history and code contributions - from the original
>> version by Mark Burkley through the work I've done on it. Would you mind
>> pulling in the patch history instead of just the final code?
> 
> I've first tried to rebase the ehci branch to latest master exactly to
> keep the history.  Was quite messy with lots of conflicts though, so I
> gave up.  For review & upstream merge having the whole history isn't
> that helpful anyway.

I'm was regularly merging master into ehci, and that worked quite well.
For the development phase, it might be nice to keep the history if
possible. But I agree that we need a clean series once upstream
submission is in sight.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

Re: [Qemu-devel] State of EHCI emulation for QEMU

[David S. Ahern]

On 12/08/10 01:32, Jan Kiszka wrote:
> Am 08.12.2010 09:26, Gerd Hoffmann wrote:
>>   Hi,
>>
>>> It appears that the import of the ehci code to spice has completely lost
>>> the development history and code contributions - from the original
>>> version by Mark Burkley through the work I've done on it. Would you mind
>>> pulling in the patch history instead of just the final code?
>>
>> I've first tried to rebase the ehci branch to latest master exactly to
>> keep the history.  Was quite messy with lots of conflicts though, so I
>> gave up.  For review & upstream merge having the whole history isn't
>> that helpful anyway.

Where was the messiness given that most of the changes are to a brand
new file? The biggest change after that is to usb-linux to handle large
requests.

David


> 
> I'm was regularly merging master into ehci, and that worked quite well.
> For the development phase, it might be nice to keep the history if
> possible. But I agree that we need a clean series once upstream
> submission is in sight.
> 
> Jan
> 

Re: [Qemu-devel] State of EHCI emulation for QEMU

[Gerd Hoffmann]

   Hi,

> Where was the messiness given that most of the changes are to a brand
> new file?

 From the "devel -> merge with upstream -> fixup breakage + commit -> 
devel" cycle.  When rebasing *that* you simply don't get a nice + 
bisectable patch series.  Early patches didn't apply cleanly due to 
buildsystem changes.  After fixing up that ehci didn't build.  Of course 
there are likely fixup patches coming later which solve that which I 
could try to find + cherry-pick + squash-in.  But at that point I felt 
it simply isn't worth the trouble given that we would squash it all 
together anyway for patch review + upstream merge.

cheers,
   Gerd

RE: [Qemu-devel] State of EHCI emulation for QEMU

[David Ahern (daahern)]

[-- Attachment #1: Type: text/plain, Size: 1331 bytes --]




-----Original Message-----
From: Gerd Hoffmann [mailto:kraxel@redhat.com]
Sent: Wed 12/8/2010 10:14 AM
To: David Ahern (daahern)
Cc: Jan Kiszka; qemu-devel; Jes Sorensen
Subject: Re: [Qemu-devel] State of EHCI emulation for QEMU
 

   Hi,

> Where was the messiness given that most of the changes are to a brand
> new file?

 From the "devel -> merge with upstream -> fixup breakage + commit -> 
devel" cycle.  When rebasing *that* you simply don't get a nice + 
bisectable patch series.  Early patches didn't apply cleanly due to 
buildsystem changes.  After fixing up that ehci didn't build.  Of course 
there are likely fixup patches coming later which solve that which I 
could try to find + cherry-pick + squash-in.  But at that point I felt 
it simply isn't worth the trouble given that we would squash it all 
together anyway for patch review + upstream merge.

cheers,
   Gerd


New features developed for the kernel are done in a separate git trees. When a feature is ready for inclusion into the main kernel tree, a pull request is sent. That workflow maintains a complete change history for the feature. Take performance events for example: you can go into Linus' git tree and see the complete history of changes. There's no reason the same methodology cannot be done for qemu.

David

[-- Attachment #2: Type: text/html, Size: 1951 bytes --]

Re: [Qemu-devel] State of EHCI emulation for QEMU

[Gerd Hoffmann]

   Hi,

> New features developed for the kernel are done in a separate git trees.
> When a feature is ready for inclusion into the main kernel tree, a pull
> request is sent. That workflow maintains a complete change history for
> the feature. Take performance events for example: you can go into Linus'
> git tree and see the complete history of changes. There's no reason the
> same methodology cannot be done for qemu.

It is done for qemu, pci and block are maintained that way for example. 
  The key difference is that the patches which are accepted into the 
subsystem branches and then are pulled go through a full review @ 
qemu-devel before.

cheers,
   Gerd

Re: [Qemu-devel] State of EHCI emulation for QEMU

[David S. Ahern]

On 12/09/10 06:05, Gerd Hoffmann wrote:
> 
>   Hi,
> 
>> New features developed for the kernel are done in a separate git trees.
>> When a feature is ready for inclusion into the main kernel tree, a pull
>> request is sent. That workflow maintains a complete change history for
>> the feature. Take performance events for example: you can go into Linus'
>> git tree and see the complete history of changes. There's no reason the
>> same methodology cannot be done for qemu.
> 
> It is done for qemu, pci and block are maintained that way for example.
>  The key difference is that the patches which are accepted into the
> subsystem branches and then are pulled go through a full review @
> qemu-devel before.

Yes, I know I've been following qemu and kvm for 3 years now. And there
is no reason the same process cannot be done for usb as a subsystem or
ehci as a feature branch. Jan already has that started.

I realize this is most likely a moot point given that xhci appears
better suited for virtualization than ehci, nonetheless it bugs me that
you are not wanting to take the time to maintain the code change and
sign-off histories.

David

> 
> cheers,
>   Gerd
> 

Re: [Qemu-devel] State of EHCI emulation for QEMU

[Blue Swirl]

On Thu, Dec 9, 2010 at 8:32 PM, David S. Ahern <daahern@cisco.com> wrote:
>
>
> On 12/09/10 06:05, Gerd Hoffmann wrote:
>>
>>   Hi,
>>
>>> New features developed for the kernel are done in a separate git trees.
>>> When a feature is ready for inclusion into the main kernel tree, a pull
>>> request is sent. That workflow maintains a complete change history for
>>> the feature. Take performance events for example: you can go into Linus'
>>> git tree and see the complete history of changes. There's no reason the
>>> same methodology cannot be done for qemu.
>>
>> It is done for qemu, pci and block are maintained that way for example.
>>  The key difference is that the patches which are accepted into the
>> subsystem branches and then are pulled go through a full review @
>> qemu-devel before.
>
> Yes, I know I've been following qemu and kvm for 3 years now. And there
> is no reason the same process cannot be done for usb as a subsystem or
> ehci as a feature branch. Jan already has that started.
>
> I realize this is most likely a moot point given that xhci appears
> better suited for virtualization than ehci, nonetheless it bugs me that
> you are not wanting to take the time to maintain the code change and
> sign-off histories.

The problem with that approach is that the introduction of new
features becomes a single commit. From bisectability point of view
this is bad, especially with big features. So I'm much more interested
in having a lot of small commits than huge commits or merges, even at
the expense of loss of some history information. If we can have both,
for example by doing some kind of partial pull, that would be the best
of both. I may as well be ignorant about some great git features.

Re: [Qemu-devel] State of EHCI emulation for QEMU

[Gerd Hoffmann]

   Hi,

> - command line must look like this:
>          ...
>          -drive if=none,id=usbstick,file=/path/to/image   \
>          -device usb-storage,bus=ehci.0,drive=usbstick
>    ie. register driver device referencing it, and there was a typo id->if

I'll fix.

> - long-term, "-usb" should become a shorthand for "-device
>    usb-uhci,id=uhci -device usb-ehci,id=ehci" (at least for x86)

Hmm, I'd tend to leave '-M pc' as-is and enable ehci by default in the 
upcoming q35 emulation, especially in case both make it into 0.15.

> Unfortunately, msd is not yet magically healed:
>
>    non queue head request in async schedule
>    processing error - resetting ehci HC

Yea, I've seen a flaw in async handling.  Just need to dig some more 
into the code to fix it properly, also the packet workflow (including 
async handling) is one of the spots where the qemu usb subsystem needs 
some work ...

> PS: Your tree lacks conversion of usb-musb.c.

Huh?  'git diff kiszka/master kiszka/ehci hw/usb-musb.c' is empty.

cheers,
   Gerd

Re: [Qemu-devel] State of EHCI emulation for QEMU

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1385 bytes --]

Am 08.12.2010 09:18, Gerd Hoffmann wrote:
>   Hi,
> 
>> - command line must look like this:
>>          ...
>>          -drive if=none,id=usbstick,file=/path/to/image   \
>>          -device usb-storage,bus=ehci.0,drive=usbstick
>>    ie. register driver device referencing it, and there was a typo id->if
> 
> I'll fix.
> 
>> - long-term, "-usb" should become a shorthand for "-device
>>    usb-uhci,id=uhci -device usb-ehci,id=ehci" (at least for x86)
> 
> Hmm, I'd tend to leave '-M pc' as-is and enable ehci by default in the
> upcoming q35 emulation, especially in case both make it into 0.15.

I'm was concerned about the semantic of -usb. If ehci&uhci is going to
be on by default in the future, we should obsolete and finally drop this
switch.

> 
>> Unfortunately, msd is not yet magically healed:
>>
>>    non queue head request in async schedule
>>    processing error - resetting ehci HC
> 
> Yea, I've seen a flaw in async handling.  Just need to dig some more
> into the code to fix it properly, also the packet workflow (including
> async handling) is one of the spots where the qemu usb subsystem needs
> some work ...
> 
>> PS: Your tree lacks conversion of usb-musb.c.
> 
> Huh?  'git diff kiszka/master kiszka/ehci hw/usb-musb.c' is empty.

The breakage comes from one of your patches. Just try to build an ARM
target.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]