Re: [PATCH 1/2] usb: chipidea: udc: limit usb request length to max 16KB

Xu Yang
2024-09-13T15:26:42Z

On Fri, Sep 13, 2024 at 05:53:44PM +0800, Peter Chen wrote:
> On 24-09-13 15:11:33, Xu Yang wrote:
> > On Fri, Sep 13, 2024 at 09:20:45AM +0800, Peter Chen wrote:
> > > On 24-09-12 12:51:49, Xu Yang wrote:
> > > > Currently, the deivice controller has below limitations:
> > > > 1. can't generate short packet interrupt if IOC not set in dTD. So if one
> > > > request span more than one dTDs and only the last dTD set IOC, the usb
> > > > request will pending there if no more data comes.
> > > > 2. the controller can't accurately deliver data to differtent usb requests
> > > > in some cases due to short packet. For example: one usb request span 3
> > > > dTDs, then if the controller received a short packet the next packet
> > > > will go to 2nd dTD of current request rather than the first dTD of next
> > > > request.
> > > >
> > >
> > > Are there any IP errata for it?
> >
> > No. It's decided by hw IP design. This old design may not suit current
> > requirements.
> >
> > >
> > > > To let the device controller work properly, one usb request should only
> > > > correspond to one dTD. Then every dTD will set IOC. In theory, each dTD
> > > > support up to 20KB data transfer if the offset is 0. Due to we cannot
> > > > predetermine the offset, this will limit the usb request length to max
> > > > 16KB. This should be fine since most of the user transfer data based on
> > > > this size policy.
> > > >
> > > > Although these limitations found on OUT eps, we can put the request to IN
> > > > eps too, this will benefit the following patches.
> > >
> > > Since IN endpoints have not found the problem, please limit the changes
> > > only for OUT endpoints.
> >
> > This 1st patch is mainly used to serve the 2nd patch which may impact
> > both IN and OUT eps.
> ...
> > Because it's hard to judge whether a request is
> > suit for transfer if it spans more dTDs. So it's needed for both eps.
>
> Sorry, I do not understand you above words. First, you may know this
> request is for IN or OUT, second, according to TD size and data buffer
> address, you may know you use one or more dTDs.

If req.num_sgs = 0, then we can know how many TDs need to transfer data.

For example:
req.buf = 0xA0001800 req.length = 40KB

- TD1 addr:0xA0001800 size:18KB
- TD2 addr:0xA0017000 size:20KB
- TD3 addr:0xA002D000 size:2KB

We basically won't meet issue for non-sg case. The only expection is that
received short packet on TD1 (or TD2). Then the next data packet will go
to TD2. But it should go to TD1 of next request.

But if num_sgs > 0, we need to check validity of each sg entry due to above
limitations.

For example:
req.num_sgs = 3 req.length = 40KB

- sg1.addr = 0xA0001800 length = 18KB -> TD1
- sg2.addr = 0xA0016000 length = 20KB -> TD2
- sg3.addr = 0xA0028800 length = 2KB -> TD3

This request can be safty used to transfer data. But we can also meet
previous short packet issue.

req.num_sgs = 5 req.length = 10B + 20KB

- sg1.addr = 0xA0001800 length = 10B -> TD1
- sg2.addr = 0xA0016000 length = 6KB -> TD2
- sg3.addr = 0xA0028800 length = 6KB -> TD3
- sg4.addr = 0xA003A000 length = 4KB -> TD3
- sg5.addr = 0xA004C000 length = 4KB -> TD3

This request can't be used to transfer data since sg1 + sg2 can't
form a data packet. The host will see a short packet (100 bytes).

req.num_sgs = 5 req.length = 20KB + 10B

- sg1.addr = 0xA0001800 length = 2KB -> TD1
- sg2.addr = 0xA0016400 length = 5KB -> TD2
- sg3.addr = 0xA0028800 length = 8KB -> TD3
- sg4.addr = 0xA003A800 length = 5KB -> TD4
- sg5.addr = 0xA004C200 length = 10B -> TD5

Compared to previous request, it need 5 TDs even though req.length
are same. Most of the sg entries can't share same TD since their
address is not page aligned. For high-speed isoc eps, sg1 + sg2 can't
form a 3KB DATA2 + DATA1 + DATA0 data sequence too.

Therefore, it's a bit complicated to validate request if num_sgs > 0,
especially when req.length is larger than 16KB (1 TD size).

When add such condition, each of the sg entry must follow below
requirements:
1. the end address of the first sg buffer must be 4KB aligned.
2. the start and end address of the middle sg buffer must be 4KB aligned.
3. the start address of the last sg buffer must be 4KB aligned.

So it will be more easy to validate the request.

Hope this will help you understand the motivation of 1st patch.

Thanks,
Xu Yang