Re: [Qemu-devel] "usb: uhci: Look up queue by address, not token"

[Hans de Goede]

Hi Jan,

I just saw your $subject patch in Gerd's usb-next tree, and I've a question
about it. The token should be enough to uniquely identify a device + ep,
and unless a guest uses multiple qhs for a singe ep, that _should_ be enough.

So I'm wondering if you can give a (short) description of exactly what you
were seeing which is fixed by this patch ? And were you seeing this with
1.2, or with master ?

The reason why I want to know, is that identifying the queue by qh has a
disadvantage, to be precise I believe the following can then happen:

1) The guest queues up multiple requests for a queue
2) We execute one, go async
3) The guest changes it mind an unlinks the qh
4) The guest will think the queue is cancelled after frindex has
changed by 1, but we keep it around for 64 frames (which sucks,
and I want to improve on this, but we need to for various reasons)
5) The guests submits some new requests to the queue, using a
new qh (which possibly has a different address).
6) We see the new qh, and execute a request from there
7) The 1st request on the old qh completes, and we execute the next
8) Now things are a mess as we're executing requests from the old
(cancelled from the guest pov) and new queue intermixed...

Using the token to identify the queue fixes this, cause we will
find the same queue for the old and new qh, and uhci_queue_verify()
will then fail because of the qh addr change, and then we cancel
the old queue. IOW then we do the right thing.

So I'm wondering if there is another, potentially better fix for
what you are seeing?

Regards,

Hans

p.s.

Did you send this directly to Gerd, without CC-ing qemu-devel, or
did I just miss it on qemu-devel ? If the former, please add qemu-devel
to the CC next time.