Re: [PATCH 2/4] ppc/xive: Introduce a new XiveRouter end_notify() handler

[Cédric Le Goater]

On 8/31/23 09:50, Frederic Barrat wrote:
> On 29/08/2023 16:32, Cédric Le Goater wrote:
> > It will help us model the END triggers on the PowerNV machine, which
> > can be rerouted to another interrupt controller.
> >
> > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > ---
>
>
>
> > diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> > index 56670b2cac6e..df3ee0496fe7 100644
> > --- a/hw/intc/xive.c
> > +++ b/hw/intc/xive.c
> > @@ -1518,6 +1518,13 @@ static void xive_router_realize(DeviceState *dev, Error 
> > **errp)
> >       assert(xrtr->xfb);
> >   }
> > +static void xive_router_end_notify_handler(XiveRouter *xrtr, XiveEAS *eas)
> > +{
> > +    XiveRouterClass *xrc = XIVE_ROUTER_GET_CLASS(xrtr);
> > +
> > +    return xrc->end_notify(xrtr, eas);
> > +}
> > +
> >   /*
> >    * Encode the HW CAM line in the block group mode format :
> >    *
> > @@ -1664,8 +1671,7 @@ static bool xive_router_end_es_notify(XiveRouter *xrtr, 
> > uint8_t end_blk,
> >    * another chip. We don't model the PowerBus but the END trigger
> >    * message has the same parameters than in the function below.
> >    */
> > -static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk,
> > -                                   uint32_t end_idx, uint32_t end_data)
> > +void xive_router_end_notify(XiveRouter *xrtr, XiveEAS *eas)
> >   {
> >       XiveEND end;
> >       uint8_t priority;
> > @@ -1675,6 +1681,10 @@ static void xive_router_end_notify(XiveRouter *xrtr, 
> > uint8_t end_blk,
> >       XiveNVT nvt;
> >       bool found;
> > +    uint8_t end_blk = xive_get_field64(EAS_END_BLOCK, eas->w);
> > +    uint32_t end_idx = xive_get_field64(EAS_END_INDEX, eas->w);
> > +    uint32_t end_data = xive_get_field64(EAS_END_DATA,  eas->w);
> > +
> >       /* END cache lookup */
> >       if (xive_router_get_end(xrtr, end_blk, end_idx, &end)) {
> >           qemu_log_mask(LOG_GUEST_ERROR, "XIVE: No END %x/%x\n", end_blk,
> > @@ -1817,10 +1827,7 @@ do_escalation:
> >       /*
> >        * The END trigger becomes an Escalation trigger
> >        */
> > -    xive_router_end_notify(xrtr,
> > -                           xive_get_field32(END_W4_ESC_END_BLOCK, end.w4),
> > -                           xive_get_field32(END_W4_ESC_END_INDEX, end.w4),
> > -                           xive_get_field32(END_W5_ESC_END_DATA,  end.w5));
> > +    xive_router_end_notify_handler(xrtr, (XiveEAS *) &end.w4);
>
>
> I didn't like the cast, but I can see why you're doing it this way. 

OPAL does a similar cast :

 https://github.com/open-power/skiboot/blob/master/hw/xive.c#L783

> We should be fine as long as the notify handler is not testing the validity bit 
> of the EAS structure.

Indeed. It doesn't seem to be set :

XIVE[0] #0 END Escalation EAT
  0000008e -Q    end:00/008f data:00000000
  0000008f --    end:01/003f data:0000008f
  00000096 -Q    end:00/0097 data:00000000
  00000097 --    end:01/003f data:00000097

I don't remember to be honest. This is from a few years back. Ask the
HW designer may be ?
 
> Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>

Thanks,

C.

>    Fred
>
>
> >   }
> >   void xive_router_notify(XiveNotifier *xn, uint32_t lisn, bool pq_checked)
> > @@ -1871,10 +1878,7 @@ void xive_router_notify(XiveNotifier *xn, uint32_t lisn, 
> > bool pq_checked)
> >       /*
> >        * The event trigger becomes an END trigger
> >        */
> > -    xive_router_end_notify(xrtr,
> > -                           xive_get_field64(EAS_END_BLOCK, eas.w),
> > -                           xive_get_field64(EAS_END_INDEX, eas.w),
> > -                           xive_get_field64(EAS_END_DATA,  eas.w));
> > +    xive_router_end_notify_handler(xrtr, &eas);
> >   }
> >   static Property xive_router_properties[] = {
> > @@ -1887,12 +1891,16 @@ static void xive_router_class_init(ObjectClass *klass, 
> > void *data)
> >   {
> >       DeviceClass *dc = DEVICE_CLASS(klass);
> >       XiveNotifierClass *xnc = XIVE_NOTIFIER_CLASS(klass);
> > +    XiveRouterClass *xrc = XIVE_ROUTER_CLASS(klass);
> >       dc->desc    = "XIVE Router Engine";
> >       device_class_set_props(dc, xive_router_properties);
> >       /* Parent is SysBusDeviceClass. No need to call its realize hook */
> >       dc->realize = xive_router_realize;
> >       xnc->notify = xive_router_notify;
> > +
> > +    /* By default, the router handles END triggers locally */
> > +    xrc->end_notify = xive_router_end_notify;
> >   }
> >   static const TypeInfo xive_router_info = {