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

[Jan Kiszka]

Hi Hans,

On 2012-11-15 16:19, Hans de Goede wrote:
> 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.

But what disallows that the guest issues multiple requests (QH + series
of TDs) for a single endpoint? I'm not finding any trace in the spec
that disallows this. And my special guest is stumbling over that
limitation in QEMU.

> 
> 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 ?

I'm on git 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.

I was afraid of triggering some conflict but, as pointed out above, the
current code also does something wrong. It associates two different
active QH with the same queue and then triggers a failure for the first
QH as it wrongly thinks the guest has unlinked it.

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

/me too. I'm not fully understanding your scenario above yet,
specifically when and how the guest can correctly remove an active QH,
so I cannot provide good suggestions at this point.

> 
> 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.

http://permalink.gmane.org/gmane.comp.emulators.qemu/180483

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux