Re: usb-storage: how to extend quirks flags to 64bit?

[Milan Broz]

On 8/27/23 20:55, Alan Stern wrote:

...

> > > > Someone will need a new quirks flag in the future anyway... :)
> > >
> > > I can think of only one way to accomplish this on 32-bit systems: Change
> > > the driver_info field from a bit array to an index into a static table
> > > of 64-bit flags values.  Each unusual_devs structure would have its own
> > > entry in this table.  As far as I can tell, the other unusual_*.h tables
> > > could retain their current driver_info interpretations, since no new
> > > quirk bits are likely to be relevant to them.
> > >
> > > Making this change would be an awkward nuisance, but it should be
> > > doable.
> >
> > Hm, yes, thanks for the idea,that is a possible solution.
> > It will need to modify all unusual macros, though. Just I am not sure I want
> > to spent time patching all the drivers as I have not way how to test it.
>
> I don't think it will be necessary to change all those macros, just the
> ones in usual_tables.c.  And to create the new table containing the
> actual flag values, of course.
>
> There will also have to be a new argument to usb_stor_probe1()
> specifying whether the id->driver_info field is standard (i.e., it
> contains the flags directly) or is one of the new indirect index values.
>
> And you'll have to figure out a comparable change to the dynamic device
> ID table mechanism.
>
> (If you want to be really fancy about it, you could design things in
> such a way that the indirect flags approach is used only on 32-bit
> systems.  64-bit systems can put the new flag bits directly into the
> driver_info field.  However, it's probably best not to worry about this
> initially.)

Hi Alan,

So, I really tried this approach, spent more time on in than I expected, but
produced working code... that I am really not proud of :-]
(Thus avoiding to send it here, for now.)

I pushed it to my dm-cryptsetup branch here
https://git.kernel.org/pub/scm/linux/kernel/git/mbroz/linux.git/log/?h=dm-cryptsetup

The last patch is the reason why I need it, just for reference.
More comments in the patch headers.

Could you please check it if it is *really* what we want?
If so, I'll rebase it for usb next tree and send as a patchset.

But the macro magic is crazy... and I really did not find the better way.

Anyway, it also uncovered some problems
 - some macros need to be changed a little bit, there is even old one unused
 - duplicity of entries in UAS and mass-storage are strange (and complicates
   the approach).
   I guess the sorting is intentionally that mass-storage is included
   before UAS, so the mass-storage quirk is found as the first (for non-UAS).
   (While UAS drive includes only own header.)
- the patch significantly increases size of module for 32bit
  (64bit system use the direct flag store approach)
- I stored a pointer to the flags array, not the index. Perhaps it should be
  index only (trivial change, though).

Thanks,
Milan