Re: [Qemu-devel] [PATCH v4 7/9] ppc/xics: Add "native" XICS subclass

[Cédric Le Goater]

On 09/22/2016 02:02 AM, David Gibson wrote:
> On Mon, Sep 19, 2016 at 11:59:35AM +0530, Nikunj A Dadhania wrote:
>> From: Benjamin Herrenschmidt <address@hidden>
>>
>> This provides MMIO based ICP access as found on POWER8
>>
>> Signed-off-by: Benjamin Herrenschmidt <address@hidden>
>> Signed-off-by: Nikunj A Dadhania <address@hidden>
>> ---
>>  default-configs/ppc64-softmmu.mak |   3 +-
>>  hw/intc/Makefile.objs             |   1 +
>>  hw/intc/xics_native.c             | 295 
>> ++++++++++++++++++++++++++++++++++++++
>>  include/hw/ppc/xics.h             |  14 ++
>>  4 files changed, 312 insertions(+), 1 deletion(-)
>>  create mode 100644 hw/intc/xics_native.c
>>
>> diff --git a/default-configs/ppc64-softmmu.mak 
>> b/default-configs/ppc64-softmmu.mak
>> index c4be59f..315e30b 100644
>> --- a/default-configs/ppc64-softmmu.mak
>> +++ b/default-configs/ppc64-softmmu.mak
>> @@ -48,8 +48,9 @@ CONFIG_PLATFORM_BUS=y
>>  CONFIG_ETSEC=y
>>  CONFIG_LIBDECNUMBER=y
>>  # For pSeries
>> -CONFIG_XICS=$(CONFIG_PSERIES)
>> +CONFIG_XICS=$(or $(CONFIG_PSERIES),$(CONFIG_POWERNV))
>>  CONFIG_XICS_SPAPR=$(CONFIG_PSERIES)
>> +CONFIG_XICS_NATIVE=$(CONFIG_POWERNV)
>>  CONFIG_XICS_KVM=$(and $(CONFIG_PSERIES),$(CONFIG_KVM))
>>  # For PReP
>>  CONFIG_MC146818RTC=y
>> diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
>> index 05ec21b..7be5dfc 100644
>> --- a/hw/intc/Makefile.objs
>> +++ b/hw/intc/Makefile.objs
>> @@ -31,6 +31,7 @@ obj-$(CONFIG_RASPI) += bcm2835_ic.o bcm2836_control.o
>>  obj-$(CONFIG_SH4) += sh_intc.o
>>  obj-$(CONFIG_XICS) += xics.o
>>  obj-$(CONFIG_XICS_SPAPR) += xics_spapr.o
>> +obj-$(CONFIG_XICS_NATIVE) += xics_native.o
>>  obj-$(CONFIG_XICS_KVM) += xics_kvm.o
>>  obj-$(CONFIG_ALLWINNER_A10_PIC) += allwinner-a10-pic.o
>>  obj-$(CONFIG_S390_FLIC) += s390_flic.o
>> diff --git a/hw/intc/xics_native.c b/hw/intc/xics_native.c
>> new file mode 100644
>> index 0000000..26e45cc
>> --- /dev/null
>> +++ b/hw/intc/xics_native.c
>> @@ -0,0 +1,295 @@
>> +/*
>> + * QEMU PowerPC hardware System Emulator
>> + *
>> + * Native version of ICS/ICP
>> + *
>> + * Copyright (c) 2010,2011 David Gibson, IBM Corporation.
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a 
>> copy
>> + * of this software and associated documentation files (the "Software"), to 
>> deal
>> + * in the Software without restriction, including without limitation the 
>> rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included 
>> in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
>> OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
>> OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
>> FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>> + *
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "hw/hw.h"
>> +#include "trace.h"
>> +#include "qemu/timer.h"
>> +#include "hw/ppc/xics.h"
>> +#include "qapi/visitor.h"
>> +#include "qapi/error.h"
>> +
>> +#include <libfdt.h>
>> +
>> +/* #define DEBUG_MM(fmt...)      printf(fmt) */
>> +#define DEBUG_MM(fmt...)        do { } while (0)
>> +
>> +static void xics_native_initfn(Object *obj)
>> +{
>> +    XICSState *xics = XICS_NATIVE(obj);
>> +
>> +    QLIST_INIT(&xics->ics);
> 
> Isn't this redundant with the QLIST_INIT() in the common initfn?
> 
>> +}
>> +
>> +static uint64_t icp_mm_read(void *opaque, hwaddr addr, unsigned width)
>> +{
>> +    XICSState *s = opaque;
>> +    int32_t cpu_id, server;
>> +    uint32_t val;
>> +    ICPState *ss;
>> +    bool byte0 = (width == 1 && (addr & 0x3) == 0);
>> +
>> +    cpu_id = (addr & (ICP_MM_SIZE - 1)) >> 12;
>> +    server = get_cpu_index_by_dt_id(cpu_id);
>> +    if (server < 0) {
>> +        fprintf(stderr, "XICS: Bad ICP server %d\n", server);
> 
> Please use error_report() instead of raw fprintf()s.
> 
>> +        goto bad_access;
>> +    }
>> +    ss = &s->ss[server];
>> +
>> +    switch (addr & 0xffc) {
>> +    case 0: /* poll */
>> +        val = icp_ipoll(ss, NULL);
>> +        if (byte0) {
>> +            val >>= 24;
>> +        } else if (width != 4) {
>> +            goto bad_access;
>> +        }
>> +        break;
>> +    case 4: /* xirr */
>> +        if (byte0) {
>> +            val = icp_ipoll(ss, NULL) >> 24;
>> +        } else if (width == 4) {
>> +            val = icp_accept(ss);
>> +        } else {
>> +            goto bad_access;
>> +        }
>> +        break;
>> +    case 12:
>> +        if (byte0) {
>> +            val = ss->mfrr;
>> +        } else {
>> +            goto bad_access;
>> +        }
>> +        break;
>> +    case 16:
>> +        if (width == 4) {
>> +            val = ss->links[0];
>> +        } else {
>> +            goto bad_access;
>> +        }
>> +        break;
>> +    case 20:
>> +        if (width == 4) {
>> +            val = ss->links[1];
>> +        } else {
>> +            goto bad_access;
>> +        }
>> +        break;
>> +    case 24:
>> +        if (width == 4) {
>> +            val = ss->links[2];
>> +        } else {
>> +            goto bad_access;
>> +        }
>> +        break;
>> +    default:
>> +bad_access:
>> +        fprintf(stderr, "XICS: Bad ICP access %llx/%d\n",
>> +                (unsigned long long)addr, width);
>> +        val = 0xffffffff;
>> +    }
>> +    
>> DEBUG_MM("icp_mm_read(addr=%016llx,serv=0x%x/%d,off=%d,w=%d,val=0x%08x)\n",
>> +             (unsigned long long)addr, cpu_id, server, (int)(addr & 0xffc),
>> +             width, val);
> 
> It's preferred to use trace-events rather than DEBUG macros.
> 
>> +
>> +    return val;
>> +}
>> +
>> +static void icp_mm_write(void *opaque, hwaddr addr, uint64_t val,
>> +                        unsigned width)
>> +{
>> +    XICSState *s = opaque;
>> +    int32_t cpu_id, server;
>> +    ICPState *ss;
>> +    bool byte0 = (width == 1 && (addr & 0x3) == 0);
>> +
>> +    cpu_id = (addr & (ICP_MM_SIZE - 1)) >> 12;
>> +    server = get_cpu_index_by_dt_id(cpu_id);
>> +    if (server < 0) {
>> +        fprintf(stderr, "XICS: Bad ICP server %d\n", server);
>> +        goto bad_access;
>> +    }
>> +    ss = &s->ss[server];
>> +
>> +    
>> DEBUG_MM("icp_mm_write(addr=%016llx,serv=0x%x/%d,off=%d,w=%d,val=0x%08x)\n",
>> +             (unsigned long long)addr, cpu_id, server,
>> +             (int)(addr & 0xffc), width, (uint32_t)val);
>> +
>> +    switch (addr & 0xffc) {
>> +    case 4: /* xirr */
>> +        if (byte0) {
>> +            icp_set_cppr(s, server, val);
>> +        } else if (width == 4) {
>> +            icp_eoi(s, server, val);
>> +        } else {
>> +            goto bad_access;
>> +        }
>> +        break;
>> +    case 12:
>> +        if (byte0) {
>> +            icp_set_mfrr(s, server, val);
>> +        } else {
>> +            goto bad_access;
>> +        }
>> +        break;
>> +    case 16:
>> +        if (width == 4) {
>> +            ss->links[0] = val;
>> +        } else {
>> +            goto bad_access;
>> +        }
>> +        break;
>> +    case 20:
>> +        if (width == 4) {
>> +            ss->links[1] = val;
>> +        } else {
>> +            goto bad_access;
>> +        }
>> +        break;
>> +    case 24:
>> +        if (width == 4) {
>> +            ss->links[2] = val;
>> +        } else {
>> +            goto bad_access;
>> +        }
>> +        break;
>> +    default:
>> + bad_access:
>> +        val = 0xffffffff;
>> +    }
>> +}
>> +
>> +static const MemoryRegionOps icp_mm_ops = {
>> +    .read = icp_mm_read,
>> +    .write = icp_mm_write,
>> +    .valid.min_access_size = 1,
>> +    .valid.max_access_size = 4,
>> +    .impl.min_access_size = 1,
>> +    .impl.max_access_size = 4,
>> +    .endianness = DEVICE_BIG_ENDIAN,
>> +};
>> +
>> +#define _FDT(exp) \
>> +    do { \
>> +        int ret = (exp);                                           \
>> +        if (ret < 0) {                                             \
>> +            fprintf(stderr, "qemu: error creating device tree: %s: %s\n", \
>> +                    #exp, fdt_strerror(ret));                      \
>> +            exit(1);                                               \
>> +        }                                                          \
>> +    } while (0)
>> +
>> +void xics_create_native_icp_node(XICSState *s, void *fdt,
>> +                                 uint32_t base, uint32_t count)
>> +{
>> +    uint64_t addr;
>> +    char *name;
>> +    const char compat[] = "IBM,power8-icp\0IBM,ppc-xicp";
>> +    uint32_t irange[2], i, rsize;
>> +    uint64_t *reg;
>> +
>> +    addr = ICP_MM_BASE | (base << 12);
>> +
>> +    irange[0] = cpu_to_be32(base);
>> +    irange[1] = cpu_to_be32(count);
>> +
>> +    rsize = sizeof(uint64_t) * 2 * count;
>> +    reg = g_malloc(rsize);
>> +    for (i = 0; i < count; i++) {
>> +        reg[i * 2] = cpu_to_be64(addr | ((base + i) * 0x1000));
>> +        reg[i * 2 + 1] = cpu_to_be64(0x1000);
>> +    }
>> +
>> +    name = g_strdup_printf("address@hidden"PRIX64, addr);
>> +
>> +    /* interrupt controller */
>> +    _FDT((fdt_begin_node(fdt, name)));
>> +    g_free(name);
>> +
>> +    _FDT((fdt_property(fdt, "compatible", compat, sizeof(compat))));
>> +    _FDT((fdt_property(fdt, "reg", reg, rsize)));
>> +    _FDT((fdt_property_string(fdt, "device_type",
>> +                              "PowerPC-External-Interrupt-Presentation")));
>> +    _FDT((fdt_property(fdt, "interrupt-controller", NULL, 0)));
>> +    _FDT((fdt_property(fdt, "ibm,interrupt-server-ranges",
>> +                       irange, sizeof(irange))));
>> +    _FDT((fdt_property_cell(fdt, "#interrupt-cells", 1)));
>> +    _FDT((fdt_property_cell(fdt, "#address-cells", 0)));
>> +    _FDT((fdt_end_node(fdt)));
>> +}
>> +
>> +static void xics_native_realize(DeviceState *dev, Error **errp)
>> +{
>> +    XICSState *s = XICS_NATIVE(dev);
>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> +    Error *error = NULL;
>> +    int i;
>> +
>> +    if (!s->nr_servers) {
>> +        error_setg(errp, "Number of servers needs to be greater 0");
>> +        return;
>> +    }
>> +
>> +    /* Register MMIO regions */
>> +    memory_region_init_io(&s->icp_mmio, OBJECT(s), &icp_mm_ops, s, "icp",
>> +                          ICP_MM_SIZE);
>> +    sysbus_init_mmio(sbd, &s->icp_mmio);
>> +    sysbus_mmio_map(sbd, 0, ICP_MM_BASE);
>> +
>> +    for (i = 0; i < s->nr_servers; i++) {
>> +        object_property_set_bool(OBJECT(&s->ss[i]), true, "realized", 
>> &error);
>> +        if (error) {
>> +            error_propagate(errp, error);
>> +            return;
>> +        }
>> +    }
>> +}
>> +
>> +static void xics_native_class_init(ObjectClass *oc, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(oc);
>> +    XICSStateClass *xsc = XICS_NATIVE_CLASS(oc);
>> +
>> +    dc->realize = xics_native_realize;
>> +    xsc->set_nr_servers = xics_set_nr_servers;
>> +}
>> +
>> +static const TypeInfo xics_native_info = {
>> +    .name          = TYPE_XICS_NATIVE,
>> +    .parent        = TYPE_XICS_COMMON,
>> +    .instance_size = sizeof(XICSState),
>> +    .class_size = sizeof(XICSStateClass),
>> +    .class_init    = xics_native_class_init,
>> +    .instance_init = xics_native_initfn,
>> +};
>> +
>> +static void xics_native_register_types(void)
>> +{
>> +    type_register_static(&xics_native_info);
>> +}
>> +type_init(xics_native_register_types)
>> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
>> index 2231f2a..f81155c 100644
>> --- a/include/hw/ppc/xics.h
>> +++ b/include/hw/ppc/xics.h
>> @@ -44,6 +44,9 @@
>>  #define XICS_SPAPR_KVM(obj) \
>>       OBJECT_CHECK(KVMXICSState, (obj), TYPE_XICS_SPAPR_KVM)
>>  
>> +#define TYPE_XICS_NATIVE "xics-native"
>> +#define XICS_NATIVE(obj) OBJECT_CHECK(XICSState, (obj), TYPE_XICS_NATIVE)
>> +
>>  #define XICS_COMMON_CLASS(klass) \
>>       OBJECT_CLASS_CHECK(XICSStateClass, (klass), TYPE_XICS_COMMON)
>>  #define XICS_SPAPR_CLASS(klass) \
>> @@ -52,6 +55,8 @@
>>       OBJECT_GET_CLASS(XICSStateClass, (obj), TYPE_XICS_COMMON)
>>  #define XICS_SPAPR_GET_CLASS(obj) \
>>       OBJECT_GET_CLASS(XICSStateClass, (obj), TYPE_XICS_SPAPR)
>> +#define XICS_NATIVE_CLASS(klass) \
>> +     OBJECT_CLASS_CHECK(XICSStateClass, (klass), TYPE_XICS_NATIVE)
>>  
>>  #define XICS_IPI        0x2
>>  #define XICS_BUID       0x1
>> @@ -86,6 +91,7 @@ struct XICSState {
>>      uint32_t nr_irqs;
>>      ICPState *ss;
>>      QLIST_HEAD(, ICSState) ics;
>> +    MemoryRegion icp_mmio;
> 
> Shouldn't this just be in the native subclass?

yes. 

Nikunj,

I am reworking quite a bit this patch in PowerNV to handle 
the cpu index -> icp index mapping and for an other reasons. 
I will push another version of XICS native in sync with the 
code base. 

Thanks,

C. 


> 
>>  };
>>  
>>  #define TYPE_ICP "icp"
>> @@ -117,8 +123,13 @@ struct ICPState {
>>      uint8_t mfrr;
>>      qemu_irq output;
>>      bool cap_irq_xics_enabled;
>> +    uint32_t links[3];
> 
> Likewise here.
> 
>>  };
>>  
>> +/* This should be an XSCOM BAR ... the size is arbitrary as well */
>> +#define ICP_MM_BASE     0x0003FFFF80000000
>> +#define ICP_MM_SIZE     0x0000000010000000
>> +
>>  #define TYPE_ICS_BASE "ics-base"
>>  #define ICS_BASE(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_SIMPLE)
>>  
>> @@ -189,6 +200,9 @@ void xics_spapr_free(XICSState *icp, int irq, int num);
>>  void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu);
>>  void xics_cpu_destroy(XICSState *icp, PowerPCCPU *cpu);
>>  
>> +void xics_create_native_icp_node(XICSState *s, void *fdt,
>> +                                 uint32_t base, uint32_t count);
>> +
>>  /* Internal XICS interfaces */
>>  int xics_get_cpu_index_by_dt_id(int cpu_dt_id);
>>  
>