From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44016) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1buwWT-0002CY-Ng for qemu-devel@nongnu.org; Fri, 14 Oct 2016 03:03:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1buwWQ-0005DQ-Bz for qemu-devel@nongnu.org; Fri, 14 Oct 2016 03:03:17 -0400 Date: Fri, 14 Oct 2016 17:10:50 +1100 From: David Gibson Message-ID: <20161014061050.GQ28562@umbus> References: <1475479496-16158-1-git-send-email-clg@kaod.org> <1475479496-16158-16-git-send-email-clg@kaod.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="lG9v85r552aFjg4G" Content-Disposition: inline In-Reply-To: <1475479496-16158-16-git-send-email-clg@kaod.org> Subject: Re: [Qemu-devel] [PATCH v4 15/20] ppc/xics: Add "native" XICS subclass List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?iso-8859-1?Q?C=E9dric?= Le Goater Cc: qemu-ppc@nongnu.org, Benjamin Herrenschmidt , qemu-devel@nongnu.org --lG9v85r552aFjg4G Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Oct 03, 2016 at 09:24:51AM +0200, C=E9dric Le Goater wrote: > From: Benjamin Herrenschmidt >=20 > This provides access to the MMIO based Interrupt Presentation > Controllers (ICP) as found on a POWER8 system. >=20 > A new XICSNative class is introduced to hold the MMIO region of the > ICPs. It also makes use of a hash table to associate the ICPState of a > CPU with a HW processor id, as this is the server number presented in > the XIVEs. >=20 > The class routine 'find_icp' provide the way to do the lookups when > needed in the XICS base class. >=20 > Signed-off-by: Benjamin Herrenschmidt > [clg: - naming cleanups > - replaced the use of xics_get_cpu_index_by_dt_id() by xics_find_ic= p() > - added some qemu logging in case of error =20 > - introduced a xics_native_find_icp routine to map icp index to > cpu index =20 > - moved sysbus mapping to chip ] > Signed-off-by: C=E9dric Le Goater > --- >=20 > checkpatch complains on this one, but it seems to be a false positive : > =20 > ERROR: spaces required around that '&' (ctx:WxV) > #314: FILE: hw/intc/xics_native.c:246: > + (gpointer) &xics->ss[cs->cpu_index]); >=20 > default-configs/ppc64-softmmu.mak | 3 +- > hw/intc/Makefile.objs | 1 + > hw/intc/xics_native.c | 327 ++++++++++++++++++++++++++++++++= ++++++ > include/hw/ppc/pnv.h | 19 +++ > include/hw/ppc/xics.h | 24 +++ > 5 files changed, 373 insertions(+), 1 deletion(-) > create mode 100644 hw/intc/xics_native.c >=20 > diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-so= ftmmu.mak > index 67a9bcaa67fa..a22c93a48686 100644 > --- a/default-configs/ppc64-softmmu.mak > +++ b/default-configs/ppc64-softmmu.mak > @@ -48,8 +48,9 @@ CONFIG_PLATFORM_BUS=3Dy > CONFIG_ETSEC=3Dy > CONFIG_LIBDECNUMBER=3Dy > # For pSeries > -CONFIG_XICS=3D$(CONFIG_PSERIES) > +CONFIG_XICS=3D$(or $(CONFIG_PSERIES),$(CONFIG_POWERNV)) > CONFIG_XICS_SPAPR=3D$(CONFIG_PSERIES) > +CONFIG_XICS_NATIVE=3D$(CONFIG_POWERNV) > CONFIG_XICS_KVM=3D$(and $(CONFIG_PSERIES),$(CONFIG_KVM)) > # For PReP > CONFIG_MC146818RTC=3Dy > diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs > index 05ec21b21e0e..7be5dfc8347b 100644 > --- a/hw/intc/Makefile.objs > +++ b/hw/intc/Makefile.objs > @@ -31,6 +31,7 @@ obj-$(CONFIG_RASPI) +=3D bcm2835_ic.o bcm2836_control.o > obj-$(CONFIG_SH4) +=3D sh_intc.o > obj-$(CONFIG_XICS) +=3D xics.o > obj-$(CONFIG_XICS_SPAPR) +=3D xics_spapr.o > +obj-$(CONFIG_XICS_NATIVE) +=3D xics_native.o > obj-$(CONFIG_XICS_KVM) +=3D xics_kvm.o > obj-$(CONFIG_ALLWINNER_A10_PIC) +=3D allwinner-a10-pic.o > obj-$(CONFIG_S390_FLIC) +=3D s390_flic.o > diff --git a/hw/intc/xics_native.c b/hw/intc/xics_native.c > new file mode 100644 > index 000000000000..16413d807f65 > --- /dev/null > +++ b/hw/intc/xics_native.c > @@ -0,0 +1,327 @@ > +/* > + * QEMU PowerPC PowerNV machine model > + * > + * Native version of ICS/ICP > + * > + * Copyright (c) 2016, IBM Corporation. > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, see . > + */ > + > +#include "qemu/osdep.h" > +#include "qapi/error.h" > +#include "qemu-common.h" > +#include "cpu.h" > +#include "hw/hw.h" > +#include "qemu/log.h" > +#include "qapi/error.h" > + > +#include "hw/ppc/fdt.h" > +#include "hw/ppc/xics.h" > +#include "hw/ppc/pnv.h" > + > +#include > + > +static void xics_native_reset(void *opaque) > +{ > + device_reset(DEVICE(opaque)); > +} > + > +static void xics_native_initfn(Object *obj) > +{ > + XICSState *xics =3D XICS_COMMON(obj); > + > + QLIST_INIT(&xics->ics); This shouldn't be necessary, because it's done in the base class initfn (which is called before the subclass initfns). > + /* > + * Let's not forget to register a reset handler else the ICPs > + * won't be initialized with the correct values. Trouble ahead ! > + */ > + qemu_register_reset(xics_native_reset, xics); And. this shouldn't be necessary. If you set dc->reset to the right thing in class_init (which the xics base class should already do) then the device model will automatically reset the device, you shouldn't need an extra reset handler. > +} > + > +static uint64_t xics_native_read(void *opaque, hwaddr addr, unsigned wid= th) > +{ > + XICSState *s =3D opaque; > + uint32_t cpu_id =3D (addr & (PNV_XICS_SIZE - 1)) >> 12; > + bool byte0 =3D (width =3D=3D 1 && (addr & 0x3) =3D=3D 0); > + uint64_t val =3D 0xffffffff; > + ICPState *ss; > + > + ss =3D xics_find_icp(s, cpu_id); IIUC, cpu_id is the hardware id here... so why aren't you using the hash table you added to do this mapping? (although see comments elswhere, that I'm not sure that mapping belongs within xics). Another option to avoid the lookup would be to register each icp page as a separate MR, then you could set the opaque pointer directly to ss instead of to the global xics. > + if (!ss) { > + qemu_log_mask(LOG_GUEST_ERROR, "XICS: Bad ICP server %d\n", cpu_= id); > + return val; > + } > + > + switch (addr & 0xffc) { > + case 0: /* poll */ > + val =3D icp_ipoll(ss, NULL); > + if (byte0) { > + val >>=3D 24; > + } else if (width !=3D 4) { > + goto bad_access; > + } > + break; > + case 4: /* xirr */ > + if (byte0) { > + val =3D icp_ipoll(ss, NULL) >> 24; > + } else if (width =3D=3D 4) { > + val =3D icp_accept(ss); > + } else { > + goto bad_access; > + } > + break; > + case 12: > + if (byte0) { > + val =3D ss->mfrr; > + } else { > + goto bad_access; > + } > + break; > + case 16: > + if (width =3D=3D 4) { > + val =3D ss->links[0]; > + } else { > + goto bad_access; > + } > + break; > + case 20: > + if (width =3D=3D 4) { > + val =3D ss->links[1]; > + } else { > + goto bad_access; > + } > + break; > + case 24: > + if (width =3D=3D 4) { > + val =3D ss->links[2]; > + } else { > + goto bad_access; > + } > + break; > + default: > +bad_access: > + qemu_log_mask(LOG_GUEST_ERROR, "XICS: Bad ICP access 0x%" > + HWADDR_PRIx"/%d\n", addr, width); > + } > + > + return val; > +} > + > +static void xics_native_write(void *opaque, hwaddr addr, uint64_t val, > + unsigned width) > +{ > + XICSState *s =3D opaque; > + uint32_t cpu_id =3D (addr & (PNV_XICS_SIZE - 1)) >> 12; > + bool byte0 =3D (width =3D=3D 1 && (addr & 0x3) =3D=3D 0); > + ICPState *ss; > + > + ss =3D xics_find_icp(s, cpu_id); > + if (!ss) { > + qemu_log_mask(LOG_GUEST_ERROR, "XICS: Bad ICP server %d\n", cpu_= id); > + return; > + } > + > + switch (addr & 0xffc) { > + case 4: /* xirr */ > + if (byte0) { > + icp_set_cppr(s, cpu_id, val); > + } else if (width =3D=3D 4) { > + icp_eoi(s, cpu_id, val); > + } else { > + goto bad_access; > + } > + break; > + case 12: > + if (byte0) { > + icp_set_mfrr(s, cpu_id, val); > + } else { > + goto bad_access; > + } > + break; > + case 16: > + if (width =3D=3D 4) { > + ss->links[0] =3D val; > + } else { > + goto bad_access; > + } > + break; > + case 20: > + if (width =3D=3D 4) { > + ss->links[1] =3D val; > + } else { > + goto bad_access; > + } > + break; > + case 24: > + if (width =3D=3D 4) { > + ss->links[2] =3D val; > + } else { > + goto bad_access; > + } > + break; > + default: > +bad_access: > + qemu_log_mask(LOG_GUEST_ERROR, "XICS: Bad ICP access 0x%" > + HWADDR_PRIx"/%d\n", addr, width); > + } > +} > + > +static const MemoryRegionOps xics_native_ops =3D { > + .read =3D xics_native_read, > + .write =3D xics_native_write, > + .valid.min_access_size =3D 1, > + .valid.max_access_size =3D 4, > + .impl.min_access_size =3D 1, > + .impl.max_access_size =3D 4, > + .endianness =3D DEVICE_BIG_ENDIAN, > +}; > + > +static void xics_set_nr_servers(XICSState *icp, uint32_t nr_servers, > + Error **errp) > +{ > + int i; > + > + icp->nr_servers =3D nr_servers; > + > + icp->ss =3D g_malloc0(icp->nr_servers * sizeof(ICPState)); > + for (i =3D 0; i < icp->nr_servers; i++) { > + char name[32]; > + object_initialize(&icp->ss[i], sizeof(icp->ss[i]), TYPE_ICP); > + snprintf(name, sizeof(name), "icp[%d]", i); > + object_property_add_child(OBJECT(icp), name, OBJECT(&icp->ss[i]), > + errp); AFAICT this is identical to xics-spapr version and only difference to xics-kvm is it uses TYPE_KVM_ICP. We should look at fusing these - maybe just having an icp typename in the xics class. > + } > +} > + > +static void xics_native_realize(DeviceState *dev, Error **errp) > +{ > + XICSState *xics =3D XICS_COMMON(dev); > + XICSNative *xicsn =3D XICS_NATIVE(dev); > + Error *error =3D NULL; > + int i; > + > + if (!xics->nr_servers) { > + error_setg(errp, "Number of servers needs to be greater than 0"); > + return; > + } > + > + for (i =3D 0; i < xics->nr_servers; i++) { > + object_property_set_bool(OBJECT(&xics->ss[i]), true, "realized", > + &error); > + if (error) { > + error_propagate(errp, error); > + return; > + } > + } > + > + xicsn->pir_table =3D g_hash_table_new(g_direct_hash, g_direct_equal); I'm not sure having this map inside xics makes sense. Wouldn't it be more widely useful to have a fast PIR -> cpu_index map at the machine level? That could also use the structure of the hwids to do a fast lookup neatly (collapse core id with a small table, recombine with chip and thread id). A hash table seems a big hammer to throw at it. > + /* Register MMIO regions */ > + memory_region_init_io(&xicsn->icp_mmio, OBJECT(dev), &xics_native_op= s, > + xicsn, "xics", PNV_XICS_SIZE); > + sysbus_init_mmio(SYS_BUS_DEVICE(dev), &xicsn->icp_mmio); > +} > + > +static void xics_native_cpu_setup(XICSState *xics, PowerPCCPU *cpu) > +{ > + CPUState *cs =3D CPU(cpu); > + CPUPPCState *env =3D &cpu->env; > + XICSNative *xicsn =3D XICS_NATIVE(xics); > + > + assert(cs->cpu_index < xics->nr_servers); > + g_hash_table_insert(xicsn->pir_table, GINT_TO_POINTER(env->spr[SPR_P= IR]), > + (gpointer) &xics->ss[cs->cpu_index]); > +} > + > +static ICPState *xics_native_find_icp(XICSState *xics, int pir) > +{ > + XICSNative *xicsn =3D XICS_NATIVE(xics); > + gpointer key, value; > + gboolean found =3D g_hash_table_lookup_extended(xicsn->pir_table, > + GINT_TO_POINTER(pir), &key, &value); > + > + assert(found); > + > + return (ICPState *) value; > +} > + > +static void xics_native_class_init(ObjectClass *oc, void *data) > +{ > + DeviceClass *dc =3D DEVICE_CLASS(oc); > + XICSStateClass *xsc =3D XICS_NATIVE_CLASS(oc); > + > + dc->realize =3D xics_native_realize; > + xsc->set_nr_servers =3D xics_set_nr_servers; > + xsc->cpu_setup =3D xics_native_cpu_setup; > + xsc->find_icp =3D xics_native_find_icp; > +} > + > +static const TypeInfo xics_native_info =3D { > + .name =3D TYPE_XICS_NATIVE, > + .parent =3D TYPE_XICS_COMMON, > + .instance_size =3D sizeof(XICSNative), > + .class_size =3D sizeof(XICSStateClass), > + .class_init =3D xics_native_class_init, > + .instance_init =3D xics_native_initfn, > +}; > + > +static void xics_native_register_types(void) > +{ > + type_register_static(&xics_native_info); > +} > + > +type_init(xics_native_register_types) > + > +void xics_native_populate_icp(PnvChip *chip, void *fdt, int offset, > + uint32_t pir, uint32_t count) > +{ > + uint64_t addr; > + char *name; > + const char compat[] =3D "IBM,power8-icp\0IBM,ppc-xicp"; > + uint32_t irange[2], i, rsize; > + uint64_t *reg; > + > + /* > + * TODO: add multichip ICP BAR > + */ > + addr =3D PNV_XICS_BASE | (pir << 12); > + > + irange[0] =3D cpu_to_be32(pir); > + irange[1] =3D cpu_to_be32(count); > + > + rsize =3D sizeof(uint64_t) * 2 * count; > + reg =3D g_malloc(rsize); > + for (i =3D 0; i < count; i++) { > + reg[i * 2] =3D cpu_to_be64(addr | ((pir + i) * 0x1000)); > + reg[i * 2 + 1] =3D cpu_to_be64(0x1000); > + } > + > + name =3D g_strdup_printf("interrupt-controller@%"PRIX64, addr); > + offset =3D fdt_add_subnode(fdt, offset, name); > + _FDT(offset); > + g_free(name); > + > + _FDT((fdt_setprop(fdt, offset, "compatible", compat, sizeof(compat))= )); > + _FDT((fdt_setprop(fdt, offset, "reg", reg, rsize))); > + _FDT((fdt_setprop_string(fdt, offset, "device_type", > + "PowerPC-External-Interrupt-Presentation")= )); > + _FDT((fdt_setprop(fdt, offset, "interrupt-controller", NULL, 0))); > + _FDT((fdt_setprop(fdt, offset, "ibm,interrupt-server-ranges", > + irange, sizeof(irange)))); > + _FDT((fdt_setprop_cell(fdt, offset, "#interrupt-cells", 1))); > + _FDT((fdt_setprop_cell(fdt, offset, "#address-cells", 0))); > + g_free(reg); > +} > diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h > index 617c3fdd4f06..3f24b87d199b 100644 > --- a/include/hw/ppc/pnv.h > +++ b/include/hw/ppc/pnv.h > @@ -125,4 +125,23 @@ typedef struct PnvMachineState { > #define PNV_XSCOM_BASE(chip) \ > (chip->xscom_base + ((uint64_t)(chip)->chip_id) * PNV_XSCOM_SIZE) > =20 > +/* > + * XSCOM 0x20109CA defines the ICP BAR: > + * > + * 0:29 : bits 14 to 43 of address to define 1 MB region. > + * 30 : 1 to enable ICP to receive loads/stores against its BAR regi= on > + * 31:63 : Constant 0 > + * > + * Usually defined as : > + * > + * 0xffffe00200000000 -> 0x0003ffff80000000 > + * 0xffffe00600000000 -> 0x0003ffff80100000 > + * 0xffffe02200000000 -> 0x0003ffff80800000 > + * 0xffffe02600000000 -> 0x0003ffff80900000 > + * > + * TODO: make a macro using the chip hw id ? > + */ > +#define PNV_XICS_BASE 0x0003ffff80000000ull > +#define PNV_XICS_SIZE 0x0000000000100000ull > + > #endif /* _PPC_PNV_H */ > diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h > index dea9b92d4726..77ce786f000e 100644 > --- a/include/hw/ppc/xics.h > +++ b/include/hw/ppc/xics.h > @@ -118,8 +118,27 @@ struct ICPState { > uint8_t mfrr; > qemu_irq output; > bool cap_irq_xics_enabled; > + > + /* > + * for XICSNative (not used by Linux). > + */ > + uint32_t links[3]; > }; > =20 > +#define TYPE_XICS_NATIVE "xics-native" > +#define XICS_NATIVE(obj) OBJECT_CHECK(XICSNative, (obj), TYPE_XICS_NATIV= E) > +#define XICS_NATIVE_CLASS(klass) \ > + OBJECT_CLASS_CHECK(XICSStateClass, (klass), TYPE_XICS_NATIVE) > +#define XICS_NATIVE_GET_CLASS(obj) \ > + OBJECT_CLASS_CHECK(XICSStateClass, (obj), TYPE_XICS_NATIVE) > + > +typedef struct XICSNative { > + XICSState parent_obj; > + > + GHashTable *pir_table; > + MemoryRegion icp_mmio; > +} XICSNative; > + > #define TYPE_ICS_BASE "ics-base" > #define ICS_BASE(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_BASE) > =20 > @@ -210,4 +229,9 @@ void xics_hmp_info_pic(Monitor *mon, const QDict *qdi= ct); > ICPState *xics_find_icp(XICSState *xics, int cpu_index); > void xics_insert_ics(XICSState *xics, ICSState *ics); > =20 > +typedef struct PnvChip PnvChip; > + > +void xics_native_populate_icp(PnvChip *chip, void *fdt, int offset, > + uint32_t base, uint32_t count); > + > #endif /* XICS_H */ --=20 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 --lG9v85r552aFjg4G Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYAHbnAAoJEGw4ysog2bOSiLIQAITmBa3Z12U6OFQc5pL+YY59 +v1PCskORjwakA+E9WWwI9vHRErWMQuOjjxxP2GoBe8dx8wzgJQRKMCCRz6/nY1R 2noK8ZEfs/mT2Mpa/0kWqkzaziuSb+Slbd9XwZkacj9iG/+66x/M2WBnYnLC9wky eMxs7M/lFBUTYlXEOvYFh0mQZCt4MKirDqX+5pHL4HQzRr/hPTOM8GZ7rYz/1P2e DIDgMtCTe2ylgfyhe012Q29QGAsqGkrMA4JckzJP2+SsLDkDK2T+SxSyMnTWs6wF ymiXpmgthLwvPQpDJ25IyR4NyIlVytV33AMsZJf3aIo+XDYGV/COINjslsM19EJD VvvjbyImNT2oCYRvG5HULognb0U5YriXfWGjuQGhM2xg/sI5vzI9wAuFSf/5gxYi t04oisDb+8CtVWtSWh1UeHAP9OimsVX2sfEdzM/GtfYNO1i/2uRJHImMgipH/Z93 +ZutssjDfC5VuUttjyrExER/t3NeJcUilGPQiWoYWa5YU3YfFCkZPE0MjsQrbJCw rEWqhxpSXtlWJ1Apx17nda3N/qtzWzn3QKjQDQBU2NpuH+YTpl/joHyhW3k1seWO I2S/cqpnkyWf9wjB1vVcGHrkh9sHTO67i28/GH6yPlKoR5DvzRbE85naZ2zKyyGo dKJ8zch3vwa4cTWf/U/n =TjF1 -----END PGP SIGNATURE----- --lG9v85r552aFjg4G--