On 8 April 2015 at 22:20, Christoffer Dall <christoffer.d...@linaro.org> wrote:
> The ARM GICv2m widget is a little device that handle MSI interrupt > "handles" > writes to a trigger register and ties them to a range of interrupt lines > wires to the GIC. It has a few status/id registers and the interrupt > wires, > and that's about it. > > A board instantiates the device by setting the base SPI number and > number SPIs for the frame. The base-spi parameter is indexed in the SPI > number space only, so base-spi == 0, means IRQ number 32. When a device > (the PCI host controller) writes to the trigger register, the payload is > the GIC IRQ number, so we have to subtract 32 from that and then index > into our frame of SPIs. > > When instantiating a GICv2m device, tell PCI that we have instantiated > something that can deal with MSIs. We rely on the board actually wiring > up the GICv2m to the PCI host controller. > > Signed-off-by: Christoffer Dall <christoffer.d...@linaro.org> > --- > hw/intc/Makefile.objs | 1 + > hw/intc/arm_gicv2m.c | 180 > ++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 181 insertions(+) > create mode 100644 hw/intc/arm_gicv2m.c > > diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs > index 843864a..6b63dfc 100644 > --- a/hw/intc/Makefile.objs > +++ b/hw/intc/Makefile.objs > @@ -15,6 +15,7 @@ common-obj-$(CONFIG_OPENPIC) += openpic.o > > obj-$(CONFIG_APIC) += apic.o apic_common.o > obj-$(CONFIG_ARM_GIC_KVM) += arm_gic_kvm.o > +obj-$(CONFIG_ARM_GIC) += arm_gicv2m.o > Should be common-obj-, as per other email. > obj-$(CONFIG_STELLARIS) += armv7m_nvic.o > obj-$(CONFIG_EXYNOS4) += exynos4210_gic.o exynos4210_combiner.o > obj-$(CONFIG_GRLIB) += grlib_irqmp.o > diff --git a/hw/intc/arm_gicv2m.c b/hw/intc/arm_gicv2m.c > new file mode 100644 > index 0000000..a80a16d > --- /dev/null > +++ b/hw/intc/arm_gicv2m.c > @@ -0,0 +1,180 @@ > +/* > + * GICv2m extension for MSI/MSI-x support with a GICv2-based system > + * > + * Copyright (C) 2015 Linaro, All rights reserved. > + * > + * Author: Christoffer Dall <christoffer.d...@linaro.org> > + * > + * 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/>. > + */ > A reference to the document defining the GICv2M would be nice here (it's the SBSA unless I'm confused). > + > +#include "hw/sysbus.h" > +#include "hw/pci/msi.h" > + > +#define TYPE_GICV2M "gicv2m" > I think this should be #define TYPE_ARM_GICV2M "arm-gicv2m" > +#define GICV2M(obj) OBJECT_CHECK(GICv2mState, (obj), TYPE_GICV2M) > and ARM_GICV2M(obj) > + > +#define GICV2M_NUM_SPI_MAX 128 > + > +#define V2M_MSI_TYPER 0x008 > +#define V2M_MSI_SETSPI_NS 0x040 > +#define V2M_MSI_IIDR 0xFCC > +#define V2M_IIDR0 0xFD0 > + > +#define PRODUCT_ID_QEMU 0x51 /* ASCII code Q */ > +#define IMPLEMENTER_ARM 0x43b > + > +typedef struct GICv2mState { > ARMGICv2mState + SysBusDevice parent_obj; > + > + MemoryRegion iomem; > + qemu_irq spi[GICV2M_NUM_SPI_MAX]; > + > + uint32_t base_spi; > + uint32_t num_spi; > +} GICv2mState; > + > +static void gicv2m_set_irq(void *opaque, int irq) > +{ > + GICv2mState *s = (GICv2mState *)opaque; > + > + qemu_irq_pulse(s->spi[irq]); > +} > + > +static uint64_t gicv2m_read(void *opaque, hwaddr offset, > + unsigned size) > +{ > + GICv2mState *s = (GICv2mState *)opaque; > + uint32_t val; > + > + if (size != 4) { > + qemu_log_mask(LOG_GUEST_ERROR, "gicv2m_read: bad size %u\n", > size); > + return 0; > + } > + > + switch (offset) { > + case V2M_MSI_TYPER: > + val = (s->base_spi + 32) << 16; > + val |= s->num_spi; > + return val; > + case V2M_MSI_IIDR: > + return (PRODUCT_ID_QEMU << 20) | IMPLEMENTER_ARM; > This choice of implementer field value is a little hard to justify :-) > + default: > + if (offset > V2M_IIDR0 && offset <= 0xFFC) { > Why not just have a "case 0xFD0..0xFFC" ? Could do with a comment about what the ID regs are (ie that they're mostly impdef and we choose to read as zero). + return 0; > + } > + > + qemu_log_mask(LOG_GUEST_ERROR, > + "gicv2m_read: Bad offset %x\n", (int)offset); > + return 0; > + } > +} > + > +static void gicv2m_write(void *opaque, hwaddr offset, > + uint64_t value, unsigned size) > +{ > + GICv2mState *s = (GICv2mState *)opaque; > + > + if (size != 4) { > + qemu_log_mask(LOG_GUEST_ERROR, "gicv2m_write: bad size %u\n", > size); > + return; > + } > "The MSI_SETSPI_S and MSI_SETSPI_NS registers must also support 16 bit writes to bits [15:0]", so you can't insist on 32 bit accesses only here. > + > + switch (offset) { > + case V2M_MSI_SETSPI_NS: { > + int spi; > + > + spi = (value & 0x3ff) - (s->base_spi + 32); > + if (spi < s->num_spi) { > + gicv2m_set_irq(s, spi); > + } > + return; > + } > + default: > + qemu_log_mask(LOG_GUEST_ERROR, > + "gicv2m_write: Bad offset %x\n", (int)offset); > + return; > + } > +} > + > +static const MemoryRegionOps gicv2m_ops = { > + .read = gicv2m_read, > + .write = gicv2m_write, > + .endianness = DEVICE_LITTLE_ENDIAN, > +}; > + > +static void gicv2m_realize(DeviceState *dev, Error **errp) > +{ > + GICv2mState *s = GICV2M(dev); > + int i; > + > + if (s->num_spi > GICV2M_NUM_SPI_MAX) { > + error_setg(errp, > + "requested %u SPIs exceeds GICv2m frame maximum %d", > + s->num_spi, GICV2M_NUM_SPI_MAX); > + return; > + } > + > + if (s->base_spi + 32 > 1020 - s->num_spi) { > + error_setg(errp, > + "requested base SPI %u+%u exceeds max. number 1020", > + s->base_spi + 32, s->num_spi); > + return; > + } > + > + for (i = 0; i < s->num_spi; i++) { > + sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->spi[i]); > + } > + > + msi_supported = true; > +} > + > +static void gicv2m_init(Object *obj) > +{ > + SysBusDevice *sbd = SYS_BUS_DEVICE(obj); > + GICv2mState *s = GICV2M(obj); > + > + memory_region_init_io(&s->iomem, OBJECT(s), &gicv2m_ops, s, > + "gicv2m", 0x1000); > + sysbus_init_mmio(sbd, &s->iomem); > +} > + > +static Property gicv2m_properties[] = { > + DEFINE_PROP_UINT32("base-spi", GICv2mState, base_spi, 0), > + DEFINE_PROP_UINT32("num-spi", GICv2mState, num_spi, 64), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > +static void gicv2m_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + > + dc->props = gicv2m_properties; > + dc->realize = gicv2m_realize; > +} > + > +static const TypeInfo gicv2m_info = { > + .name = TYPE_GICV2M, > + .parent = TYPE_SYS_BUS_DEVICE, > + .instance_size = sizeof(GICv2mState), > + .instance_init = gicv2m_init, > + .class_init = gicv2m_class_init, > +}; > + > +static void gicv2m_register_types(void) > +{ > + type_register_static(&gicv2m_info); > +} > + > +type_init(gicv2m_register_types) > -- > 2.1.2.330.g565301e.dirty Are we ever going to want to support the security extensions for gicv2m? If we do can we backwards-compatibly add it later? (I think the answer to that is probably 'yes' but I haven't thought through the details...) thanks -- PMM