Protect against use of unlocked channel doneq in ahci driver

[smh]

Summary

With the current locking doneq can be processed by multiple handlers.

This can cause a ccb to be processed more than once which in turn can produce a use after panic.

Prevent this possibility by using local temporary queue in ahci_ch_intr_direct.

Also protect against overun of irqs if a controller has more than 16 interrupts.

While here fix some style (9) nits.

[mav]

I still can't see why this change something, but of it does -- I don't mind.

[smh]

It does prevent double take errors, which is what bdrewery was seeing when he was missing the interrupt ordering fix.

So if there is some other situation we're not aware of this will still ensure lock consistency for done ccb's.

[adrian]

I'd do this as a handful of commits:

• one to change 16 -> #define
• one to change the return ENXIO -> return error
• one to clamp the number of interrupts to the #define'd value and log
• one to do the queue concat within lock and dequeue outside of lock

Other than that, this looks good to me.

[imp]

Looks like there's three commits here: (1) Change the return foo; to return (foo); (2) cap interrupt count and add #defines for it (3) fix done queue. The first one likely shouldn't be done, the second and third are good, but it would be better to do them as separate commits if possible. #2 isn't mentioned...

sys/dev/ahci/ahci.c

228

This is a gratuitous change. Most of the rest of the file uses the w/o () style as well.

239

Ditto

318

ditto

361

Why not return an error?

383

ditto

392

ditto.

1137

this last chunk would make a good commit by itself.

sys/dev/ahci/ahci.h

498

This could be a separate and the above #define could be a separate commit.

[smh]

I'll look to split up the commits but easier to get it all reviewed as one as they are all easy little changes anyway.

Thanks for the feedback guys, be interested to hear what people think about returning an error if we have more than 16 interrupts.

I did contemplate just making it dynamic with malloc, which is also pretty easy thoughts?

sys/dev/ahci/ahci.c

361

I went with no error as I believe this is only going to happen with the interrupts aren't used anyway, like we have at the moment where we wire up 16 interrupts but only ever see up to 6 used (the general number or supported channels).

[smh]

These changes have all now been including via the following individual commits:

• r272223
• r276012
• r276013
• r276016
• r276019