DragonFly kernel List (threaded) for 2006-02
Re: MSI(-X) interface RFC (was: MSI prototype)
:struct resource *
:bus_alloc_resource(device_t dev, int type, int *rid, u_long start,
: u_long end, u_long count, u_int flags);
:Same as before with a couple of comments. There are a number of
:reasons the implementation may return NULL in spite of having
:unallocated vectors. Some examples include:
:- The implementation may not be able to satisfy MSI starting alignment
:restrictions. Drivers may be able to recover from this failure by
:requesting fewer vectors or increasing the range of allowable vectors.
:- Separate from the alignment restrictions, there may not be enough
:contiguous vectors to fulfill a request. Drivers may be able to
:recover from this failure by requesting fewer vectors or increasing
:the range of allowable vectors.
:- The driver may have specified a range that does not have enough
:available vectors. Drivers may be able to recover from this failure by
:increasing the range defined by start/end or not setting the
:Unfortunately, I wasn't as lucky with bus_setup_intr() and couldn't
:map the necessary functionality into the current interface. We can
:either define a new interface (bus_setup_intr_vec?) that supports the
:extra arguments, or we can modify bus_setup_intr(). I don't have a
:preference and can see good arguments for both approaches.
:bus_setup_intr*(device_t dev, struct resource *r, int flags,
: driver_intr_t handler, void *arg, int vec, int entry,
: void **cookiep, lwkt_serialize_t serializer);
:The majority of the parameters are identical in meaning and function
:to their counter parts in bus_setup_intr(). The vec and entry
:parameters exist to assign an allocated vector to a table entry. As an
:additional benefit, this approach allows vector aliasing (i.e. filling
:multiple table entries with the same vector value). Note that the PCI
:3.0 spec specifically suggests aliasing as a way to handle vector
:The arguments are as follows:
:vec corresponds to a vector in the range returned by bus_alloc_resource().
:entry specifies which slot in the device's vector table to fill. Note
:that for SYS_RES_IRQ and SYS_RES_MSI only a value of zero may be used.
Hmm. I think that might be a bit too messy, not so much because of
the extra arguments, but because the arguments aren't really all that
generic and only apply to MSI(X) vectors.
I have an idea.
Have bus_alloc_resource() allocate the container that represents all
the vectors, just as you describe, but then create a
bus_alloc_sub_resource() procedure to separately allocate each vector
within that container. Then call bus_setup_intr() for each vector
individually. The driver needs to have a loop of some sort anyway
and this way we would actually have a resource descriptor for
each individual vector and we wouldn't have to hack up bus_setup_intr().
This allows us to retain the current interrupt vector resource descriptor
and all the internal wiring we currently have to deal with interrupt
vectors without having to rewrite any of that code. The only new code
would be in the bus_alloc_resource() support for the MSI(X) vector
block, and the new bus_alloc_sub_resource() call to get a handle for
each vector within that block.
:Some other notes:
:- Drivers are not allowed to modify the values in the Message Control
:register including the MSI Enable bit.
:- Enabling MSI via bus_setup_intr_vec() will also clear bit 10
:(Interrupt Disable) of the Device Control register as well as bit 15
:(MSI-X Enable) of the Message Control for MSI-X register. Only one
:type of interrupt may be active at any one time.
:- Enabling MSI-X via bus_setup_intr_vec() will also clear bit 10
:(Interrupt Disable) of the Device Control register as well as bit 0
:(MSI Enable) of the Message Control for MSI register. Only one type of
:interrupt may be active at any one time.
:- Device drivers need to be aware of the memory region associated with
:the device's MSI-X table and not map it. The bus driver will take care
:of that task.
You'll want to clean up those bits when the interrupt is torn down,
which I guess means reverting them to normal interrupt operation or
saving and restoring the state of the bits or something like that.
Not critical, but it could save us hassle in the future.
:TIA for your feedback.