On Wed, Oct 02, 2019 at 11:29:08AM +0200, Greg Kurz wrote:
> On Wed, 2 Oct 2019 12:52:04 +1000
> David Gibson <da...@gibson.dropbear.id.au> wrote:
>
> > It turns out that all the logic in the SpaprIrq::reset hooks (and some in
> > the SpaprIrq::post_load hooks) isn't really related to resetting the irq
> > backend (that's handled by the backends' own reset routines). Rather its
> > about getting the backend ready to be the active interrupt controller or
> > stopping being the active interrupt controller - reset (and post_load) is
> > just the only time that changes at present.
> >
> > To make this flow clearer, move the logic into the explicit backend
> > activate and deactivate hooks.
> >
> > Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
> > ---
> > hw/intc/spapr_xive.c | 35 ++++++++++++++++++++
> > hw/intc/xics_spapr.c | 16 +++++++++
> > hw/ppc/spapr_irq.c | 67 ++------------------------------------
> > include/hw/ppc/spapr_irq.h | 4 ++-
> > 4 files changed, 57 insertions(+), 65 deletions(-)
> >
> > diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> > index 37ffb74ca5..e8b946982c 100644
> > --- a/hw/intc/spapr_xive.c
> > +++ b/hw/intc/spapr_xive.c
> > @@ -640,6 +640,39 @@ static void spapr_xive_dt(SpaprInterruptController
> > *intc, uint32_t nr_servers,
> > plat_res_int_priorities,
> > sizeof(plat_res_int_priorities)));
> > }
> >
> > +static void spapr_xive_activate(SpaprInterruptController *intc, Error
> > **errp)
> > +{
> > + SpaprXive *xive = SPAPR_XIVE(intc);
> > + CPUState *cs;
> > +
> > + CPU_FOREACH(cs) {
> > + PowerPCCPU *cpu = POWERPC_CPU(cs);
> > +
> > + /* (TCG) Set the OS CAM line of the thread interrupt context. */
> > + spapr_xive_set_tctx_os_cam(spapr_cpu_state(cpu)->tctx);
> > + }
> > +
>
> I think this ^^ can go...
>
> > + if (kvm_enabled()) {
> > + if (spapr_irq_init_kvm(kvmppc_xive_connect, intc, errp) < 0) {
> > + return;
> > + }
> > + }
> > +
>
> ... here. If which case, spapr_irq_init_kvm() could be called from
> set_active_intc() instead of being called by each backend if I get
> it right. This would avoid the frontend->backend->frontend flow.
Hm. I don't love the idea. KVM setup seems like an internal
implementation detail of the backend, which I'd prefer not to handle
in the frontend.
I'm not really considering spapr_irq_init_kvm() as part of the
frontend proper, but rather a helper function for use in the backend.
--
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
