Hi Peter, On 12/08/2016 16:12, Peter Maydell wrote: > On 2 August 2016 at 19:07, Eric Auger <eric.au...@redhat.com> wrote: >> From: Pavel Fedin <p.fe...@samsung.com> >> >> This is the basic skeleton for both KVM and software-emulated ITS. >> Since we already prepare status structure, we also introduce complete >> VMState description. But, because we currently have no migratable >> implementations, we also set unmigratable flag. >> >> Signed-off-by: Pavel Fedin <p.fe...@samsung.com> >> Signed-off-by: Eric Auger <eric.au...@redhat.com> >> >> --- >> v4 -> v5: >> - send_msi can return 0 meaning the MSI was rejected by guest. Handle this >> value as an error >> >> v3 -> v4: >> - fix compilation error >> - msi_supported -> msi_nonbroken >> - streamid -> requester_id >> - use PRIx64 >> - read ops uses _with_attrs form >> - 16/32b write access allowed to GITS_TRANSLATER >> - move its_class_name in kvm_arm.h and remove #ifdef TARGET_AARCH64 >> - add new kvm device dev_fd field >> - add new gits_translater_gpa field in GICv3ITSState >> --- >> hw/intc/Makefile.objs | 1 + >> hw/intc/arm_gicv3_its_common.c | 155 >> +++++++++++++++++++++++++++++++++ >> include/hw/intc/arm_gicv3_its_common.h | 75 ++++++++++++++++ >> target-arm/kvm_arm.h | 19 ++++ >> 4 files changed, 250 insertions(+) >> create mode 100644 hw/intc/arm_gicv3_its_common.c >> create mode 100644 include/hw/intc/arm_gicv3_its_common.h >> >> diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs >> index 05ec21b..23a39f7 100644 >> --- a/hw/intc/Makefile.objs >> +++ b/hw/intc/Makefile.objs >> @@ -16,6 +16,7 @@ common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_common.o >> common-obj-$(CONFIG_ARM_GIC) += arm_gicv3.o >> common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_dist.o >> common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_redist.o >> +common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_its_common.o >> common-obj-$(CONFIG_OPENPIC) += openpic.o >> >> obj-$(CONFIG_APIC) += apic.o apic_common.o >> diff --git a/hw/intc/arm_gicv3_its_common.c b/hw/intc/arm_gicv3_its_common.c >> new file mode 100644 >> index 0000000..0d08ff2 >> --- /dev/null >> +++ b/hw/intc/arm_gicv3_its_common.c >> @@ -0,0 +1,155 @@ >> +/* >> + * ITS base class for a GICv3-based system >> + * >> + * Copyright (c) 2015 Samsung Electronics Co., Ltd. >> + * Written by Pavel Fedin >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation, either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * This program 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 General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License along >> + * with this program; if not, see <http://www.gnu.org/licenses/>. >> + */ >> + >> +#include "qemu/osdep.h" >> +#include "hw/pci/msi.h" >> +#include "hw/intc/arm_gicv3_its_common.h" >> +#include "qemu/log.h" >> + >> +static void gicv3_its_pre_save(void *opaque) >> +{ >> + GICv3ITSState *s = (GICv3ITSState *)opaque; >> + GICv3ITSCommonClass *c = ARM_GICV3_ITS_COMMON_GET_CLASS(s); >> + >> + if (c->pre_save) { >> + c->pre_save(s); >> + } >> +} >> + >> +static int gicv3_its_post_load(void *opaque, int version_id) >> +{ >> + GICv3ITSState *s = (GICv3ITSState *)opaque; >> + GICv3ITSCommonClass *c = ARM_GICV3_ITS_COMMON_GET_CLASS(s); >> + >> + if (c->post_load) { >> + c->post_load(s); >> + } >> + return 0; >> +} >> + >> +static const VMStateDescription vmstate_its = { >> + .name = "arm_gicv3_its", >> + .pre_save = gicv3_its_pre_save, >> + .post_load = gicv3_its_post_load, >> + .fields = (VMStateField[]) { >> + VMSTATE_UINT32(ctlr, GICv3ITSState), >> + VMSTATE_UINT64(cbaser, GICv3ITSState), >> + VMSTATE_UINT64(cwriter, GICv3ITSState), >> + VMSTATE_UINT64(creadr, GICv3ITSState), >> + VMSTATE_UINT64_ARRAY(baser, GICv3ITSState, 8), >> + VMSTATE_END_OF_LIST() >> + }, >> + .unmigratable = true, > > Why define the fields and also mark it unmigratable? > A comment saying why the device is unmigratable would be good. Yes sure I will remove the fields and add a comment on the migration status. > >> +}; >> + >> +static MemTxResult gicv3_its_trans_read(void *opaque, hwaddr offset, >> + uint64_t *data, unsigned size, >> + MemTxAttrs attrs) >> +{ >> + qemu_log_mask(LOG_GUEST_ERROR, "ITS read at offset 0x%"PRIx64"\n", >> offset); >> + return MEMTX_ERROR; >> +} >> + >> +static MemTxResult gicv3_its_trans_write(void *opaque, hwaddr offset, >> + uint64_t value, unsigned size, >> + MemTxAttrs attrs) >> +{ >> + if (offset == 0x0040 && ((size == 2) || (size == 4))) { >> + GICv3ITSState *s = ARM_GICV3_ITS_COMMON(opaque); >> + GICv3ITSCommonClass *c = ARM_GICV3_ITS_COMMON_GET_CLASS(s); >> + int ret = c->send_msi(s, le64_to_cpu(value), attrs.requester_id); >> + >> + if (ret <= 0) { >> + qemu_log_mask(LOG_GUEST_ERROR, >> + "ITS: Error sending MSI: %s\n", strerror(-ret)); >> + return MEMTX_DECODE_ERROR; >> + } >> + >> + return MEMTX_OK; >> + } else { >> + qemu_log_mask(LOG_GUEST_ERROR, >> + "ITS write at bad offset 0x%jX\n", offset); > > Can we be consistent about the format string used for printing hwaddrs? > I suspect this one won't build on 32-bit. I will use PRIx64 instead.
Thanks Eric > >> + return MEMTX_DECODE_ERROR; >> + } >> +} >> + >> +static const MemoryRegionOps gicv3_its_trans_ops = { >> + .read_with_attrs = gicv3_its_trans_read, >> + .write_with_attrs = gicv3_its_trans_write, >> + .endianness = DEVICE_NATIVE_ENDIAN, >> +}; >> + >> +void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops) >> +{ >> + SysBusDevice *sbd = SYS_BUS_DEVICE(s); >> + >> + memory_region_init_io(&s->iomem_its_cntrl, OBJECT(s), ops, s, >> + "control", ITS_CONTROL_SIZE); >> + memory_region_init_io(&s->iomem_its, OBJECT(s), &gicv3_its_trans_ops, s, >> + "translation", ITS_TRANS_SIZE); >> + >> + /* Our two regions are always adjacent, therefore we now combine them >> + * into a single one in order to make our users' life easier. >> + */ >> + memory_region_init(&s->iomem_main, OBJECT(s), "gicv3_its", ITS_SIZE); >> + memory_region_add_subregion(&s->iomem_main, 0, &s->iomem_its_cntrl); >> + memory_region_add_subregion(&s->iomem_main, ITS_CONTROL_SIZE, >> + &s->iomem_its); >> + sysbus_init_mmio(sbd, &s->iomem_main); >> + >> + msi_nonbroken = true; >> +} >> + >> +static void gicv3_its_common_reset(DeviceState *dev) >> +{ >> + GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev); >> + >> + s->ctlr = 0; >> + s->cbaser = 0; >> + s->cwriter = 0; >> + s->creadr = 0; >> + memset(&s->baser, 0, sizeof(s->baser)); >> + >> + gicv3_its_post_load(s, 0); >> +} > > > thanks > -- PMM >