[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: OHCI/usb pass through
From: |
BALATON Zoltan |
Subject: |
Re: OHCI/usb pass through |
Date: |
Fri, 1 Oct 2021 13:35:49 +0200 (CEST) |
Hello,
On Fri, 1 Oct 2021, Gerd Hoffmann wrote:
[...]
/* Active packets. */
uint32_t old_ctl;
USBPacket usb_packet;
uint8_t usb_buf[8192];
uint32_t async_td;
bool async_complete;
void (*ohci_die)(struct OHCIState *ohci);
} OHCIState;
Then everything in hcd-ohci seems to reuse ohci->usb_packet and I wonder if
it can happen that it's overwritten while an async packet is still using it.
Plausible theory. That also nicely explains why you need traffic on two
endpoints at the same time to trigger it.
We've added an assert to check this and Howard could trigger it at least
once (hope he'll answer with details) so I think that proves this but
there may be other problems too as it does not work even when the assert
is not triggered but maybe that's becuase not allowing traffic while an
async packet is pending. It looks like it starts an interrupt transfer on
an endpoint while sends iso data to another. I don't know usb audio
protocol but maybe it's waiting for the iso transfer to finish which won't
as those packets are now rejected due to too many waiting. Howard has some
logs but I've only seen excerpts and did not reproduce it myself so I
don't know exactly. In any case, fixing both of this possible breakage of
ohci->usb_packet and emulating multiple pending packets correctly should
improve the OHCI model and maybe get us further or at least prove we have
more problems to look for.
In any case to both fix the device model and to avoid this possible problem
described above it seems we would need to ditch the packet and async_td
fields from OHCIState and move them to the endpoint to allow one active
packet per endpoint.
Either that, or maintain a linked list of packets.
I'd like to avoid duplicating state and keep everyting attached to the
endpoint so we don't need to keep track of it in ohci. We'd need a packet
for each endpoint and also make async_td an array then so if instead we
can get this info from the endpoint that we already have a pointer to, we
could use that and remove both of these duplicate values from OHCIState so
I'd try that as it looks less error prone.
We can get the endpoint from a packet and from ohci so
I wonder if we can get the active packet from ep->queue (and how to do that)
I think ohci never looks beyond the active td so there should never be
more than one packet on the list.
OK, how to get the packet from that QTAILQ list? If there are multiple
packets is the active one first or last? How to get that? I could try to
find the answers in the code but I realy did not want to spend much time
with it just trying to help Howard so I'd like to ask for some help with
this.
Regards,
BALATON Zoltan