Re: [Qemu-ppc] [PATCH v3 03/35] ppc/xive: introduce the XiveFabric interface

[Cédric Le Goater]

On 04/23/2018 08:46 AM, David Gibson wrote:
> On Thu, Apr 19, 2018 at 02:42:59PM +0200, Cédric Le Goater wrote:
>> The XiveFabric offers a simple interface, between the XiveSourve
>> object and the device model owning the interrupt sources, to forward
>> an event notification to the XIVE interrupt controller of the machine
>> and if the owner is the controller, to call directly the routing
>> sub-engine.
>>
>> Signed-off-by: Cédric Le Goater <address@hidden>
>> ---
>>  hw/intc/xive.c        | 37 ++++++++++++++++++++++++++++++++++++-
>>  include/hw/ppc/xive.h | 25 +++++++++++++++++++++++++
>>  2 files changed, 61 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
>> index 060976077dd7..b4c3d06c1219 100644
>> --- a/hw/intc/xive.c
>> +++ b/hw/intc/xive.c
>> @@ -17,6 +17,21 @@
>>  #include "hw/ppc/xive.h"
>>  
>>  /*
>> + * XIVE Fabric
>> + */
>> +
>> +static void xive_fabric_route(XiveFabric *xf, int lisn)
>> +{
>> +
>> +}
>> +
>> +static const TypeInfo xive_fabric_info = {
>> +    .name = TYPE_XIVE_FABRIC,
>> +    .parent = TYPE_INTERFACE,
>> +    .class_size = sizeof(XiveFabricClass),
>> +};
>> +
>> +/*
>>   * XIVE Interrupt Source
>>   */
>>  
>> @@ -97,11 +112,19 @@ static bool xive_source_pq_trigger(XiveSource *xsrc, 
>> uint32_t srcno)
>>  
>>  /*
>>   * Forward the source event notification to the associated XiveFabric,
>> - * the device owning the sources.
>> + * the device owning the sources, or perform the routing if the device
>> + * is the interrupt controller.
>>   */
>>  static void xive_source_notify(XiveSource *xsrc, int srcno)
>>  {
>>  
>> +    XiveFabricClass *xfc = XIVE_FABRIC_GET_CLASS(xsrc->xive);
>> +
>> +    if (xfc->notify) {
>> +        xfc->notify(xsrc->xive, srcno + xsrc->offset);
>> +    } else {
>> +        xive_fabric_route(xsrc->xive, srcno + xsrc->offset);
>> +    }
> 
> Why 2 cases?  Can't the XiveFabric object just make its notify equal
> to xive_fabric_route if that's what it wants?
Under sPAPR, all the sources, IPIs and virtual device interrupts, 
generate events which are directly routed by xive_fabric_route(). 
There is no need of an extra hop. Indeed. 

Under PowerNV, some sources forward the notification to the routing 
engine using a specific MMIO load on a notify address which is stored 
in one of the controller registers. So we need a hop to reach the 
device model, owning the sources, and do that load : 

        static void pnv_psi_notify(XiveFabric *xf, uint32_t lisn)
        {
            PnvPsi *psi = PNV_PSI(xf);
            uint64_t notif_port =
                psi->regs[PSIHB_REG(PSIHB9_ESB_NOTIF_ADDR)];
            bool valid = notif_port & PSIHB9_ESB_NOTIF_VALID;
            uint64_t notify_addr = notif_port & ~PSIHB9_ESB_NOTIF_VALID;
            uint32_t data = cpu_to_be32(lisn);
        
            if (valid) {
                cpu_physical_memory_write(notify_addr, &data, sizeof(data));
            }
        }

The PnvXive model handles the load and forwards to the fabric again.  

The IPIs under PowerNV do not need an extra hop so they reach the 
routing routine directly without the extra notify() hop. 

However, PowerNV at the end should be using xive_fabric_route() 
but there are some differences on how the NVT registers are 
updated (HV vs. OS mode) and it's not handled yet so it uses a 
notify() handler. But is should disappear and call directly 
xive_fabric_route() in a near future.


May be, XiveFabricNotifier would be a better name for this feature ?
I am adding a few ops later which are more related to routing.

Thanks,

C.


> 
>>  }
>>  
>>  /*
>> @@ -302,6 +325,17 @@ static void xive_source_reset(DeviceState *dev)
>>  static void xive_source_realize(DeviceState *dev, Error **errp)
>>  {
>>      XiveSource *xsrc = XIVE_SOURCE(dev);
>> +    Object *obj;
>> +    Error *local_err = NULL;
>> +
>> +    obj = object_property_get_link(OBJECT(dev), "xive", &local_err);
>> +    if (!obj) {
>> +        error_propagate(errp, local_err);
>> +        error_prepend(errp, "required link 'xive' not found: ");
>> +        return;
>> +    }
>> +
>> +    xsrc->xive = XIVE_FABRIC(obj);
>>  
>>      if (!xsrc->nr_irqs) {
>>          error_setg(errp, "Number of interrupt needs to be greater than 0");
>> @@ -376,6 +410,7 @@ static const TypeInfo xive_source_info = {
>>  static void xive_register_types(void)
>>  {
>>      type_register_static(&xive_source_info);
>> +    type_register_static(&xive_fabric_info);
>>  }
>>  
>>  type_init(xive_register_types)
>> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
>> index 0b76dd278d9b..4fcae2c763e6 100644
>> --- a/include/hw/ppc/xive.h
>> +++ b/include/hw/ppc/xive.h
>> @@ -12,6 +12,8 @@
>>  
>>  #include "hw/sysbus.h"
>>  
>> +typedef struct XiveFabric XiveFabric;
>> +
>>  /*
>>   * XIVE Interrupt Source
>>   */
>> @@ -46,6 +48,8 @@ typedef struct XiveSource {
>>      hwaddr       esb_base;
>>      uint32_t     esb_shift;
>>      MemoryRegion esb_mmio;
>> +
>> +    XiveFabric   *xive;
>>  } XiveSource;
>>  
>>  /*
>> @@ -143,4 +147,25 @@ static inline void xive_source_irq_set(XiveSource 
>> *xsrc, uint32_t srcno,
>>      xsrc->status[srcno] |= lsi ? XIVE_STATUS_LSI : 0;
>>  }
>>  
>> +/*
>> + * XIVE Fabric
>> + */
>> +
>> +typedef struct XiveFabric {
>> +    Object parent;
>> +} XiveFabric;
>> +
>> +#define TYPE_XIVE_FABRIC "xive-fabric"
>> +#define XIVE_FABRIC(obj)                                     \
>> +    OBJECT_CHECK(XiveFabric, (obj), TYPE_XIVE_FABRIC)
>> +#define XIVE_FABRIC_CLASS(klass)                                     \
>> +    OBJECT_CLASS_CHECK(XiveFabricClass, (klass), TYPE_XIVE_FABRIC)
>> +#define XIVE_FABRIC_GET_CLASS(obj)                                   \
>> +    OBJECT_GET_CLASS(XiveFabricClass, (obj), TYPE_XIVE_FABRIC)
>> +
>> +typedef struct XiveFabricClass {
>> +    InterfaceClass parent;
>> +    void (*notify)(XiveFabric *xf, uint32_t lisn);
>> +} XiveFabricClass;
>> +
>>  #endif /* PPC_XIVE_H */
>