| Message ID | 20210401024152.203896-2-shashi.mallela@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | GICv3 LPI and ITS feature implementation | expand |
On Thu, 1 Apr 2021 at 03:41, Shashi Mallela <shashi.mallela@linaro.org> wrote: > > Added register definitions relevant to ITS,implemented overall > ITS device framework with stubs for ITS control and translater > regions read/write,extended ITS common to handle mmio init between > existing kvm device and newer qemu device. > > Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org> > --- > hw/intc/arm_gicv3_its.c | 334 +++++++++++++++++++++++++ > hw/intc/arm_gicv3_its_common.c | 12 +- > hw/intc/arm_gicv3_its_kvm.c | 2 +- > hw/intc/gicv3_internal.h | 86 ++++++- > hw/intc/meson.build | 1 + > include/hw/intc/arm_gicv3_its_common.h | 12 +- > 6 files changed, 430 insertions(+), 17 deletions(-) > create mode 100644 hw/intc/arm_gicv3_its.c Overall I think the structure of the patchset and of the device is good, so I'm just going to dive into more detailed review comments. > +static MemTxResult its_trans_writew(GICv3ITSState *s, hwaddr offset, > + uint64_t value, MemTxAttrs attrs) > +{ > + MemTxResult result = MEMTX_OK; > + > + return result; > +} > + > +static MemTxResult its_trans_writel(GICv3ITSState *s, hwaddr offset, > + uint64_t value, MemTxAttrs attrs) > +{ > + MemTxResult result = MEMTX_OK; > + > + return result; > +} > + > +static MemTxResult gicv3_its_translation_write(void *opaque, hwaddr offset, > + uint64_t data, unsigned size, MemTxAttrs attrs) > +{ > + GICv3ITSState *s = (GICv3ITSState *)opaque; > + MemTxResult result; > + > + switch (size) { > + case 2: > + result = its_trans_writew(s, offset, data, attrs); > + break; > + case 4: > + result = its_trans_writel(s, offset, data, attrs); > + break; > + default: > + result = MEMTX_ERROR; > + break; > + } > + > + if (result == MEMTX_ERROR) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "%s: invalid guest write at offset " TARGET_FMT_plx > + "size %u\n", __func__, offset, size); > + /* > + * The spec requires that reserved registers are RAZ/WI; > + * so use MEMTX_ERROR returns from leaf functions as a way to > + * trigger the guest-error logging but don't return it to > + * the caller, or we'll cause a spurious guest data abort. > + */ > + result = MEMTX_OK; > + } > + return result; > +} There is exactly one register in the translation register frame, so having this generic top level write function that calls out to separate writew and writel functions is overkill and ends up with duplication of code. I would suggest you just fold the implementation into gicv3_its_translation_write(), which would end up looking something like switch (offset) { case GITS_TRANSLATER: if (s->ctlr & ITS_CTLR_ENABLED) { /* 16 bit accesses behave as writes with bits [31:16] zero */ s->translater = value; devid = attrs.requester_id; process_int(s, s->translater, devid, NONE); } break; default: break; } return MEMTX_OK; which handles both the 16 and 32 bit case. You can tell the core memory system code not to hand you 8 or 64 bit accesses using fields in the MemoryRegionOps; see below. > +static MemTxResult gicv3_its_translation_read(void *opaque, hwaddr offset, > + uint64_t *data, unsigned size, MemTxAttrs attrs) > +{ > + qemu_log_mask(LOG_GUEST_ERROR, > + "%s: Invalid read from translation register area at offset " > + TARGET_FMT_plx "\n", __func__, offset); > + return MEMTX_ERROR; > +} > + > +static MemTxResult its_writeb(GICv3ITSState *s, hwaddr offset, > + uint64_t value, MemTxAttrs attrs) > +{ > + qemu_log_mask(LOG_UNIMP, > + "%s: unsupported byte write to register at offset " > + TARGET_FMT_plx "\n", __func__, offset); > + return MEMTX_ERROR; > +} > + > +static MemTxResult its_readb(GICv3ITSState *s, hwaddr offset, > + uint64_t *data, MemTxAttrs attrs) > +{ > + qemu_log_mask(LOG_UNIMP, > + "%s: unsupported byte read from register at offset " > + TARGET_FMT_plx "\n", __func__, offset); > + return MEMTX_ERROR; > +} > + > +static MemTxResult its_writew(GICv3ITSState *s, hwaddr offset, > + uint64_t value, MemTxAttrs attrs) > +{ > + qemu_log_mask(LOG_UNIMP, > + "%s: unsupported word write to register at offset " > + TARGET_FMT_plx "\n", __func__, offset); > + return MEMTX_ERROR; > +} > + > +static MemTxResult its_readw(GICv3ITSState *s, hwaddr offset, > + uint64_t *data, MemTxAttrs attrs) > +{ > + qemu_log_mask(LOG_UNIMP, > + "%s: unsupported word read from register at offset " > + TARGET_FMT_plx "\n", __func__, offset); > + return MEMTX_ERROR; > +} Similarly, there are no byte or halfword accessible registers on the ITS, so these are unnecessary; just use MemoryRegionOps to cause the core code to reject them. > +static MemTxResult its_writel(GICv3ITSState *s, hwaddr offset, > + uint64_t value, MemTxAttrs attrs) > +{ > + MemTxResult result = MEMTX_OK; > + > + return result; > +} > + > +static MemTxResult its_readl(GICv3ITSState *s, hwaddr offset, > + uint64_t *data, MemTxAttrs attrs) > +{ > + MemTxResult result = MEMTX_OK; > + > + return result; > +} > + > +static MemTxResult its_writell(GICv3ITSState *s, hwaddr offset, > + uint64_t value, MemTxAttrs attrs) > +{ > + MemTxResult result = MEMTX_OK; > + > + return result; > +} > + > +static MemTxResult its_readll(GICv3ITSState *s, hwaddr offset, > + uint64_t *data, MemTxAttrs attrs) > +{ > + MemTxResult result = MEMTX_OK; > + > + return result; > +} > + > +static MemTxResult gicv3_its_read(void *opaque, hwaddr offset, uint64_t *data, > + unsigned size, MemTxAttrs attrs) > +{ > + GICv3ITSState *s = (GICv3ITSState *)opaque; > + MemTxResult result; > + > + switch (size) { > + case 1: > + result = its_readb(s, offset, data, attrs); > + break; > + case 2: > + result = its_readw(s, offset, data, attrs); > + break; > + case 4: > + result = its_readl(s, offset, data, attrs); > + break; > + case 8: > + result = its_readll(s, offset, data, attrs); > + break; > + default: > + result = MEMTX_ERROR; > + break; > + } > + > + if (result == MEMTX_ERROR) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "%s: invalid guest read at offset " TARGET_FMT_plx > + "size %u\n", __func__, offset, size); > + /* > + * The spec requires that reserved registers are RAZ/WI; > + * so use MEMTX_ERROR returns from leaf functions as a way to > + * trigger the guest-error logging but don't return it to > + * the caller, or we'll cause a spurious guest data abort. > + */ > + result = MEMTX_OK; > + *data = 0; > + } > + return result; > +} > + > +static MemTxResult gicv3_its_write(void *opaque, hwaddr offset, uint64_t data, > + unsigned size, MemTxAttrs attrs) > +{ > + GICv3ITSState *s = (GICv3ITSState *)opaque; > + MemTxResult result; > + > + switch (size) { > + case 1: > + result = its_writeb(s, offset, data, attrs); > + break; > + case 2: > + result = its_writew(s, offset, data, attrs); > + break; > + case 4: > + result = its_writel(s, offset, data, attrs); > + break; > + case 8: > + result = its_writell(s, offset, data, attrs); > + break; > + default: > + result = MEMTX_ERROR; > + break; > + } > + > + if (result == MEMTX_ERROR) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "%s: invalid guest write at offset " TARGET_FMT_plx > + "size %u\n", __func__, offset, size); > + /* > + * The spec requires that reserved registers are RAZ/WI; > + * so use MEMTX_ERROR returns from leaf functions as a way to > + * trigger the guest-error logging but don't return it to > + * the caller, or we'll cause a spurious guest data abort. > + */ > + result = MEMTX_OK; > + } > + return result; > +} > + > +static const MemoryRegionOps gicv3_its_control_ops = { > + .read_with_attrs = gicv3_its_read, > + .write_with_attrs = gicv3_its_write, > + .endianness = DEVICE_NATIVE_ENDIAN, .valid.min_access_size = 4, .valid.max_access_size = 8, .impl.min_access_size = 4, .impl.max_access_size = 8, And then you don't need to handle 1 and 2 in your read/write fns. > +}; > + > +static const MemoryRegionOps gicv3_its_translation_ops = { > + .read_with_attrs = gicv3_its_translation_read, > + .write_with_attrs = gicv3_its_translation_write, > + .endianness = DEVICE_NATIVE_ENDIAN, Similarly here set the valid and impl min and max to 2 and 4. > +}; > + > +static void gicv3_arm_its_realize(DeviceState *dev, Error **errp) > +{ > + GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev); > + > + gicv3_its_init_mmio(s, &gicv3_its_control_ops, &gicv3_its_translation_ops); > +} > + > +static void gicv3_its_reset(DeviceState *dev) > +{ > + GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev); > + GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s); > + > + if (s->gicv3->cpu->gicr_typer & GICR_TYPER_PLPIS) { > + c->parent_reset(dev); > + > + /* set the ITS default features supported */ > + s->typer = FIELD_DP64(s->typer, GITS_TYPER, PHYSICAL, > + GITS_TYPE_PHYSICAL); > + s->typer = FIELD_DP64(s->typer, GITS_TYPER, ITT_ENTRY_SIZE, > + ITS_ITT_ENTRY_SIZE); > + s->typer = FIELD_DP64(s->typer, GITS_TYPER, IDBITS, ITS_IDBITS); > + s->typer = FIELD_DP64(s->typer, GITS_TYPER, DEVBITS, ITS_DEVBITS); > + s->typer = FIELD_DP64(s->typer, GITS_TYPER, CIL, 1); > + s->typer = FIELD_DP64(s->typer, GITS_TYPER, CIDBITS, ITS_CIDBITS); This is a read-only constant value. Two options: (1) set s->typer in realize, not in reset (this is how we handle GICR_TYPER) (2) don't have an s->typer at all, and just return the right constant value when in the read function (this is how we handle GICD_TYPER) > + > + /* > + * We claim to be an ARM r0p0 with a zero ProductID. > + * This is the same as an r0p0 GIC-500. > + */ > + s->iidr = gicv3_iidr(); IIDR is constant, so just call gicv3_iidr() to get its value directly in the register read function. > + > + /* Quiescent bit reset to 1 */ > + s->ctlr = FIELD_DP32(s->ctlr, GITS_CTLR, QUIESCENT, 1); > + > + /* > + * setting GITS_BASER0.Type = 0b001 (Device) > + * GITS_BASER1.Type = 0b100 (Collection Table) > + * GITS_BASER<n>.Type,where n = 3 to 7 are 0b00 (Unimplemented) > + * GITS_BASER<0,1>.Page_Size = 64KB > + * and default translation table entry size to 16 bytes > + */ > + s->baser[0] = FIELD_DP64(s->baser[0], GITS_BASER, TYPE, > + GITS_ITT_TYPE_DEVICE); > + s->baser[0] = FIELD_DP64(s->baser[0], GITS_BASER, PAGESIZE, > + GITS_BASER_PAGESIZE_64K); > + s->baser[0] = FIELD_DP64(s->baser[0], GITS_BASER, ENTRYSIZE, > + GITS_DTE_SIZE); > + > + s->baser[1] = FIELD_DP64(s->baser[1], GITS_BASER, TYPE, > + GITS_ITT_TYPE_COLLECTION); > + s->baser[1] = FIELD_DP64(s->baser[1], GITS_BASER, PAGESIZE, > + GITS_BASER_PAGESIZE_64K); > + s->baser[1] = FIELD_DP64(s->baser[1], GITS_BASER, ENTRYSIZE, > + GITS_CTE_SIZE); > + } > +} > + > +static Property gicv3_its_props[] = { > + DEFINE_PROP_LINK("parent-gicv3", GICv3ITSState, gicv3, "arm-gicv3", > + GICv3State *), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > +static void gicv3_its_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + GICv3ITSClass *ic = ARM_GICV3_ITS_CLASS(klass); > + > + dc->realize = gicv3_arm_its_realize; > + device_class_set_props(dc, gicv3_its_props); > + device_class_set_parent_reset(dc, gicv3_its_reset, &ic->parent_reset); > +} > + > +static const TypeInfo gicv3_its_info = { > + .name = TYPE_ARM_GICV3_ITS, > + .parent = TYPE_ARM_GICV3_ITS_COMMON, > + .instance_size = sizeof(GICv3ITSState), > + .class_init = gicv3_its_class_init, > + .class_size = sizeof(GICv3ITSClass), > +}; > + > +static void gicv3_its_register_types(void) > +{ > + type_register_static(&gicv3_its_info); > +} > + > +type_init(gicv3_its_register_types) > diff --git a/hw/intc/arm_gicv3_its_common.c b/hw/intc/arm_gicv3_its_common.c > index 66c4c6a188..80cc9ec6d8 100644 > --- a/hw/intc/arm_gicv3_its_common.c > +++ b/hw/intc/arm_gicv3_its_common.c > @@ -50,12 +50,16 @@ static int gicv3_its_post_load(void *opaque, int version_id) > > static const VMStateDescription vmstate_its = { > .name = "arm_gicv3_its", > + .version_id = 1, > + .minimum_version_id = 1, > .pre_save = gicv3_its_pre_save, > .post_load = gicv3_its_post_load, > .priority = MIG_PRI_GICV3_ITS, > .fields = (VMStateField[]) { > VMSTATE_UINT32(ctlr, GICv3ITSState), > + VMSTATE_UINT32(translater, GICv3ITSState), > VMSTATE_UINT32(iidr, GICv3ITSState), > + VMSTATE_UINT64(typer, GICv3ITSState), You cannot change a VMStateDescription like this -- you will break migration compatibility. Luckily, in this case there's no need: GITS_TYPER is read-only, so it doesn't need to be migrated, and GITS_TRANSLATER is not a real contains-state register (it's write-only), so there's no new state there either. Neither needs to be a new field in the GICv3ITSState structure either (though typer could be if you prefer). > VMSTATE_UINT64(cbaser, GICv3ITSState), > VMSTATE_UINT64(cwriter, GICv3ITSState), > VMSTATE_UINT64(creadr, GICv3ITSState), > @@ -99,15 +103,16 @@ static const MemoryRegionOps gicv3_its_trans_ops = { > .endianness = DEVICE_NATIVE_ENDIAN, > }; > +/** > + * Default features advertised by this version of ITS > + */ > +/* Physical LPIs supported */ > +#define GITS_TYPE_PHYSICAL (1U << 0) > + > +/* > + * 11 bytes Interrupt translation Table Entry size > + * Valid = 1 bit,InterruptType = 1 bit, > + * Size of LPI number space[considering max 24 bits], > + * Size of LPI number space[considering max 24 bits], > + * ICID = 16 bits, > + * vPEID = 16 bits > + */ > +#define ITS_ITT_ENTRY_SIZE 0xB > + > +/* 16 bits EventId */ > +#define ITS_IDBITS GICD_TYPER_IDBITS > + > +/* 16 bits DeviceId */ > +#define ITS_DEVBITS 0xF > + > +/* 16 bits CollectionId */ > +#define ITS_CIDBITS 0xF > + > +/* > + * 8 bytes Device Table Entry size > + * Valid = 1 bit,ITTAddr = 44 bits,Size = 5 bits > + */ > +#define GITS_DTE_SIZE (0x8ULL) > + > +/* > + * 8 bytes Collection Table Entry size > + * Valid = 1 bit,RDBase = 36 bits(considering max RDBASE) > + */ > +#define GITS_CTE_SIZE (0x8ULL) > + Is this the same data structure the in-kernel KVM ITS uses, or is it just one specific to the QEMU emulated implementation ? > /* Special interrupt IDs */ > #define INTID_SECURE 1020 > #define INTID_NONSECURE 1021 > diff --git a/hw/intc/meson.build b/hw/intc/meson.build > index 1c299039f6..53472239f0 100644 > --- a/hw/intc/meson.build > +++ b/hw/intc/meson.build > @@ -8,6 +8,7 @@ softmmu_ss.add(when: 'CONFIG_ARM_GIC', if_true: files( > 'arm_gicv3_dist.c', > 'arm_gicv3_its_common.c', > 'arm_gicv3_redist.c', > + 'arm_gicv3_its.c', > )) > softmmu_ss.add(when: 'CONFIG_ETRAXFS', if_true: files('etraxfs_pic.c')) > softmmu_ss.add(when: 'CONFIG_HEATHROW_PIC', if_true: files('heathrow_pic.c')) > diff --git a/include/hw/intc/arm_gicv3_its_common.h b/include/hw/intc/arm_gicv3_its_common.h > index 5a0952b404..08bc5652ed 100644 > --- a/include/hw/intc/arm_gicv3_its_common.h > +++ b/include/hw/intc/arm_gicv3_its_common.h > @@ -25,17 +25,24 @@ > #include "hw/intc/arm_gicv3_common.h" > #include "qom/object.h" > > +#define TYPE_ARM_GICV3_ITS "arm-gicv3-its" > + > #define ITS_CONTROL_SIZE 0x10000 > #define ITS_TRANS_SIZE 0x10000 > #define ITS_SIZE (ITS_CONTROL_SIZE + ITS_TRANS_SIZE) > > #define GITS_CTLR 0x0 > #define GITS_IIDR 0x4 > +#define GITS_TYPER 0x8 > #define GITS_CBASER 0x80 > #define GITS_CWRITER 0x88 > #define GITS_CREADR 0x90 > #define GITS_BASER 0x100 > > +#define GITS_TRANSLATER 0x0040 > + > +#define GITS_PIDR2 0xFFE8 You probably don't want an offset for GITS_PIDR2 specifically. Compare handling of GICD_IDREGS in the distributor emulation. thanks -- PMM
On Fri, 2021-04-16 at 18:21 +0100, Peter Maydell wrote: > On Thu, 1 Apr 2021 at 03:41, Shashi Mallela < > shashi.mallela@linaro.org> wrote: > > Added register definitions relevant to ITS,implemented overall > > ITS device framework with stubs for ITS control and translater > > regions read/write,extended ITS common to handle mmio init between > > existing kvm device and newer qemu device. > > > > Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org> > > --- > > hw/intc/arm_gicv3_its.c | 334 > > +++++++++++++++++++++++++ > > hw/intc/arm_gicv3_its_common.c | 12 +- > > hw/intc/arm_gicv3_its_kvm.c | 2 +- > > hw/intc/gicv3_internal.h | 86 ++++++- > > hw/intc/meson.build | 1 + > > include/hw/intc/arm_gicv3_its_common.h | 12 +- > > 6 files changed, 430 insertions(+), 17 deletions(-) > > create mode 100644 hw/intc/arm_gicv3_its.c > > Overall I think the structure of the patchset and of the device > is good, so I'm just going to dive into more detailed review > comments. > > Have accepted and addressed all the comments and will share the > updated patch sets soon,responding to comments which need > clarification in all the relevant patch sets > > > +static MemTxResult its_trans_writew(GICv3ITSState *s, hwaddr > > offset, > > + uint64_t value, MemTxAttrs attrs) > > +{ > > + MemTxResult result = MEMTX_OK; > > + > > + return result; > > +} > > + > > +static MemTxResult its_trans_writel(GICv3ITSState *s, hwaddr > > offset, > > + uint64_t value, MemTxAttrs attrs) > > +{ > > + MemTxResult result = MEMTX_OK; > > + > > + return result; > > +} > > + > > +static MemTxResult gicv3_its_translation_write(void *opaque, > > hwaddr offset, > > + uint64_t data, unsigned size, > > MemTxAttrs attrs) > > +{ > > + GICv3ITSState *s = (GICv3ITSState *)opaque; > > + MemTxResult result; > > + > > + switch (size) { > > + case 2: > > + result = its_trans_writew(s, offset, data, attrs); > > + break; > > + case 4: > > + result = its_trans_writel(s, offset, data, attrs); > > + break; > > + default: > > + result = MEMTX_ERROR; > > + break; > > + } > > + > > + if (result == MEMTX_ERROR) { > > + qemu_log_mask(LOG_GUEST_ERROR, > > + "%s: invalid guest write at offset " > > TARGET_FMT_plx > > + "size %u\n", __func__, offset, size); > > + /* > > + * The spec requires that reserved registers are RAZ/WI; > > + * so use MEMTX_ERROR returns from leaf functions as a way > > to > > + * trigger the guest-error logging but don't return it to > > + * the caller, or we'll cause a spurious guest data abort. > > + */ > > + result = MEMTX_OK; > > + } > > + return result; > > +} > > There is exactly one register in the translation register frame, so > having this generic top level write function that calls out to > separate writew and writel functions is overkill and ends up > with duplication of code. I would suggest you just fold the > implementation into gicv3_its_translation_write(), which would > end up looking something like > > switch (offset) { > case GITS_TRANSLATER: > if (s->ctlr & ITS_CTLR_ENABLED) { > /* 16 bit accesses behave as writes with bits [31:16] > zero */ > s->translater = value; > devid = attrs.requester_id; > process_int(s, s->translater, devid, NONE); > } > break; > default: > break; > } > return MEMTX_OK; > > which handles both the 16 and 32 bit case. You can tell the > core memory system code not to hand you 8 or 64 bit accesses > using fields in the MemoryRegionOps; see below. > > > +static MemTxResult gicv3_its_translation_read(void *opaque, hwaddr > > offset, > > + uint64_t *data, unsigned size, > > MemTxAttrs attrs) > > +{ > > + qemu_log_mask(LOG_GUEST_ERROR, > > + "%s: Invalid read from translation register area at offset > > " > > + TARGET_FMT_plx "\n", __func__, offset); > > + return MEMTX_ERROR; > > +} > > + > > +static MemTxResult its_writeb(GICv3ITSState *s, hwaddr offset, > > + uint64_t value, MemTxAttrs attrs) > > +{ > > + qemu_log_mask(LOG_UNIMP, > > + "%s: unsupported byte write to register at offset > > " > > + TARGET_FMT_plx "\n", __func__, offset); > > + return MEMTX_ERROR; > > +} > > + > > +static MemTxResult its_readb(GICv3ITSState *s, hwaddr offset, > > + uint64_t *data, MemTxAttrs attrs) > > +{ > > + qemu_log_mask(LOG_UNIMP, > > + "%s: unsupported byte read from register at offset > > " > > + TARGET_FMT_plx "\n", __func__, offset); > > + return MEMTX_ERROR; > > +} > > + > > +static MemTxResult its_writew(GICv3ITSState *s, hwaddr offset, > > + uint64_t value, MemTxAttrs attrs) > > +{ > > + qemu_log_mask(LOG_UNIMP, > > + "%s: unsupported word write to register at offset " > > + TARGET_FMT_plx "\n", __func__, offset); > > + return MEMTX_ERROR; > > +} > > + > > +static MemTxResult its_readw(GICv3ITSState *s, hwaddr offset, > > + uint64_t *data, MemTxAttrs attrs) > > +{ > > + qemu_log_mask(LOG_UNIMP, > > + "%s: unsupported word read from register at offset " > > + TARGET_FMT_plx "\n", __func__, offset); > > + return MEMTX_ERROR; > > +} > > Similarly, there are no byte or halfword accessible registers > on the ITS, so these are unnecessary; just use MemoryRegionOps > to cause the core code to reject them. > > > +static MemTxResult its_writel(GICv3ITSState *s, hwaddr offset, > > + uint64_t value, MemTxAttrs attrs) > > +{ > > + MemTxResult result = MEMTX_OK; > > + > > + return result; > > +} > > + > > +static MemTxResult its_readl(GICv3ITSState *s, hwaddr offset, > > + uint64_t *data, MemTxAttrs attrs) > > +{ > > + MemTxResult result = MEMTX_OK; > > + > > + return result; > > +} > > + > > +static MemTxResult its_writell(GICv3ITSState *s, hwaddr offset, > > + uint64_t value, MemTxAttrs attrs) > > +{ > > + MemTxResult result = MEMTX_OK; > > + > > + return result; > > +} > > + > > +static MemTxResult its_readll(GICv3ITSState *s, hwaddr offset, > > + uint64_t *data, MemTxAttrs attrs) > > +{ > > + MemTxResult result = MEMTX_OK; > > + > > + return result; > > +} > > + > > +static MemTxResult gicv3_its_read(void *opaque, hwaddr offset, > > uint64_t *data, > > + unsigned size, MemTxAttrs attrs) > > +{ > > + GICv3ITSState *s = (GICv3ITSState *)opaque; > > + MemTxResult result; > > + > > + switch (size) { > > + case 1: > > + result = its_readb(s, offset, data, attrs); > > + break; > > + case 2: > > + result = its_readw(s, offset, data, attrs); > > + break; > > + case 4: > > + result = its_readl(s, offset, data, attrs); > > + break; > > + case 8: > > + result = its_readll(s, offset, data, attrs); > > + break; > > + default: > > + result = MEMTX_ERROR; > > + break; > > + } > > + > > + if (result == MEMTX_ERROR) { > > + qemu_log_mask(LOG_GUEST_ERROR, > > + "%s: invalid guest read at offset " > > TARGET_FMT_plx > > + "size %u\n", __func__, offset, size); > > + /* > > + * The spec requires that reserved registers are RAZ/WI; > > + * so use MEMTX_ERROR returns from leaf functions as a way > > to > > + * trigger the guest-error logging but don't return it to > > + * the caller, or we'll cause a spurious guest data abort. > > + */ > > + result = MEMTX_OK; > > + *data = 0; > > + } > > + return result; > > +} > > + > > +static MemTxResult gicv3_its_write(void *opaque, hwaddr offset, > > uint64_t data, > > + unsigned size, MemTxAttrs attrs) > > +{ > > + GICv3ITSState *s = (GICv3ITSState *)opaque; > > + MemTxResult result; > > + > > + switch (size) { > > + case 1: > > + result = its_writeb(s, offset, data, attrs); > > + break; > > + case 2: > > + result = its_writew(s, offset, data, attrs); > > + break; > > + case 4: > > + result = its_writel(s, offset, data, attrs); > > + break; > > + case 8: > > + result = its_writell(s, offset, data, attrs); > > + break; > > + default: > > + result = MEMTX_ERROR; > > + break; > > + } > > + > > + if (result == MEMTX_ERROR) { > > + qemu_log_mask(LOG_GUEST_ERROR, > > + "%s: invalid guest write at offset " > > TARGET_FMT_plx > > + "size %u\n", __func__, offset, size); > > + /* > > + * The spec requires that reserved registers are RAZ/WI; > > + * so use MEMTX_ERROR returns from leaf functions as a way > > to > > + * trigger the guest-error logging but don't return it to > > + * the caller, or we'll cause a spurious guest data abort. > > + */ > > + result = MEMTX_OK; > > + } > > + return result; > > +} > > + > > +static const MemoryRegionOps gicv3_its_control_ops = { > > + .read_with_attrs = gicv3_its_read, > > + .write_with_attrs = gicv3_its_write, > > + .endianness = DEVICE_NATIVE_ENDIAN, > > .valid.min_access_size = 4, > .valid.max_access_size = 8, > .impl.min_access_size = 4, > .impl.max_access_size = 8, > > And then you don't need to handle 1 and 2 in your read/write fns. > > > > +}; > > + > > +static const MemoryRegionOps gicv3_its_translation_ops = { > > + .read_with_attrs = gicv3_its_translation_read, > > + .write_with_attrs = gicv3_its_translation_write, > > + .endianness = DEVICE_NATIVE_ENDIAN, > > Similarly here set the valid and impl min and max to 2 and 4. > > > +}; > > + > > +static void gicv3_arm_its_realize(DeviceState *dev, Error **errp) > > +{ > > + GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev); > > + > > + gicv3_its_init_mmio(s, &gicv3_its_control_ops, > > &gicv3_its_translation_ops); > > +} > > + > > +static void gicv3_its_reset(DeviceState *dev) > > +{ > > + GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev); > > + GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s); > > + > > + if (s->gicv3->cpu->gicr_typer & GICR_TYPER_PLPIS) { > > + c->parent_reset(dev); > > + > > + /* set the ITS default features supported */ > > + s->typer = FIELD_DP64(s->typer, GITS_TYPER, PHYSICAL, > > + GITS_TYPE_PHYSICAL); > > + s->typer = FIELD_DP64(s->typer, GITS_TYPER, > > ITT_ENTRY_SIZE, > > + ITS_ITT_ENTRY_SIZE); > > + s->typer = FIELD_DP64(s->typer, GITS_TYPER, IDBITS, > > ITS_IDBITS); > > + s->typer = FIELD_DP64(s->typer, GITS_TYPER, DEVBITS, > > ITS_DEVBITS); > > + s->typer = FIELD_DP64(s->typer, GITS_TYPER, CIL, 1); > > + s->typer = FIELD_DP64(s->typer, GITS_TYPER, CIDBITS, > > ITS_CIDBITS); > > This is a read-only constant value. Two options: > (1) set s->typer in realize, not in reset (this is how we handle > GICR_TYPER) > (2) don't have an s->typer at all, and just return the right > constant value when in the read function (this is how we > handle GICD_TYPER) > Implemented option 1 > > > + > > + /* > > + * We claim to be an ARM r0p0 with a zero ProductID. > > + * This is the same as an r0p0 GIC-500. > > + */ > > + s->iidr = gicv3_iidr(); > > IIDR is constant, so just call gicv3_iidr() to get its value directly > in the register read function. > > > + > > + /* Quiescent bit reset to 1 */ > > + s->ctlr = FIELD_DP32(s->ctlr, GITS_CTLR, QUIESCENT, 1); > > + > > + /* > > + * setting GITS_BASER0.Type = 0b001 (Device) > > + * GITS_BASER1.Type = 0b100 (Collection Table) > > + * GITS_BASER<n>.Type,where n = 3 to 7 are 0b00 > > (Unimplemented) > > + * GITS_BASER<0,1>.Page_Size = 64KB > > + * and default translation table entry size to 16 bytes > > + */ > > + s->baser[0] = FIELD_DP64(s->baser[0], GITS_BASER, TYPE, > > + GITS_ITT_TYPE_DEVICE > > ); > > + s->baser[0] = FIELD_DP64(s->baser[0], GITS_BASER, > > PAGESIZE, > > + GITS_BASER_PAGESIZE_ > > 64K); > > + s->baser[0] = FIELD_DP64(s->baser[0], GITS_BASER, > > ENTRYSIZE, > > + GITS_DTE_SIZE); > > + > > + s->baser[1] = FIELD_DP64(s->baser[1], GITS_BASER, TYPE, > > + GITS_ITT_TYPE_COLLEC > > TION); > > + s->baser[1] = FIELD_DP64(s->baser[1], GITS_BASER, > > PAGESIZE, > > + GITS_BASER_PAGESIZE_ > > 64K); > > + s->baser[1] = FIELD_DP64(s->baser[1], GITS_BASER, > > ENTRYSIZE, > > + GITS_CTE_SIZE); > > + } > > +} > > + > > +static Property gicv3_its_props[] = { > > + DEFINE_PROP_LINK("parent-gicv3", GICv3ITSState, gicv3, "arm- > > gicv3", > > + GICv3State *), > > + DEFINE_PROP_END_OF_LIST(), > > +}; > > + > > +static void gicv3_its_class_init(ObjectClass *klass, void *data) > > +{ > > + DeviceClass *dc = DEVICE_CLASS(klass); > > + GICv3ITSClass *ic = ARM_GICV3_ITS_CLASS(klass); > > + > > + dc->realize = gicv3_arm_its_realize; > > + device_class_set_props(dc, gicv3_its_props); > > + device_class_set_parent_reset(dc, gicv3_its_reset, &ic- > > >parent_reset); > > +} > > + > > +static const TypeInfo gicv3_its_info = { > > + .name = TYPE_ARM_GICV3_ITS, > > + .parent = TYPE_ARM_GICV3_ITS_COMMON, > > + .instance_size = sizeof(GICv3ITSState), > > + .class_init = gicv3_its_class_init, > > + .class_size = sizeof(GICv3ITSClass), > > +}; > > + > > +static void gicv3_its_register_types(void) > > +{ > > + type_register_static(&gicv3_its_info); > > +} > > + > > +type_init(gicv3_its_register_types) > > diff --git a/hw/intc/arm_gicv3_its_common.c > > b/hw/intc/arm_gicv3_its_common.c > > index 66c4c6a188..80cc9ec6d8 100644 > > --- a/hw/intc/arm_gicv3_its_common.c > > +++ b/hw/intc/arm_gicv3_its_common.c > > @@ -50,12 +50,16 @@ static int gicv3_its_post_load(void *opaque, > > int version_id) > > > > static const VMStateDescription vmstate_its = { > > .name = "arm_gicv3_its", > > + .version_id = 1, > > + .minimum_version_id = 1, > > .pre_save = gicv3_its_pre_save, > > .post_load = gicv3_its_post_load, > > .priority = MIG_PRI_GICV3_ITS, > > .fields = (VMStateField[]) { > > VMSTATE_UINT32(ctlr, GICv3ITSState), > > + VMSTATE_UINT32(translater, GICv3ITSState), > > VMSTATE_UINT32(iidr, GICv3ITSState), > > + VMSTATE_UINT64(typer, GICv3ITSState), > > You cannot change a VMStateDescription like this -- you will break > migration compatibility. Luckily, in this case there's no need: > GITS_TYPER is read-only, so it doesn't need to be migrated, and > GITS_TRANSLATER is not a real contains-state register (it's write- > only), > so there's no new state there either. Neither needs to be a new field > in the GICv3ITSState structure either (though typer could be if you > prefer). > > > VMSTATE_UINT64(cbaser, GICv3ITSState), > > VMSTATE_UINT64(cwriter, GICv3ITSState), > > VMSTATE_UINT64(creadr, GICv3ITSState), > > @@ -99,15 +103,16 @@ static const MemoryRegionOps > > gicv3_its_trans_ops = { > > .endianness = DEVICE_NATIVE_ENDIAN, > > }; > > +/** > > + * Default features advertised by this version of ITS > > + */ > > +/* Physical LPIs supported */ > > +#define GITS_TYPE_PHYSICAL (1U << 0) > > + > > +/* > > + * 11 bytes Interrupt translation Table Entry size > > + * Valid = 1 bit,InterruptType = 1 bit, > > + * Size of LPI number space[considering max 24 bits], > > + * Size of LPI number space[considering max 24 bits], > > + * ICID = 16 bits, > > + * vPEID = 16 bits > > + */ > > +#define ITS_ITT_ENTRY_SIZE 0xB > > + > > +/* 16 bits EventId */ > > +#define ITS_IDBITS GICD_TYPER_IDBITS > > + > > +/* 16 bits DeviceId */ > > +#define ITS_DEVBITS 0xF > > + > > +/* 16 bits CollectionId */ > > +#define ITS_CIDBITS 0xF > > + > > +/* > > + * 8 bytes Device Table Entry size > > + * Valid = 1 bit,ITTAddr = 44 bits,Size = 5 bits > > + */ > > +#define GITS_DTE_SIZE (0x8ULL) > > + > > +/* > > + * 8 bytes Collection Table Entry size > > + * Valid = 1 bit,RDBase = 36 bits(considering max RDBASE) > > + */ > > +#define GITS_CTE_SIZE (0x8ULL) > > + > > Is this the same data structure the in-kernel KVM ITS uses, or is > it just one specific to the QEMU emulated implementation ? > This is specific to QEMU emulated implementation > > > /* Special interrupt IDs */ > > #define INTID_SECURE 1020 > > #define INTID_NONSECURE 1021 > > diff --git a/hw/intc/meson.build b/hw/intc/meson.build > > index 1c299039f6..53472239f0 100644 > > --- a/hw/intc/meson.build > > +++ b/hw/intc/meson.build > > @@ -8,6 +8,7 @@ softmmu_ss.add(when: 'CONFIG_ARM_GIC', if_true: > > files( > > 'arm_gicv3_dist.c', > > 'arm_gicv3_its_common.c', > > 'arm_gicv3_redist.c', > > + 'arm_gicv3_its.c', > > )) > > softmmu_ss.add(when: 'CONFIG_ETRAXFS', if_true: > > files('etraxfs_pic.c')) > > softmmu_ss.add(when: 'CONFIG_HEATHROW_PIC', if_true: > > files('heathrow_pic.c')) > > diff --git a/include/hw/intc/arm_gicv3_its_common.h > > b/include/hw/intc/arm_gicv3_its_common.h > > index 5a0952b404..08bc5652ed 100644 > > --- a/include/hw/intc/arm_gicv3_its_common.h > > +++ b/include/hw/intc/arm_gicv3_its_common.h > > @@ -25,17 +25,24 @@ > > #include "hw/intc/arm_gicv3_common.h" > > #include "qom/object.h" > > > > +#define TYPE_ARM_GICV3_ITS "arm-gicv3-its" > > + > > #define ITS_CONTROL_SIZE 0x10000 > > #define ITS_TRANS_SIZE 0x10000 > > #define ITS_SIZE (ITS_CONTROL_SIZE + ITS_TRANS_SIZE) > > > > #define GITS_CTLR 0x0 > > #define GITS_IIDR 0x4 > > +#define GITS_TYPER 0x8 > > #define GITS_CBASER 0x80 > > #define GITS_CWRITER 0x88 > > #define GITS_CREADR 0x90 > > #define GITS_BASER 0x100 > > > > +#define GITS_TRANSLATER 0x0040 > > + > > +#define GITS_PIDR2 0xFFE8 > > You probably don't want an offset for GITS_PIDR2 specifically. > Compare handling of GICD_IDREGS in the distributor emulation. > Tried re-using the GICD_IDREGS offset for GITS_PIDR2,but from testing > observed that the linux kernel driver and kvm-unit-tests both relied > on using the 0xFFE8 offset instead of 0xFFD0 (used by GICD & GICR) to > complete ITS initialization and setup > > thanks > -- PMM
On Fri, 30 Apr 2021 at 00:36, <shashi.mallela@linaro.org> wrote: > > On Fri, 2021-04-16 at 18:21 +0100, Peter Maydell wrote: > > On Thu, 1 Apr 2021 at 03:41, Shashi Mallela < > > shashi.mallela@linaro.org> wrote: > > > +#define GITS_PIDR2 0xFFE8 > > > > You probably don't want an offset for GITS_PIDR2 specifically. > > Compare handling of GICD_IDREGS in the distributor emulation. > > Tried re-using the GICD_IDREGS offset for GITS_PIDR2,but from testing > > observed that the linux kernel driver and kvm-unit-tests both relied > > on using the 0xFFE8 offset instead of 0xFFD0 (used by GICD & GICR) to > > complete ITS initialization and setup I didn't mean "just put PIDR2 at the GICD_IDREGS offset", which obviously won't work, I meant "implement the whole set of ID registers (of which PIDR2 is one in the middle) in the same way we implement that set on the other GIC components". -- PMM
Have taken care of it.please ignore PIDR2 changes in the current patchset ,will update the latest changes in the next patchset version Thanks Shashi On Apr 30 2021, at 6:09 am, Peter Maydell <peter.maydell@linaro.org> wrote: > On Fri, 30 Apr 2021 at 00:36, <shashi.mallela@linaro.org> wrote: > > > > On Fri, 2021-04-16 at 18:21 +0100, Peter Maydell wrote: > > > On Thu, 1 Apr 2021 at 03:41, Shashi Mallela < > > > shashi.mallela@linaro.org> wrote: > > > > +#define GITS_PIDR2 0xFFE8 > > > > > > You probably don't want an offset for GITS_PIDR2 specifically. > > > Compare handling of GICD_IDREGS in the distributor emulation. > > > Tried re-using the GICD_IDREGS offset for GITS_PIDR2,but from testing > > > observed that the linux kernel driver and kvm-unit-tests both relied > > > on using the 0xFFE8 offset instead of 0xFFD0 (used by GICD & GICR) to > > > complete ITS initialization and setup > > I didn't mean "just put PIDR2 at the GICD_IDREGS offset", which > obviously won't work, I meant "implement the whole set of ID registers > (of which PIDR2 is one in the middle) in the same way we implement that > set on the other GIC components". > > -- PMM <div>Have taken care of it.please ignore PIDR2 changes in the current patchset ,will update the latest changes in the next patchset version</div><br><div>Thanks</div><div>Shashi</div><br><div class="gmail_quote_attribution">On Apr 30 2021, at 6:09 am, Peter Maydell <peter.maydell@linaro.org> wrote:</div><blockquote><div><div>On Fri, 30 Apr 2021 at 00:36, <shashi.mallela@linaro.org> wrote:</div><div>></div><div>> On Fri, 2021-04-16 at 18:21 +0100, Peter Maydell wrote:</div><div>> > On Thu, 1 Apr 2021 at 03:41, Shashi Mallela <</div><div>> > shashi.mallela@linaro.org> wrote:</div><div>> > > +#define GITS_PIDR2 0xFFE8</div><div>> ></div><div>> > You probably don't want an offset for GITS_PIDR2 specifically.</div><div>> > Compare handling of GICD_IDREGS in the distributor emulation.</div><div>> > Tried re-using the GICD_IDREGS offset for GITS_PIDR2,but from testing</div><div>> > observed that the linux kernel driver and kvm-unit-tests both relied</div><div>> > on using the 0xFFE8 offset instead of 0xFFD0 (used by GICD & GICR) to</div><div>> > complete ITS initialization and setup</div><br><div>I didn't mean "just put PIDR2 at the GICD_IDREGS offset", which</div><div>obviously won't work, I meant "implement the whole set of ID registers</div><div>(of which PIDR2 is one in the middle) in the same way we implement that</div><div>set on the other GIC components".</div><br><div>-- PMM</div></div></blockquote><img class="mailspring-open" alt="Sent from Mailspring" width="0" height="0" style="border:0; width:0; height:0;" src="https://link.getmailspring.com/open/4B93C023-32E5-468F-80A8-DF58C8CD442F@getmailspring.com?me=2a4b90d6&recipient=cWVtdS1kZXZlbEBub25nbnUub3Jn">
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c new file mode 100644 index 0000000000..209120d102 --- /dev/null +++ b/hw/intc/arm_gicv3_its.c @@ -0,0 +1,334 @@ +/* + * ITS emulation for a GICv3-based system + * + * Copyright Linaro.org 2021 + * + * Authors: + * Shashi Mallela <shashi.mallela@linaro.org> + * + * This work is licensed under the terms of the GNU GPL, version 2 or (at your + * option) any later version. See the COPYING file in the top-level directory. + * + */ + +#include "qemu/osdep.h" +#include "qemu/log.h" +#include "hw/qdev-properties.h" +#include "hw/intc/arm_gicv3_its_common.h" +#include "gicv3_internal.h" +#include "qom/object.h" + +typedef struct GICv3ITSClass GICv3ITSClass; +/* This is reusing the GICv3ITSState typedef from ARM_GICV3_ITS_COMMON */ +DECLARE_OBJ_CHECKERS(GICv3ITSState, GICv3ITSClass, + ARM_GICV3_ITS, TYPE_ARM_GICV3_ITS) + +struct GICv3ITSClass { + GICv3ITSCommonClass parent_class; + void (*parent_reset)(DeviceState *dev); +}; + +static MemTxResult its_trans_writew(GICv3ITSState *s, hwaddr offset, + uint64_t value, MemTxAttrs attrs) +{ + MemTxResult result = MEMTX_OK; + + return result; +} + +static MemTxResult its_trans_writel(GICv3ITSState *s, hwaddr offset, + uint64_t value, MemTxAttrs attrs) +{ + MemTxResult result = MEMTX_OK; + + return result; +} + +static MemTxResult gicv3_its_translation_write(void *opaque, hwaddr offset, + uint64_t data, unsigned size, MemTxAttrs attrs) +{ + GICv3ITSState *s = (GICv3ITSState *)opaque; + MemTxResult result; + + switch (size) { + case 2: + result = its_trans_writew(s, offset, data, attrs); + break; + case 4: + result = its_trans_writel(s, offset, data, attrs); + break; + default: + result = MEMTX_ERROR; + break; + } + + if (result == MEMTX_ERROR) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: invalid guest write at offset " TARGET_FMT_plx + "size %u\n", __func__, offset, size); + /* + * The spec requires that reserved registers are RAZ/WI; + * so use MEMTX_ERROR returns from leaf functions as a way to + * trigger the guest-error logging but don't return it to + * the caller, or we'll cause a spurious guest data abort. + */ + result = MEMTX_OK; + } + return result; +} + +static MemTxResult gicv3_its_translation_read(void *opaque, hwaddr offset, + uint64_t *data, unsigned size, MemTxAttrs attrs) +{ + qemu_log_mask(LOG_GUEST_ERROR, + "%s: Invalid read from translation register area at offset " + TARGET_FMT_plx "\n", __func__, offset); + return MEMTX_ERROR; +} + +static MemTxResult its_writeb(GICv3ITSState *s, hwaddr offset, + uint64_t value, MemTxAttrs attrs) +{ + qemu_log_mask(LOG_UNIMP, + "%s: unsupported byte write to register at offset " + TARGET_FMT_plx "\n", __func__, offset); + return MEMTX_ERROR; +} + +static MemTxResult its_readb(GICv3ITSState *s, hwaddr offset, + uint64_t *data, MemTxAttrs attrs) +{ + qemu_log_mask(LOG_UNIMP, + "%s: unsupported byte read from register at offset " + TARGET_FMT_plx "\n", __func__, offset); + return MEMTX_ERROR; +} + +static MemTxResult its_writew(GICv3ITSState *s, hwaddr offset, + uint64_t value, MemTxAttrs attrs) +{ + qemu_log_mask(LOG_UNIMP, + "%s: unsupported word write to register at offset " + TARGET_FMT_plx "\n", __func__, offset); + return MEMTX_ERROR; +} + +static MemTxResult its_readw(GICv3ITSState *s, hwaddr offset, + uint64_t *data, MemTxAttrs attrs) +{ + qemu_log_mask(LOG_UNIMP, + "%s: unsupported word read from register at offset " + TARGET_FMT_plx "\n", __func__, offset); + return MEMTX_ERROR; +} + +static MemTxResult its_writel(GICv3ITSState *s, hwaddr offset, + uint64_t value, MemTxAttrs attrs) +{ + MemTxResult result = MEMTX_OK; + + return result; +} + +static MemTxResult its_readl(GICv3ITSState *s, hwaddr offset, + uint64_t *data, MemTxAttrs attrs) +{ + MemTxResult result = MEMTX_OK; + + return result; +} + +static MemTxResult its_writell(GICv3ITSState *s, hwaddr offset, + uint64_t value, MemTxAttrs attrs) +{ + MemTxResult result = MEMTX_OK; + + return result; +} + +static MemTxResult its_readll(GICv3ITSState *s, hwaddr offset, + uint64_t *data, MemTxAttrs attrs) +{ + MemTxResult result = MEMTX_OK; + + return result; +} + +static MemTxResult gicv3_its_read(void *opaque, hwaddr offset, uint64_t *data, + unsigned size, MemTxAttrs attrs) +{ + GICv3ITSState *s = (GICv3ITSState *)opaque; + MemTxResult result; + + switch (size) { + case 1: + result = its_readb(s, offset, data, attrs); + break; + case 2: + result = its_readw(s, offset, data, attrs); + break; + case 4: + result = its_readl(s, offset, data, attrs); + break; + case 8: + result = its_readll(s, offset, data, attrs); + break; + default: + result = MEMTX_ERROR; + break; + } + + if (result == MEMTX_ERROR) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: invalid guest read at offset " TARGET_FMT_plx + "size %u\n", __func__, offset, size); + /* + * The spec requires that reserved registers are RAZ/WI; + * so use MEMTX_ERROR returns from leaf functions as a way to + * trigger the guest-error logging but don't return it to + * the caller, or we'll cause a spurious guest data abort. + */ + result = MEMTX_OK; + *data = 0; + } + return result; +} + +static MemTxResult gicv3_its_write(void *opaque, hwaddr offset, uint64_t data, + unsigned size, MemTxAttrs attrs) +{ + GICv3ITSState *s = (GICv3ITSState *)opaque; + MemTxResult result; + + switch (size) { + case 1: + result = its_writeb(s, offset, data, attrs); + break; + case 2: + result = its_writew(s, offset, data, attrs); + break; + case 4: + result = its_writel(s, offset, data, attrs); + break; + case 8: + result = its_writell(s, offset, data, attrs); + break; + default: + result = MEMTX_ERROR; + break; + } + + if (result == MEMTX_ERROR) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: invalid guest write at offset " TARGET_FMT_plx + "size %u\n", __func__, offset, size); + /* + * The spec requires that reserved registers are RAZ/WI; + * so use MEMTX_ERROR returns from leaf functions as a way to + * trigger the guest-error logging but don't return it to + * the caller, or we'll cause a spurious guest data abort. + */ + result = MEMTX_OK; + } + return result; +} + +static const MemoryRegionOps gicv3_its_control_ops = { + .read_with_attrs = gicv3_its_read, + .write_with_attrs = gicv3_its_write, + .endianness = DEVICE_NATIVE_ENDIAN, +}; + +static const MemoryRegionOps gicv3_its_translation_ops = { + .read_with_attrs = gicv3_its_translation_read, + .write_with_attrs = gicv3_its_translation_write, + .endianness = DEVICE_NATIVE_ENDIAN, +}; + +static void gicv3_arm_its_realize(DeviceState *dev, Error **errp) +{ + GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev); + + gicv3_its_init_mmio(s, &gicv3_its_control_ops, &gicv3_its_translation_ops); +} + +static void gicv3_its_reset(DeviceState *dev) +{ + GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev); + GICv3ITSClass *c = ARM_GICV3_ITS_GET_CLASS(s); + + if (s->gicv3->cpu->gicr_typer & GICR_TYPER_PLPIS) { + c->parent_reset(dev); + + /* set the ITS default features supported */ + s->typer = FIELD_DP64(s->typer, GITS_TYPER, PHYSICAL, + GITS_TYPE_PHYSICAL); + s->typer = FIELD_DP64(s->typer, GITS_TYPER, ITT_ENTRY_SIZE, + ITS_ITT_ENTRY_SIZE); + s->typer = FIELD_DP64(s->typer, GITS_TYPER, IDBITS, ITS_IDBITS); + s->typer = FIELD_DP64(s->typer, GITS_TYPER, DEVBITS, ITS_DEVBITS); + s->typer = FIELD_DP64(s->typer, GITS_TYPER, CIL, 1); + s->typer = FIELD_DP64(s->typer, GITS_TYPER, CIDBITS, ITS_CIDBITS); + + /* + * We claim to be an ARM r0p0 with a zero ProductID. + * This is the same as an r0p0 GIC-500. + */ + s->iidr = gicv3_iidr(); + + /* Quiescent bit reset to 1 */ + s->ctlr = FIELD_DP32(s->ctlr, GITS_CTLR, QUIESCENT, 1); + + /* + * setting GITS_BASER0.Type = 0b001 (Device) + * GITS_BASER1.Type = 0b100 (Collection Table) + * GITS_BASER<n>.Type,where n = 3 to 7 are 0b00 (Unimplemented) + * GITS_BASER<0,1>.Page_Size = 64KB + * and default translation table entry size to 16 bytes + */ + s->baser[0] = FIELD_DP64(s->baser[0], GITS_BASER, TYPE, + GITS_ITT_TYPE_DEVICE); + s->baser[0] = FIELD_DP64(s->baser[0], GITS_BASER, PAGESIZE, + GITS_BASER_PAGESIZE_64K); + s->baser[0] = FIELD_DP64(s->baser[0], GITS_BASER, ENTRYSIZE, + GITS_DTE_SIZE); + + s->baser[1] = FIELD_DP64(s->baser[1], GITS_BASER, TYPE, + GITS_ITT_TYPE_COLLECTION); + s->baser[1] = FIELD_DP64(s->baser[1], GITS_BASER, PAGESIZE, + GITS_BASER_PAGESIZE_64K); + s->baser[1] = FIELD_DP64(s->baser[1], GITS_BASER, ENTRYSIZE, + GITS_CTE_SIZE); + } +} + +static Property gicv3_its_props[] = { + DEFINE_PROP_LINK("parent-gicv3", GICv3ITSState, gicv3, "arm-gicv3", + GICv3State *), + DEFINE_PROP_END_OF_LIST(), +}; + +static void gicv3_its_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + GICv3ITSClass *ic = ARM_GICV3_ITS_CLASS(klass); + + dc->realize = gicv3_arm_its_realize; + device_class_set_props(dc, gicv3_its_props); + device_class_set_parent_reset(dc, gicv3_its_reset, &ic->parent_reset); +} + +static const TypeInfo gicv3_its_info = { + .name = TYPE_ARM_GICV3_ITS, + .parent = TYPE_ARM_GICV3_ITS_COMMON, + .instance_size = sizeof(GICv3ITSState), + .class_init = gicv3_its_class_init, + .class_size = sizeof(GICv3ITSClass), +}; + +static void gicv3_its_register_types(void) +{ + type_register_static(&gicv3_its_info); +} + +type_init(gicv3_its_register_types) diff --git a/hw/intc/arm_gicv3_its_common.c b/hw/intc/arm_gicv3_its_common.c index 66c4c6a188..80cc9ec6d8 100644 --- a/hw/intc/arm_gicv3_its_common.c +++ b/hw/intc/arm_gicv3_its_common.c @@ -50,12 +50,16 @@ static int gicv3_its_post_load(void *opaque, int version_id) static const VMStateDescription vmstate_its = { .name = "arm_gicv3_its", + .version_id = 1, + .minimum_version_id = 1, .pre_save = gicv3_its_pre_save, .post_load = gicv3_its_post_load, .priority = MIG_PRI_GICV3_ITS, .fields = (VMStateField[]) { VMSTATE_UINT32(ctlr, GICv3ITSState), + VMSTATE_UINT32(translater, GICv3ITSState), VMSTATE_UINT32(iidr, GICv3ITSState), + VMSTATE_UINT64(typer, GICv3ITSState), VMSTATE_UINT64(cbaser, GICv3ITSState), VMSTATE_UINT64(cwriter, GICv3ITSState), VMSTATE_UINT64(creadr, GICv3ITSState), @@ -99,15 +103,16 @@ static const MemoryRegionOps gicv3_its_trans_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; -void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops) +void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops, + const MemoryRegionOps *tops) { 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_translation, OBJECT(s), - &gicv3_its_trans_ops, s, - "translation", ITS_TRANS_SIZE); + tops ? tops : &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. @@ -130,6 +135,7 @@ static void gicv3_its_common_reset(DeviceState *dev) s->cwriter = 0; s->creadr = 0; s->iidr = 0; + s->translater = 0; memset(&s->baser, 0, sizeof(s->baser)); } diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c index b554d2ede0..0b4cbed28b 100644 --- a/hw/intc/arm_gicv3_its_kvm.c +++ b/hw/intc/arm_gicv3_its_kvm.c @@ -106,7 +106,7 @@ static void kvm_arm_its_realize(DeviceState *dev, Error **errp) kvm_arm_register_device(&s->iomem_its_cntrl, -1, KVM_DEV_ARM_VGIC_GRP_ADDR, KVM_VGIC_ITS_ADDR_TYPE, s->dev_fd, 0); - gicv3_its_init_mmio(s, NULL); + gicv3_its_init_mmio(s, NULL, NULL); if (!kvm_device_check_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_ITS_REGS, GITS_CTLR)) { diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h index 05303a55c8..96cfe2dff9 100644 --- a/hw/intc/gicv3_internal.h +++ b/hw/intc/gicv3_internal.h @@ -24,6 +24,7 @@ #ifndef QEMU_ARM_GICV3_INTERNAL_H #define QEMU_ARM_GICV3_INTERNAL_H +#include "hw/registerfields.h" #include "hw/intc/arm_gicv3_common.h" /* Distributor registers, as offsets from the distributor base address */ @@ -67,6 +68,9 @@ #define GICD_CTLR_E1NWF (1U << 7) #define GICD_CTLR_RWP (1U << 31) +/* 16 bits EventId */ +#define GICD_TYPER_IDBITS 0xf + /* * Redistributor frame offsets from RD_base */ @@ -122,18 +126,6 @@ #define GICR_WAKER_ProcessorSleep (1U << 1) #define GICR_WAKER_ChildrenAsleep (1U << 2) -#define GICR_PROPBASER_OUTER_CACHEABILITY_MASK (7ULL << 56) -#define GICR_PROPBASER_ADDR_MASK (0xfffffffffULL << 12) -#define GICR_PROPBASER_SHAREABILITY_MASK (3U << 10) -#define GICR_PROPBASER_CACHEABILITY_MASK (7U << 7) -#define GICR_PROPBASER_IDBITS_MASK (0x1f) - -#define GICR_PENDBASER_PTZ (1ULL << 62) -#define GICR_PENDBASER_OUTER_CACHEABILITY_MASK (7ULL << 56) -#define GICR_PENDBASER_ADDR_MASK (0xffffffffULL << 16) -#define GICR_PENDBASER_SHAREABILITY_MASK (3U << 10) -#define GICR_PENDBASER_CACHEABILITY_MASK (7U << 7) - #define ICC_CTLR_EL1_CBPR (1U << 0) #define ICC_CTLR_EL1_EOIMODE (1U << 1) #define ICC_CTLR_EL1_PMHE (1U << 6) @@ -239,6 +231,76 @@ #define ICH_VTR_EL2_PREBITS_SHIFT 26 #define ICH_VTR_EL2_PRIBITS_SHIFT 29 +/* ITS Registers */ + +FIELD(GITS_BASER, SIZE, 0, 8) +FIELD(GITS_BASER, PAGESIZE, 8, 2) +FIELD(GITS_BASER, SHAREABILITY, 10, 2) +FIELD(GITS_BASER, PHYADDR, 12, 36) +FIELD(GITS_BASER, PHYADDRL_64K, 16, 32) +FIELD(GITS_BASER, PHYADDRH_64K, 48, 4) +FIELD(GITS_BASER, ENTRYSIZE, 48, 5) +FIELD(GITS_BASER, OUTERCACHE, 53, 3) +FIELD(GITS_BASER, TYPE, 56, 3) +FIELD(GITS_BASER, INNERCACHE, 59, 3) +FIELD(GITS_BASER, INDIRECT, 62, 1) +FIELD(GITS_BASER, VALID, 63, 1) + +#define GITS_BASER_PAGESIZE_4K 0 +#define GITS_BASER_PAGESIZE_16K 1 +#define GITS_BASER_PAGESIZE_64K 2 + +#define GITS_ITT_TYPE_DEVICE 1ULL +#define GITS_ITT_TYPE_COLLECTION 4ULL + +FIELD(GITS_CTLR, QUIESCENT, 31, 1) + +FIELD(GITS_TYPER, PHYSICAL, 0, 1) +FIELD(GITS_TYPER, ITT_ENTRY_SIZE, 4, 4) +FIELD(GITS_TYPER, IDBITS, 8, 5) +FIELD(GITS_TYPER, DEVBITS, 13, 5) +FIELD(GITS_TYPER, SEIS, 18, 1) +FIELD(GITS_TYPER, PTA, 19, 1) +FIELD(GITS_TYPER, CIDBITS, 32, 4) +FIELD(GITS_TYPER, CIL, 36, 1) + +/** + * Default features advertised by this version of ITS + */ +/* Physical LPIs supported */ +#define GITS_TYPE_PHYSICAL (1U << 0) + +/* + * 11 bytes Interrupt translation Table Entry size + * Valid = 1 bit,InterruptType = 1 bit, + * Size of LPI number space[considering max 24 bits], + * Size of LPI number space[considering max 24 bits], + * ICID = 16 bits, + * vPEID = 16 bits + */ +#define ITS_ITT_ENTRY_SIZE 0xB + +/* 16 bits EventId */ +#define ITS_IDBITS GICD_TYPER_IDBITS + +/* 16 bits DeviceId */ +#define ITS_DEVBITS 0xF + +/* 16 bits CollectionId */ +#define ITS_CIDBITS 0xF + +/* + * 8 bytes Device Table Entry size + * Valid = 1 bit,ITTAddr = 44 bits,Size = 5 bits + */ +#define GITS_DTE_SIZE (0x8ULL) + +/* + * 8 bytes Collection Table Entry size + * Valid = 1 bit,RDBase = 36 bits(considering max RDBASE) + */ +#define GITS_CTE_SIZE (0x8ULL) + /* Special interrupt IDs */ #define INTID_SECURE 1020 #define INTID_NONSECURE 1021 diff --git a/hw/intc/meson.build b/hw/intc/meson.build index 1c299039f6..53472239f0 100644 --- a/hw/intc/meson.build +++ b/hw/intc/meson.build @@ -8,6 +8,7 @@ softmmu_ss.add(when: 'CONFIG_ARM_GIC', if_true: files( 'arm_gicv3_dist.c', 'arm_gicv3_its_common.c', 'arm_gicv3_redist.c', + 'arm_gicv3_its.c', )) softmmu_ss.add(when: 'CONFIG_ETRAXFS', if_true: files('etraxfs_pic.c')) softmmu_ss.add(when: 'CONFIG_HEATHROW_PIC', if_true: files('heathrow_pic.c')) diff --git a/include/hw/intc/arm_gicv3_its_common.h b/include/hw/intc/arm_gicv3_its_common.h index 5a0952b404..08bc5652ed 100644 --- a/include/hw/intc/arm_gicv3_its_common.h +++ b/include/hw/intc/arm_gicv3_its_common.h @@ -25,17 +25,24 @@ #include "hw/intc/arm_gicv3_common.h" #include "qom/object.h" +#define TYPE_ARM_GICV3_ITS "arm-gicv3-its" + #define ITS_CONTROL_SIZE 0x10000 #define ITS_TRANS_SIZE 0x10000 #define ITS_SIZE (ITS_CONTROL_SIZE + ITS_TRANS_SIZE) #define GITS_CTLR 0x0 #define GITS_IIDR 0x4 +#define GITS_TYPER 0x8 #define GITS_CBASER 0x80 #define GITS_CWRITER 0x88 #define GITS_CREADR 0x90 #define GITS_BASER 0x100 +#define GITS_TRANSLATER 0x0040 + +#define GITS_PIDR2 0xFFE8 + struct GICv3ITSState { SysBusDevice parent_obj; @@ -52,6 +59,8 @@ struct GICv3ITSState { /* Registers */ uint32_t ctlr; uint32_t iidr; + uint32_t translater; + uint64_t typer; uint64_t cbaser; uint64_t cwriter; uint64_t creadr; @@ -62,7 +71,8 @@ struct GICv3ITSState { typedef struct GICv3ITSState GICv3ITSState; -void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops); +void gicv3_its_init_mmio(GICv3ITSState *s, const MemoryRegionOps *ops, + const MemoryRegionOps *tops); #define TYPE_ARM_GICV3_ITS_COMMON "arm-gicv3-its-common" typedef struct GICv3ITSCommonClass GICv3ITSCommonClass;
Added register definitions relevant to ITS,implemented overall ITS device framework with stubs for ITS control and translater regions read/write,extended ITS common to handle mmio init between existing kvm device and newer qemu device. Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org> --- hw/intc/arm_gicv3_its.c | 334 +++++++++++++++++++++++++ hw/intc/arm_gicv3_its_common.c | 12 +- hw/intc/arm_gicv3_its_kvm.c | 2 +- hw/intc/gicv3_internal.h | 86 ++++++- hw/intc/meson.build | 1 + include/hw/intc/arm_gicv3_its_common.h | 12 +- 6 files changed, 430 insertions(+), 17 deletions(-) create mode 100644 hw/intc/arm_gicv3_its.c -- 2.27.0