Re: [Qemu-devel] [RFC PATCH 14/26] ppc/xive: add MMIO handlers to the XIVE interrupt presenter model

[David Gibson]

On Tue, Jul 25, 2017 at 05:01:48PM +0200, Cédric Le Goater wrote:
> On 07/25/2017 03:21 PM, David Gibson wrote:
> > On Tue, Jul 25, 2017 at 11:08:46AM +0200, Cédric Le Goater wrote:
> >> On 07/25/2017 06:20 AM, David Gibson wrote:
> >>> On Mon, Jul 24, 2017 at 04:44:00PM +0200, Cédric Le Goater wrote:
> >>>> On 07/24/2017 08:35 AM, David Gibson wrote:
> >>>>> On Wed, Jul 05, 2017 at 07:13:27PM +0200, Cédric Le Goater wrote:
> >>>>>> The Thread Interrupt Management Area for the OS is mostly used to
> >>>>>> acknowledge interrupts and set the CPPR of the CPU.
> >>>>>>
> >>>>>> The TIMA is mapped at the same address for each CPU. 'current_cpu' is
> >>>>>> used to retrieve the targeted interrupt presenter object.
> >>>>>>
> >>>>>> Signed-off-by: Cédric Le Goater <address@hidden>
> >>>>>
> >>>>> Am I right in thinking that this shoehorns the XIVE TIMA state into
> >>>>> the existing XICS ICP object.  That.. doesn't seem like a good idea.
> >>>>
> >>>> The TIMA memory region is under the XIVE object because it is 
> >>>> unique for the system. The lookup of the ICP is simply done using 
> >>>> 'current_cpu'. The TIMA state is under the ICPState, yes, but this 
> >>>> model does not seem incorrect to me as this state contains the 
> >>>> interrupt information presented to a CPU.
> >>>
> >>> Yeah, that's not the point I'm making.  My point is that the TIMA
> >>> state isn't really the same as xics ICP state.  You're squeezing one
> >>> into the other in a pretty ugly way.
> >>
> >> yes, well, we need to have compatible objects between the XICS and XIVE 
> >> mode because of the CAS negotiation. for migration compatibility, it is 
> >> much easier to extend existing objects. This approach I am taking today.
> > 
> > Yeah, I really don't think this approach is workable.
> > 
> > Roughly speaking, you're keeping the same structures between xics and
> > xive, but with mostly different code.  I can't see any way that's not
> > going to be horribly fragile, with any update to xics OR xive
> > requiring enormous caution not to break the other.
> > 
> > I really think we have to go one of two ways:
> > 
> > 1) Abstract the notion of interrupt source and interrupt presenter, so
> > we can use a truly common model between xics and xive.
> > 
> > Given the differences between the two, I don't know this is even
> > possible.
> > 
> > 2) Separate the objects entirely.  ICPs are entirely separate from
> > TIMAs, like wise ICSes and xive interrupt sources.
> > 
> > I think this is probably the way to go.  To make this work with CAS
> > switching will require different methods, but I don't think it's
> > impossible.
> >
> > For example, we could (on the new machine type) create both xics ICSes
> > and ICPs and xive sources and TIMAs.  We'd have a (migrated) flag in
> > the machine saying which is currently active.  All the objects would
> > hang around, but only the active ones would do anything.
> 
> ok. There is still the question of sharing the IRQ numbers unless we 
> find away to allocate them after the CAS negotiation. 

Right.  As I said, I think allocating after CAS is probably possible,
but it may not be necessary.

Merely lining up the irq numbers should be much simpler than actually
fusing the source and presenter structures.  Especially since both
xics and xive are flexible enough that we can map the irqs almost
however we want.

This definitely does mean the irq mapping / allocation should sit
inside the machine type, rather than the xics *or* the xive code, and
preferably be entirely common.  Then it's just a matter of setting up
both the xics and xive components so they can back that irq allocation.

> I will take a look at KVM support now before reworking the patchset.
> 
> Thanks,
> 
> C.
> 
> > Now obviously that means we'd be migrating a bunch of redundant state,
> > but I still think it's preferable to a Frankenstinian fusion of
> > xics-ish and xive-ish state.  I think there's a good chance we can
> > improve on the basic idea to remove most or all of that redundant
> > state.
> > 
> 

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature