On Wed, Oct 26, 2016 at 09:13:18AM +0200, Cédric Le Goater wrote: > On 10/25/2016 07:08 AM, David Gibson wrote: > > On Sat, Oct 22, 2016 at 11:46:44AM +0200, Cédric Le Goater wrote: > >> This provides access to the MMIO based Interrupt Presentation > >> Controllers (ICP) as found on a POWER8 system. > >> > >> A new XICSNative class is introduced to hold the MMIO region of the > >> ICPs. Each thread of the system has a subregion, indexed by its PIR > >> number, holding a XIVE (External Interrupt Vector Entry). This > >> provides a mean to make the link with the ICPState of the CPU. > >> > >> Signed-off-by: Cédric Le Goater <c...@kaod.org> > >> --- > >> > >> Changes since v4: > >> > >> - replaced the pir_table by memory subregions using an ICP. > >> - removed the find_icp() and cpu_setup() handlers which became > >> useless with the memory regions. > >> - removed the superfluous inits done in xics_native_initfn. This is > >> covered in the parent class init. > >> - took ownership of the patch. > >> > >> default-configs/ppc64-softmmu.mak | 3 +- > >> hw/intc/Makefile.objs | 1 + > >> hw/intc/xics_native.c | 304 > >> ++++++++++++++++++++++++++++++++++++++ > >> include/hw/ppc/pnv.h | 19 +++ > >> include/hw/ppc/xics.h | 24 +++ > >> 5 files changed, 350 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 67a9bcaa67fa..a22c93a48686 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 2f44a2da26e9..e44a29d75b32 100644 > >> --- a/hw/intc/Makefile.objs > >> +++ b/hw/intc/Makefile.objs > >> @@ -34,6 +34,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 000000000000..bbdd786aeb50 > >> --- /dev/null > >> +++ b/hw/intc/xics_native.c > >> @@ -0,0 +1,304 @@ > >> +/* > >> + * 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 > >> <http://www.gnu.org/licenses/>. > >> + */ > >> + > >> +#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 <libfdt.h> > >> + > >> +static void xics_native_reset(void *opaque) > >> +{ > >> + device_reset(DEVICE(opaque)); > >> +} > >> + > >> +static void xics_native_initfn(Object *obj) > >> +{ > >> + qemu_register_reset(xics_native_reset, obj); > >> +} > > > > I think we need to investigate why the xics native is not showing up > > on the SysBus. As a "raw" MMIO device, it really should. > > Well, it has sysbus mmio region, but it is not created with qdev_create(...) > so it is not under sysbus and the reset does not get called. That is my > understanding of the problem. > > May be we shouldn't be using a sysbus mmio region ?
Yeah, maybe not. We don't really fit the sysbus model well.
I do kind of wonder if the xics object should be an mmio device at
all, or if just the individual ICPs should be. But that might make
for more trouble.
> > If it was, device_reset should be called without these shenannigans.
>
> yes.
>
>
> >> +
> >> +static uint64_t xics_native_read(void *opaque, hwaddr addr, unsigned
> >> width)
> >> +{
> >> + ICPState *icp = opaque;
> >> + bool byte0 = (width == 1 && (addr & 0x3) == 0);
> >> + uint64_t val = 0xffffffff;
> >> +
> >> + switch (addr & 0xffc) {
> >> + case 0: /* poll */
> >> + val = icp_ipoll(icp, NULL);
> >> + if (byte0) {
> >> + val >>= 24;
> >> + } else if (width != 4) {
> >> + goto bad_access;
> >> + }
> >> + break;
> >> + case 4: /* xirr */
> >> + if (byte0) {
> >> + val = icp_ipoll(icp, NULL) >> 24;
> >> + } else if (width == 4) {
> >> + val = icp_accept(icp);
> >> + } else {
> >> + goto bad_access;
> >> + }
> >> + break;
> >> + case 12:
> >> + if (byte0) {
> >> + val = icp->mfrr;
> >> + } else {
> >> + goto bad_access;
> >> + }
> >> + break;
> >> + case 16:
> >> + if (width == 4) {
> >> + val = icp->links[0];
> >> + } else {
> >> + goto bad_access;
> >> + }
> >> + break;
> >> + case 20:
> >> + if (width == 4) {
> >> + val = icp->links[1];
> >> + } else {
> >> + goto bad_access;
> >> + }
> >> + break;
> >> + case 24:
> >> + if (width == 4) {
> >> + val = icp->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)
> >> +{
> >> + ICPState *icp = opaque;
> >> + bool byte0 = (width == 1 && (addr & 0x3) == 0);
> >> +
> >> + switch (addr & 0xffc) {
> >> + case 4: /* xirr */
> >> + if (byte0) {
> >> + icp_set_cppr(icp, val);
> >> + } else if (width == 4) {
> >> + icp_eoi(icp, val);
> >> + } else {
> >> + goto bad_access;
> >> + }
> >> + break;
> >> + case 12:
> >> + if (byte0) {
> >> + icp_set_mfrr(icp, val);
> >> + } else {
> >> + goto bad_access;
> >> + }
> >> + break;
> >> + case 16:
> >> + if (width == 4) {
> >> + icp->links[0] = val;
> >> + } else {
> >> + goto bad_access;
> >> + }
> >> + break;
> >> + case 20:
> >> + if (width == 4) {
> >> + icp->links[1] = val;
> >> + } else {
> >> + goto bad_access;
> >> + }
> >> + break;
> >> + case 24:
> >> + if (width == 4) {
> >> + icp->links[2] = 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 = {
> >> + .read = xics_native_read,
> >> + .write = xics_native_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,
> >> +};
> >> +
> >> +static uint64_t xics_native_default_read(void *opaque, hwaddr addr,
> >> + unsigned width)
> >> +{
> >> + qemu_log_mask(LOG_GUEST_ERROR, "XICS: Bad ICP access 0x%"
> >> + HWADDR_PRIx"/%d\n", addr, width);
> >> + return 0xffffffffffffffffull;
> >> +}
> >> +
> >> +static void xics_native_default_write(void *opaque, hwaddr addr, uint64_t
> >> val,
> >> + unsigned width)
> >> +{
> >> + qemu_log_mask(LOG_GUEST_ERROR, "XICS: Bad ICP access 0x%"
> >> + HWADDR_PRIx"/%d\n", addr, width);
> >> +}
> >> +
> >> +static const MemoryRegionOps xics_native_default_ops = {
> >> + .read = xics_native_default_read,
> >> + .write = xics_native_default_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,
> >> +};
> >> +
> >> +static void xics_native_set_nr_servers(XICSState *xics, uint32_t
> >> nr_servers,
> >> + Error **errp)
> >> +{
> >> + xics_set_nr_servers(xics, nr_servers, TYPE_ICP, errp);
> >> +}
> >> +
> >> +static void xics_native_realize(DeviceState *dev, Error **errp)
> >> +{
> >> + XICSState *xics = XICS_COMMON(dev);
> >> + XICSNative *xicsn = XICS_NATIVE(dev);
> >> + Error *error = NULL;
> >> + int i;
> >> +
> >> + if (!xics->nr_servers) {
> >> + error_setg(errp, "Number of servers needs to be greater than 0");
> >> + return;
> >> + }
> >> +
> >> + for (i = 0; i < xics->nr_servers; i++) {
> >> + object_property_set_bool(OBJECT(&xics->ss[i]), true, "realized",
> >> + &error);
> >> + if (error) {
> >> + error_propagate(errp, error);
> >> + return;
> >> + }
> >> + }
> >> +
> >> + /*
> >> + * Initialize the container region for the ICPs and the subregion
> >> + * for each cpu. The mmapping will be done at the board level
> >> + * depending on the pir of the core.
> >> + *
> >> + * TODO: build a name with the chip id
> >> + */
> >> + memory_region_init_io(&xicsn->icp_mmio, OBJECT(dev),
> >> + &xics_native_default_ops, xicsn, "icp-0",
> >> + PNV_XICS_SIZE);
> >
> > I don't think you should need these native ops. I believe you can
> > construct a memory region as a "pure" container, then just put the
> > populated regions inside it.
>
> It is a way to track bogus read/writes the guest shouldn't be doing
> but it is not that important. I can remove it.
Right. I don't recall exactly what will happen if you don't populate
this at all, but I think you should eventually arrive at the global
fallback handler which should give you a similar effect.
--
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
signature.asc
Description: PGP signature
