Re: [PATCH v6 16/20] ppc/xive: Introduce a xive_tctx_ipb_update() helper

[David Gibson]

On Wed, Nov 27, 2019 at 09:50:50AM +0100, Greg Kurz wrote:
65;5802;1c> On Mon, 25 Nov 2019 07:58:16 +0100
> Cédric Le Goater <address@hidden> wrote:
> 
> > We will use it to resend missed interrupts when a vCPU context is
> > pushed on a HW thread.
> > 
> > Signed-off-by: Cédric Le Goater <address@hidden>
> > ---
> >  include/hw/ppc/xive.h |  1 +
> >  hw/intc/xive.c        | 21 +++++++++++----------
> >  2 files changed, 12 insertions(+), 10 deletions(-)
> > 
> > diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> > index 24315480e7c2..9c0bf2c301e2 100644
> > --- a/include/hw/ppc/xive.h
> > +++ b/include/hw/ppc/xive.h
> > @@ -469,6 +469,7 @@ void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor 
> > *mon);
> >  Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp);
> >  void xive_tctx_reset(XiveTCTX *tctx);
> >  void xive_tctx_destroy(XiveTCTX *tctx);
> > +void xive_tctx_ipb_update(XiveTCTX *tctx, uint8_t ring, uint8_t ipb);
> >  
> >  /*
> >   * KVM XIVE device helpers
> > diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> > index 4bff3abdc3eb..7047e45daca1 100644
> > --- a/hw/intc/xive.c
> > +++ b/hw/intc/xive.c
> > @@ -47,12 +47,6 @@ static uint8_t ipb_to_pipr(uint8_t ibp)
> >      return ibp ? clz32((uint32_t)ibp << 24) : 0xff;
> >  }
> >  
> > -static void ipb_update(uint8_t *regs, uint8_t priority)
> > -{
> > -    regs[TM_IPB] |= priority_to_ipb(priority);
> > -    regs[TM_PIPR] = ipb_to_pipr(regs[TM_IPB]);
> > -}
> > -
> >  static uint8_t exception_mask(uint8_t ring)
> >  {
> >      switch (ring) {
> > @@ -135,6 +129,15 @@ static void xive_tctx_set_cppr(XiveTCTX *tctx, uint8_t 
> > ring, uint8_t cppr)
> >      xive_tctx_notify(tctx, ring);
> >  }
> >  
> > +void xive_tctx_ipb_update(XiveTCTX *tctx, uint8_t ring, uint8_t ipb)
> > +{
> > +    uint8_t *regs = &tctx->regs[ring];
> > +
> > +    regs[TM_IPB] |= ipb;
> > +    regs[TM_PIPR] = ipb_to_pipr(regs[TM_IPB]);
> > +    xive_tctx_notify(tctx, ring);
> > +}
> > +
> 
> Maybe rename the helper to xive_tctx_update_ipb_and_notify() to
> make it clear this raises an irq in the end ?

I'm actually happy enough with the name as is.  I think it's
reasonable for "update" on a register which has active effects on the
real hardware to implicitly have the same effects in the qemu model.

Applied to ppc-for-5.0.

> 
> This can be done as follow-up though and the rest looks good, so:
> 
> Reviewed-by: Greg Kurz <address@hidden>
> 
> >  static inline uint32_t xive_tctx_word2(uint8_t *ring)
> >  {
> >      return *((uint32_t *) &ring[TM_WORD2]);
> > @@ -336,8 +339,7 @@ static void xive_tm_set_os_cppr(XivePresenter *xptr, 
> > XiveTCTX *tctx,
> >  static void xive_tm_set_os_pending(XivePresenter *xptr, XiveTCTX *tctx,
> >                                     hwaddr offset, uint64_t value, unsigned 
> > size)
> >  {
> > -    ipb_update(&tctx->regs[TM_QW1_OS], value & 0xff);
> > -    xive_tctx_notify(tctx, TM_QW1_OS);
> > +    xive_tctx_ipb_update(tctx, TM_QW1_OS, priority_to_ipb(value & 0xff));
> >  }
> >  
> >  static void xive_os_cam_decode(uint32_t cam, uint8_t *nvt_blk,
> > @@ -1429,8 +1431,7 @@ static bool xive_presenter_notify(uint8_t format,
> >  
> >      /* handle CPU exception delivery */
> >      if (count) {
> > -        ipb_update(&match.tctx->regs[match.ring], priority);
> > -        xive_tctx_notify(match.tctx, match.ring);
> > +        xive_tctx_ipb_update(match.tctx, match.ring, 
> > priority_to_ipb(priority));
> >      }
> >  
> >      return !!count;
> 

-- 
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