Re: newbus' ivar's limitation..

On Monday, July 09, 2012 12:39:03 am Warner Losh wrote:
> 
> On Jul 8, 2012, at 9:59 PM, Arnaud Lacombe wrote:
> 
> > Hi,
> > 
> > On Sun, Jul 8, 2012 at 10:07 PM, Warner Losh <imp_at_bsdimp.com> wrote:
> >> 
> >> On Jul 8, 2012, at 7:22 PM, Arnaud Lacombe wrote:
> >>> Ok, yet another Newbus' limitation. Assuming a device exports more
> >>> than one interface, and one of its child has need to use more than one
> >>> interface, each interfaces cannot register, concurrently, its own
> >>> ivar. While I try to always have a single child per
> >>> interface/resource, I need to keep some compatibility with the old way
> >>> of doing thing (POLA wrt. drivers I cannot/will not convert and
> >>> userland). So, it would have been nice if ivar had been per-interface,
> >>> not global and unique to one device.
> >> 
> >> There's one pointer for the ivars.  The bus code gets to determine what 
the ivar looks like, because the interface is totally private to the bus.  So 
long as it returns the right thing for any key that's presented, it doesn't 
matter quite how things are done.
> >> 
> >> So I'm not sure I understand what you are saying here.
> >> 
> >> The problem, more basically, is that the ivar keys are not unique.  
Currently, there's no bits used in the key to define the values to be non-
overlapping.  For example:
> >> enum pci_device_ivars {
> >>    PCI_IVAR_SUBVENDOR,
> >>    PCI_IVAR_SUBDEVICE,
> >>    PCI_IVAR_VENDOR,
> >> ....
> >> };
> >> 
> >> We could easily reserve the upper 16-bits of this field to be that key.  
This value could then be used to differentiate them.  But this wouldn't scale 
too well.  Given that there's only about a dozen or two in the tree, that's 
right at the moment, it wouldn't be hard to do something like:
> >> 
> >> enum ivar_namespace {
> >>        IVAR_PCI = 1,
> >>        IVAR_PCCARD,
> >>        IVAR_USB,
> >> etc
> >> };
> >> #define IVAR_SHIFT 16
> >> 
> >> and the above could be changed to:
> >> 
> >> enum pci_device_ivars {
> >>    PCI_IVAR_SUBVENDOR = IVAR_PCI << IVAR_SHIFT,
> >>    PCI_IVAR_SUBDEVICE,
> >>    PCI_IVAR_VENDOR,
> >> ....
> >> };
> >> 
> >> and then we'd have an unambiguous key, and the bus could easily implement 
multiple interfaces.
> >> 
> >> but then again, most of the existing interfaces in the kernel are 
mutually exclusive, so you could implement this just for your new interfaces.
> >> 
> > ok, I think I got it now. You and I are exactly saying the same thing,
> > just in different terms; there is no way to currently specify multiple
> > independent (/overlapping) ivars in a child...
> 
> There's no way to support overlapping IVAR keys, yes.  However, if you are 
designing the ivars for multiple inheritance, then you'll need to make them 
non-overlapping.  Thankfully, this is trivial to do.

Also, I think we should do this in general.  We already have one example (e.g. 
ACPI IVARs start at 100 so that things like the ACPI PCI bus driver can
provide both ACPI and PCI IVARs to child devices).  I think we should assign
each bus it's own IVAR range.  It would make it easier to determine if a 
specific IVAR is event supported.  Certainly I think ISA and PCI at a minimum 
should be changed to start at, say, 200 and 300.

-- 
John Baldwin