Re: [RFC/PATCH 0/16] Ops based MSI Implementation

[Michael Ellerman <michael@ellerman.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3153 bytes --]

On Thu, 2007-01-25 at 23:18 -0700, Eric W. Biederman wrote:
> Michael Ellerman <michael@ellerman.id.au> writes:
> 
> > OK, here's a first cut at moving ops based MSI into the generic code. I'm
> > posting this now to make sure I'm not heading off into the weeds.
> 
> First thanks for copying me on this.  I really appreciate it.

No worries, thanks for looking at it.

> I haven't done more than skim the patches yet but I am distressed.
> 
> You code appears to be nice simple clean and to not support MSI in
> a useful way.  I may be reading too quickly but at the moment your infrastructure
> appears useless if you are on a platform that doesn't enforce MSI's get filtered
> with a legacy interrupt controller.

That's what PowerPC does, but I don't think there's anything in the top
level interface that requires that - it's all up to the alloc routine.

> You don't have MSI-X support (which is the interesting case) and you don't have
> suspend/resume support.

We have MSI-X support on RTAS ;), but that's cheating. I have 90% of
what I need for MSI-X, but I haven't implemented it yet, I will as part
of the port of the Intel code.

> You don't support the MSI mask bit.

IMHO that's a backend detail.

> Looking at your msi_ops it does not map to what I am doing on x86.  There
> is the implicit assumption that the msi_message is fixed for the lifetime
> of the msi.  Which is wrong.

Again, that's how PowerPC does it, but I don't think it's assumed. If
your backend needs to change the message then we can support that
reasonably easily I think.

> So in short summary I cannot use your msi_ops they are inappropriate for
> i386, x86_64 and ia64.
> 
> So at the moment I am opposed to this code because as it sits it appears to
> be a serious regression.
> 
> The additional bits that feel like this code was primarily targeted at supporting
> the RTAS with real hardware support thrown in as an after thought just seem
> to add insult to injury.  To date I have no information that indicates to me
> that the RTAS model is at all sane or makes any sense to duplicate elsewhere.
> If supporting the RTAS is what is obscuring your vision of what is really
> needed to support MSI I don't want to see RTAS support in a patch set
> until we get a good multiple platform architecture, merged into the kernel.
> 
> Supporting the RTAS first and breaking everyone who actually has real
> hardware seems like very much the wrong approach to get a good
> multiple platform solution.

I agree. We didn't design it to be a multi platform solution, we
designed it to work for us. It does that. Now we're hoping to expand it
to work for the Intel case as well.

I guess I wasn't clear enough in my original post, but I fully expect
that I'll need to tweak parts of the core to make Intel fit. That's
still a work in progress.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]