Re: [Qemu-devel] [PATCH v6 04/37] ppc/xive: introduce the XiveRouter model

[Cédric Le Goater]

On 12/6/18 4:41 AM, David Gibson wrote:
> On Thu, Dec 06, 2018 at 12:22:18AM +0100, Cédric Le Goater wrote:
>> The XiveRouter models the second sub-engine of the XIVE architecture :
>> the Interrupt Virtualization Routing Engine (IVRE).
>>
>> The IVRE handles event notifications of the IVSE and performs the
>> interrupt routing process. For this purpose, it uses a set of tables
>> stored in system memory, the first of which being the Event Assignment
>> Structure (EAS) table.
>>
>> The EAT associates an interrupt source number with an Event Notification
>> Descriptor (END) which will be used in a second phase of the routing
>> process to identify a Notification Virtual Target.
>>
>> The XiveRouter is an abstract class which needs to be inherited from
>> to define a storage for the EAT, and other upcoming tables.
>>
>> Signed-off-by: Cédric Le Goater <address@hidden>
>> ---
>>  include/hw/ppc/xive.h      | 31 ++++++++++++++++
>>  include/hw/ppc/xive_regs.h | 50 +++++++++++++++++++++++++
>>  hw/intc/xive.c             | 76 ++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 157 insertions(+)
>>  create mode 100644 include/hw/ppc/xive_regs.h
>>
>> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
>> index 6770cffec67d..57ec9f84f527 100644
>> --- a/include/hw/ppc/xive.h
>> +++ b/include/hw/ppc/xive.h
>> @@ -141,6 +141,8 @@
>>  #define PPC_XIVE_H
>>  
>>  #include "hw/qdev-core.h"
>> +#include "hw/sysbus.h"
>> +#include "hw/ppc/xive_regs.h"
>>  
>>  /*
>>   * XIVE Fabric (Interface between Source and Router)
>> @@ -297,4 +299,33 @@ static inline void xive_source_irq_set(XiveSource 
>> *xsrc, uint32_t srcno,
>>      }
>>  }
>>  
>> +/*
>> + * XIVE Router
>> + */
>> +
>> +typedef struct XiveRouter {
>> +    SysBusDevice    parent;
> 
> I thought the plan was to make XiveRouter as well as XiveSource a
> TYPE_DEVICE descendent rather than a SysBusDevice?

We start talking about that, indeed, but then :

        https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg06407.html

I thought we concluded that it was going to get too complex.

Also, sPAPRXive is a direct descendant of XiveRouter and we want sPAPRXive 
on SysBus.

C.

> 
>> +} XiveRouter;
>> +
>> +#define TYPE_XIVE_ROUTER "xive-router"
>> +#define XIVE_ROUTER(obj)                                \
>> +    OBJECT_CHECK(XiveRouter, (obj), TYPE_XIVE_ROUTER)
>> +#define XIVE_ROUTER_CLASS(klass)                                        \
>> +    OBJECT_CLASS_CHECK(XiveRouterClass, (klass), TYPE_XIVE_ROUTER)
>> +#define XIVE_ROUTER_GET_CLASS(obj)                              \
>> +    OBJECT_GET_CLASS(XiveRouterClass, (obj), TYPE_XIVE_ROUTER)
>> +
>> +typedef struct XiveRouterClass {
>> +    SysBusDeviceClass parent;
>> +
>> +    /* XIVE table accessors */
>> +    int (*get_eas)(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
>> +                   XiveEAS *eas);
>> +} XiveRouterClass;
>> +
>> +void xive_eas_pic_print_info(XiveEAS *eas, uint32_t lisn, Monitor *mon);
>> +
>> +int xive_router_get_eas(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
>> +                        XiveEAS *eas);
>> +
>>  #endif /* PPC_XIVE_H */
>> diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
>> new file mode 100644
>> index 000000000000..15f2470ed9cc
>> --- /dev/null
>> +++ b/include/hw/ppc/xive_regs.h
>> @@ -0,0 +1,50 @@
>> +/*
>> + * QEMU PowerPC XIVE internal structure definitions
>> + *
>> + *
>> + * The XIVE structures are accessed by the HW and their format is
>> + * architected to be big-endian. Some macros are provided to ease
>> + * access to the different fields.
>> + *
>> + *
>> + * Copyright (c) 2016-2018, IBM Corporation.
>> + *
>> + * This code is licensed under the GPL version 2 or later. See the
>> + * COPYING file in the top-level directory.
>> + */
>> +
>> +#ifndef PPC_XIVE_REGS_H
>> +#define PPC_XIVE_REGS_H
>> +
>> +/*
>> + * Interrupt source number encoding on PowerBUS
>> + */
>> +#define XIVE_SRCNO_BLOCK(srcno) (((srcno) >> 28) & 0xf)
>> +#define XIVE_SRCNO_INDEX(srcno) ((srcno) & 0x0fffffff)
>> +#define XIVE_SRCNO(blk, idx)    ((uint32_t)(blk) << 28 | (idx))
>> +
>> +/* EAS (Event Assignment Structure)
>> + *
>> + * One per interrupt source. Targets an interrupt to a given Event
>> + * Notification Descriptor (END) and provides the corresponding
>> + * logical interrupt number (END data)
>> + */
>> +typedef struct XiveEAS {
>> +        /* Use a single 64-bit definition to make it easier to
>> +         * perform atomic updates
>> +         */
>> +        uint64_t        w;
>> +#define EAS_VALID       PPC_BIT(0)
>> +#define EAS_END_BLOCK   PPC_BITMASK(4, 7)        /* Destination END block# 
>> */
>> +#define EAS_END_INDEX   PPC_BITMASK(8, 31)       /* Destination END index */
>> +#define EAS_MASKED      PPC_BIT(32)              /* Masked */
>> +#define EAS_END_DATA    PPC_BITMASK(33, 63)      /* Data written to the END 
>> */
>> +} XiveEAS;
>> +
>> +#define xive_eas_is_valid(eas)   (be64_to_cpu((eas)->w) & EAS_VALID)
>> +#define xive_eas_is_masked(eas)  (be64_to_cpu((eas)->w) & EAS_MASKED)
>> +
>> +#define GETFIELD_BE64(m, v)      GETFIELD(m, be64_to_cpu(v))
>> +#define SETFIELD_BE64(m, v, val) cpu_to_be64(SETFIELD(m, be64_to_cpu(v), 
>> val))
>> +
>> +#endif /* PPC_XIVE_REGS_H */
>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
>> index 79238eb57fae..d21df6674d8c 100644
>> --- a/hw/intc/xive.c
>> +++ b/hw/intc/xive.c
>> @@ -443,6 +443,81 @@ static const TypeInfo xive_source_info = {
>>      .class_init    = xive_source_class_init,
>>  };
>>  
>> +/*
>> + * XIVE Router (aka. Virtualization Controller or IVRE)
>> + */
>> +
>> +int xive_router_get_eas(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
>> +                        XiveEAS *eas)
>> +{
>> +    XiveRouterClass *xrc = XIVE_ROUTER_GET_CLASS(xrtr);
>> +
>> +    return xrc->get_eas(xrtr, eas_blk, eas_idx, eas);
>> +}
> 
> Didn't spot this before, but it probably makes sense for such a
> trivial wrapper to be a static inline in the header.
> 
>> +
>> +static void xive_router_notify(XiveNotifier *xn, uint32_t lisn)
>> +{
>> +    XiveRouter *xrtr = XIVE_ROUTER(xn);
>> +    uint8_t eas_blk = XIVE_SRCNO_BLOCK(lisn);
>> +    uint32_t eas_idx = XIVE_SRCNO_INDEX(lisn);
>> +    XiveEAS eas;
>> +
>> +    /* EAS cache lookup */
>> +    if (xive_router_get_eas(xrtr, eas_blk, eas_idx, &eas)) {
>> +        qemu_log_mask(LOG_GUEST_ERROR, "XIVE: Unknown LISN %x\n", lisn);
>> +        return;
>> +    }
>> +
>> +    /* The IVRE checks the State Bit Cache at this point. We skip the
>> +     * SBC lookup because the state bits of the sources are modeled
>> +     * internally in QEMU.
>> +     */
>> +
>> +    if (!xive_eas_is_valid(&eas)) {
>> +        qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid LISN %x\n", lisn);
>> +        return;
>> +    }
>> +
>> +    if (xive_eas_is_masked(&eas)) {
>> +        /* Notification completed */
>> +        return;
>> +    }
>> +}
>> +
>> +static void xive_router_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +    XiveNotifierClass *xnc = XIVE_NOTIFIER_CLASS(klass);
>> +
>> +    dc->desc    = "XIVE Router Engine";
>> +    xnc->notify = xive_router_notify;
>> +}
>> +
>> +static const TypeInfo xive_router_info = {
>> +    .name          = TYPE_XIVE_ROUTER,
>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>> +    .abstract      = true,
>> +    .class_size    = sizeof(XiveRouterClass),
>> +    .class_init    = xive_router_class_init,
>> +    .interfaces    = (InterfaceInfo[]) {
>> +        { TYPE_XIVE_NOTIFIER },
>> +        { }
>> +    }
>> +};
>> +
>> +void xive_eas_pic_print_info(XiveEAS *eas, uint32_t lisn, Monitor *mon)
>> +{
>> +    if (!xive_eas_is_valid(eas)) {
>> +        return;
>> +    }
>> +
>> +    monitor_printf(mon, "  %08x %s end:%02x/%04x data:%08x\n",
>> +                   lisn, xive_eas_is_masked(eas) ? "M" : " ",
>> +                   (uint8_t)  GETFIELD_BE64(EAS_END_BLOCK, eas->w),
>> +                   (uint32_t) GETFIELD_BE64(EAS_END_INDEX, eas->w),
>> +                   (uint32_t) GETFIELD_BE64(EAS_END_DATA, eas->w));
>> +}
>> +
>>  /*
>>   * XIVE Fabric
>>   */
>> @@ -456,6 +531,7 @@ static void xive_register_types(void)
>>  {
>>      type_register_static(&xive_source_info);
>>      type_register_static(&xive_fabric_info);
>> +    type_register_static(&xive_router_info);
>>  }
>>  
>>  type_init(xive_register_types)
>