| Message ID | 20170403202829.7278-15-andre.przywara@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Andre, On 03/04/17 21:28, Andre Przywara wrote: > Create a new file to hold the emulation code for the ITS widget. > For now we emulate the memory mapped ITS registers and provide a stub > to introduce the ITS command handling framework (but without actually > emulating any commands at this time). The ITS is a complex piece so I think it would be good to describe more in the commit message how this will work. Also a documentation in the tree would be very good to help understanding the code. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > xen/arch/arm/Makefile | 1 + > xen/arch/arm/vgic-v3-its.c | 547 ++++++++++++++++++++++++++++++++++++++ > xen/arch/arm/vgic-v3.c | 9 - > xen/include/asm-arm/gic_v3_defs.h | 19 ++ > xen/include/asm-arm/gic_v3_its.h | 2 + > 5 files changed, 569 insertions(+), 9 deletions(-) > create mode 100644 xen/arch/arm/vgic-v3-its.c > > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile > index 6be85ab..49e1fb2 100644 > --- a/xen/arch/arm/Makefile > +++ b/xen/arch/arm/Makefile > @@ -47,6 +47,7 @@ obj-y += traps.o > obj-y += vgic.o > obj-y += vgic-v2.o > obj-$(CONFIG_HAS_GICV3) += vgic-v3.o > +obj-$(CONFIG_HAS_ITS) += vgic-v3-its.o > obj-y += vm_event.o > obj-y += vtimer.o > obj-y += vpsci.o > diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c > new file mode 100644 > index 0000000..fd3b9a1 > --- /dev/null > +++ b/xen/arch/arm/vgic-v3-its.c > @@ -0,0 +1,547 @@ > +/* > + * xen/arch/arm/vgic-v3-its.c > + * > + * ARM Interrupt Translation Service (ITS) emulation > + * > + * Andre Przywara <andre.przywara@arm.com> > + * Copyright (c) 2016,2017 ARM Ltd. > + * > + * 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; under version 2 of the License. > + * > + * 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 <xen/bitops.h> > +#include <xen/config.h> > +#include <xen/domain_page.h> > +#include <xen/lib.h> > +#include <xen/init.h> > +#include <xen/softirq.h> > +#include <xen/irq.h> > +#include <xen/sched.h> > +#include <xen/sizes.h> > +#include <asm/current.h> > +#include <asm/mmio.h> > +#include <asm/gic_v3_defs.h> > +#include <asm/gic_v3_its.h> > +#include <asm/vgic.h> > +#include <asm/vgic-emul.h> > + > +/* Data structure to describe a virtual ITS */ > +#define VIRT_ITS_ENABLED 0 > +#define VIRT_ITS_COLL_VALID 1 > +#define VIRT_ITS_DEV_VALID 2 > +#define VIRT_ITS_CMDBUF_VALID 3 > +struct virt_its { > + struct domain *d; > + spinlock_t vcmd_lock; /* Protects the virtual command buffer. */ > + uint64_t cbaser; > + uint64_t cwriter; > + uint64_t creadr; > + spinlock_t its_lock; /* Protects the collection and device tables. */ > + uint64_t baser_dev, baser_coll; Well, you renamed the baser{0,1} to a more accurate name. But now, you are missing document explaining which one is BASER0 and BASER1. > + unsigned int max_collections; > + unsigned int max_devices; > + unsigned int devid_bits; > + unsigned int intid_bits; > + unsigned long flags; > +}; > + > +/* > + * An Interrupt Translation Table Entry: this is indexed by a > + * DeviceID/EventID pair and is located in guest memory. > + */ > +struct vits_itte > +{ > + uint32_t vlpi; > + uint16_t collection; > + uint16_t pad; > +}; > + > +static bool its_is_enabled(struct virt_its *its) > +{ > + return test_bit(VIRT_ITS_ENABLED, &its->flags); > +} > + > +/************************************** > + * Functions that handle ITS commands * > + **************************************/ > + > +static uint64_t its_cmd_mask_field(uint64_t *its_cmd, unsigned int word, > + unsigned int shift, unsigned int size) > +{ > + return (le64_to_cpu(its_cmd[word]) >> shift) & (BIT(size) - 1); > +} > + > +#define its_cmd_get_command(cmd) its_cmd_mask_field(cmd, 0, 0, 8) > +#define its_cmd_get_deviceid(cmd) its_cmd_mask_field(cmd, 0, 32, 32) > +#define its_cmd_get_size(cmd) its_cmd_mask_field(cmd, 1, 0, 5) > +#define its_cmd_get_id(cmd) its_cmd_mask_field(cmd, 1, 0, 32) > +#define its_cmd_get_physical_id(cmd) its_cmd_mask_field(cmd, 1, 32, 32) > +#define its_cmd_get_collection(cmd) its_cmd_mask_field(cmd, 2, 0, 16) > +#define its_cmd_get_target_addr(cmd) its_cmd_mask_field(cmd, 2, 16, 32) > +#define its_cmd_get_validbit(cmd) its_cmd_mask_field(cmd, 2, 63, 1) > + > +#define ITS_CMD_BUFFER_SIZE(baser) ((((baser) & 0xff) + 1) << 12) > + > +static int vgic_its_handle_cmds(struct domain *d, struct virt_its *its, > + uint32_t writer) s/uint32_t/uint64_t/ > +{ > + paddr_t cmdbuf_addr = its->cbaser & GENMASK_ULL(51, 12); > + void *cmdbuf = NULL; > + uint64_t *cmdptr; > + > + if ( writer >= ITS_CMD_BUFFER_SIZE(its->cbaser) ) > + return -1; You return an error value but the caller does not check it. Should not the caller do a different action when the return -1? If not, it should be documented. > + > + spin_lock(&its->vcmd_lock); I am still concerned about the locking here as you may interpret 32K of commands in one go. We spoke about mitigation and I was expected some TODOs in the code about that... > + > + while ( its->creadr != writer ) > + { > + int ret; > + > + ret = 0; > + > + /* > + * If this is the first command we handle or we cross a page boundary, > + * we need to (re)map the command buffer. > + */ > + if ( !cmdbuf || (its->creadr & ~PAGE_MASK) == 0 ) > + { > + if ( cmdbuf ) > + unmap_one_guest_page(cmdbuf); > + cmdbuf = map_one_guest_page(d, > + (cmdbuf_addr + its->creadr) & PAGE_MASK); > + if ( !cmdbuf ) > + return -EFAULT; > + } > + cmdptr = cmdbuf + (its->creadr & ~PAGE_MASK); > + > + switch ( its_cmd_get_command(cmdptr) ) > + { > + case GITS_CMD_SYNC: > + /* We handle ITS commands synchronously, so we ignore SYNC. */ > + break; The indentation is wrong. > + default: > + gdprintk(XENLOG_WARNING, "ITS: unhandled ITS command %lu\n", > + its_cmd_get_command(cmdptr)); > + break; > + } > + > + its->creadr += ITS_CMD_SIZE; > + if ( its->creadr == ITS_CMD_BUFFER_SIZE(its->cbaser) ) > + its->creadr = 0; > + > + if ( ret ) > + gdprintk(XENLOG_WARNING, > + "ITS: ITS command error %d while handling command %lu\n", > + ret, its_cmd_get_command(cmdptr)); > + } > + its->cwriter = writer; I think its->cwriter should be updated before the loop. So another vCPU could read the correct CWRITER whilst this vCPU is executing the commands. > + > + spin_unlock(&its->vcmd_lock); > + > + if ( cmdbuf ) > + unmap_one_guest_page(cmdbuf); > + > + return 0; > +} > + > +/***************************** > + * ITS registers read access * > + *****************************/ > + > +static int vgic_v3_its_mmio_read(struct vcpu *v, mmio_info_t *info, > + register_t *r, void *priv) > +{ > + struct virt_its *its = priv; > + uint64_t reg; > + > + switch ( info->gpa & 0xffff ) > + { > + case VREG32(GITS_CTLR): > + if ( info->dabt.size != DABT_WORD ) goto bad_width; > + if ( its_is_enabled(its) ) > + reg = GITS_CTLR_ENABLE | BIT(31); Please define BIT(31). Also, technically the ITS is not quiescent when commands are executed (GITS_CTLR could be read from another vCPU). > + else > + reg = BIT(31); Ditto. > + *r = vgic_reg32_extract(reg, info); > + break; > + case VREG32(GITS_IIDR): > + if ( info->dabt.size != DABT_WORD ) goto bad_width; > + *r = vgic_reg32_extract(GITS_IIDR_VALUE, info); > + break; > + case VREG64(GITS_TYPER): > + if ( !vgic_reg64_check_access(info->dabt) ) goto bad_width; > + > + reg = GITS_TYPER_PHYSICAL; Can you please document the configuration of the vITS. > + reg |= (sizeof(struct vits_itte) - 1) << GITS_TYPER_ITT_SIZE_SHIFT; > + reg |= (its->intid_bits - 1) << GITS_TYPER_IDBITS_SHIFT; > + reg |= (its->devid_bits - 1) << GITS_TYPER_DEVIDS_SHIFT; > + *r = vgic_reg64_extract(reg, info); > + break; > + case VREG64(GITS_CBASER): > + if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width; Coding style if ( !vgic... > + *r = vgic_reg64_extract(its->cbaser, info); I don't think you can assume vgic_reg64_extract will be atomic. > + break; > + case VREG64(GITS_CWRITER): > + if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width; Coding style > + *r = vgic_reg64_extract(its->cwriter, info); Ditto. > + break; > + case VREG64(GITS_CREADR): > + if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width; Coding style > + *r = vgic_reg64_extract(its->creadr, info); Ditto. > + break; > + case VREG64(GITS_BASER0): > + if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width; Coding style > + *r = vgic_reg64_extract(its->baser_dev, info); Ditto. > + break; > + case VREG64(GITS_BASER1): > + if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width; Coding style > + *r = vgic_reg64_extract(its->baser_coll, info); Ditto. > + break; > + case VRANGE64(GITS_BASER2, GITS_BASER7): > + if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width; Coding style > + *r = vgic_reg64_extract(0, info); Please introduce a label read_as_zero_64 at the end and do the implementation of RAZ there. It will acts as a documentation too (see an example in vgic-v3.c). Also, vgic_reg64_extract(0, info) will ... always return 0. So you can optimize it ;). > + break; > + case VREG32(GITS_PIDR2): > + if ( info->dabt.size != DABT_WORD ) goto bad_width; > + *r = vgic_reg32_extract(GICV3_GICD_PIDR2, info); > + break; Please add all the registers even implementation defined and reserved one. Ignoring registers without any warning is usually a bad idea as it makes very difficult to debug it. You can look at vgic-v3.c for an example. > + } > + > + return 1; > + > +bad_width: Please print an error here (see vgic-v3.c). > + domain_crash_synchronous(); > + > + return 0; > +} > + > +/****************************** > + * ITS registers write access * > + ******************************/ > + > +static int its_baser_table_size(uint64_t baser) unsigned int for the return. > +{ > + int page_size = 0; unsigned int. > + > + switch ( (baser >> 8) & 3 ) Please define 8 and 3. > + { > + case 0: page_size = SZ_4K; break; > + case 1: page_size = SZ_16K; break; > + case 2: > + case 3: page_size = SZ_64K; break; > + } It looks like to me that the switch could be turned into an array: unsigned page_size[] = {SZ_4K, SZ_16K, SZ_64K, SZ_64K}; This woudl make the code simpler. > + > + return page_size * ((baser & GENMASK_ULL(7, 0)) + 1); > +} > + > +static int its_baser_nr_entries(uint64_t baser) unsigned int for the return and the function would probably benefit to be inlined. > +{ > + int entry_size = ((baser & GENMASK_ULL(52, 48)) >> 48) + 1; unsigned int for the type. Also please use a define for 48. > + > + return its_baser_table_size(baser) / entry_size; > +} [...] > +static int vgic_v3_its_mmio_write(struct vcpu *v, mmio_info_t *info, > + register_t r, void *priv) > +{ > + struct domain *d = v->domain; > + struct virt_its *its = priv; > + uint64_t reg; > + uint32_t reg32, ctlr; ctlr could be defined in the case... > + > + switch ( info->gpa & 0xffff ) > + { > + case VREG32(GITS_CTLR): here. I tend to prefer to restrict the scope whenever it is possible. > + if ( info->dabt.size != DABT_WORD ) goto bad_width; > + > + ctlr = its_is_enabled(its) ? GITS_CTLR_ENABLE : 0; > + reg32 = ctlr; > + vgic_reg32_update(®32, r, info); > + > + if ( ctlr ^ reg32 ) > + vgic_v3_its_change_its_status(its, reg32 & GITS_CTLR_ENABLE); This does not look atomic, so you could end-up calling this twice. > + return 1; > + > + case VREG32(GITS_IIDR): > + goto write_ignore_32; > + case VREG32(GITS_TYPER): > + goto write_ignore_32; > + case VREG64(GITS_CBASER): > + if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width; Coding style. > + > + /* Changing base registers with the ITS enabled is UNPREDICTABLE. */ > + if ( its_is_enabled(its) ) See my question about atomicity. > + { > + gdprintk(XENLOG_WARNING, "ITS: Domain %d tried to change CBASER with the ITS enabled.\n", d->domain_id); > + return 1; > + } > + > + reg = its->cbaser; > + vgic_reg64_update(®, r, info); > + sanitize_its_base_reg(®); > + > + vgic_its_unmap_cmdbuf(its); Looking at the implementation of vgic_its_unmap_cmdbuf, you have no safety to check whether the region was mapped before. Also, from the spec, 8.19.2 in ARM IHI 0069C), GITS_CREADR (i.e its->creadr) should be reset to 0. > + its->cbaser = reg; > + > + return 1; > + > + case VREG64(GITS_CWRITER): > + if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width; Coding style. > + reg = its->cwriter & 0xfffe0; > + vgic_reg64_update(®, r, info); > + its->cwriter = reg & 0xfffe0; > + > + if ( its_is_enabled(its) ) > + vgic_its_handle_cmds(d, its, reg); I was expecting you to check the return here. > + > + return 1; > + > + case VREG64(GITS_CREADR): > + goto write_ignore_64; > + case VREG64(GITS_BASER0): > + if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width; Coding style. > + > + /* Changing base registers with the ITS enabled is UNPREDICTABLE. */ > + if ( its_is_enabled(its) ) > + { > + gdprintk(XENLOG_WARNING, "ITS: Domain %d tried to change BASER with the ITS enabled.\n", > + d->domain_id); > + > + return 1; > + } I was expecting some locking here because this could be called concurrently... > + > + reg = its->baser_dev; > + vgic_reg64_update(®, r, info); > + > + reg &= ~GITS_BASER_RO_MASK; > + reg |= (sizeof(uint64_t) - 1) << GITS_BASER_ENTRY_SIZE_SHIFT; > + reg |= GITS_BASER_TYPE_DEVICE << GITS_BASER_TYPE_SHIFT; > + sanitize_its_base_reg(®); > + > + /* Has the table address been changed or invalidated? */ > + if ( !(reg & GITS_VALID_BIT) || > + get_baser_phys_addr(reg) != get_baser_phys_addr(its->baser_dev) ) > + { > + vgic_its_unmap_its_table(its->d, its->baser_dev); > + clear_bit(VIRT_ITS_DEV_VALID, &its->flags); > + } > + > + if ( reg & GITS_VALID_BIT ) > + its->max_devices = its_baser_nr_entries(reg); > + else > + its->max_devices = 0; > + > + its->baser_dev = reg; > + return 1; > + case VREG64(GITS_BASER1): > + if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width; > + > + /* Changing base registers with the ITS enabled is UNPREDICTABLE. */ > + if ( its_is_enabled(its) ) > + { > + gdprintk(XENLOG_INFO, "ITS: Domain %d tried to change BASER with the ITS enabled.\n", > + d->domain_id); > + return 1; > + } > + > + reg = its->baser_coll; > + vgic_reg64_update(®, r, info); > + reg &= ~GITS_BASER_RO_MASK; > + reg |= (sizeof(uint16_t) - 1) << GITS_BASER_ENTRY_SIZE_SHIFT; > + reg |= GITS_BASER_TYPE_COLLECTION << GITS_BASER_TYPE_SHIFT; > + sanitize_its_base_reg(®); > + > + if ( !(reg & GITS_VALID_BIT) || > + get_baser_phys_addr(reg) != get_baser_phys_addr(its->baser_coll) ) > + { > + vgic_its_unmap_its_table(its->d, its->baser_coll); > + clear_bit(VIRT_ITS_COLL_VALID, &its->flags); > + } > + > + if ( reg & GITS_VALID_BIT ) > + its->max_collections = its_baser_nr_entries(reg); > + else > + its->max_collections = 0; > + its->baser_coll = reg; > + return 1; > + case VRANGE64(GITS_BASER2, GITS_BASER7): > + goto write_ignore_64; From the ITS register map, we would have to emulate more register (at least reserved, implementation defined and RAZ). > + default: > + gdprintk(XENLOG_G_WARNING, "ITS: unhandled ITS register 0x%lx\n", > + info->gpa & 0xffff); > + return 0; > + } > + > + return 1; > + > +write_ignore_64: > + if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width; Coding style. > + return 1; > + > +write_ignore_32: > + if ( info->dabt.size != DABT_WORD ) goto bad_width; > + return 1; > + > +bad_width: > + printk(XENLOG_G_ERR "%pv vGICR: bad read width %d r%d offset %#08lx\n", > + v, info->dabt.size, info->dabt.reg, info->gpa & 0xffff); > + This is ITS emulation not GICR ;). > + domain_crash_synchronous(); > + > + return 0; > +} > + > +static const struct mmio_handler_ops vgic_its_mmio_handler = { > + .read = vgic_v3_its_mmio_read, > + .write = vgic_v3_its_mmio_write, > +}; > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * indent-tabs-mode: nil > + * End: > + */ > diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c > index 2c6b317..ebcfc16 100644 > --- a/xen/arch/arm/vgic-v3.c > +++ b/xen/arch/arm/vgic-v3.c > @@ -158,15 +158,6 @@ static void vgic_store_irouter(struct domain *d, struct vgic_irq_rank *rank, > write_atomic(&rank->vcpu[offset], new_vcpu->vcpu_id); > } > > -static inline bool vgic_reg64_check_access(struct hsr_dabt dabt) > -{ > - /* > - * 64 bits registers can be accessible using 32-bit and 64-bit unless > - * stated otherwise (See 8.1.3 ARM IHI 0069A). > - */ > - return ( dabt.size == DABT_DOUBLE_WORD || dabt.size == DABT_WORD ); > -} > - A separate commit would have been nice. [...] > diff --git a/xen/include/asm-arm/gic_v3_defs.h b/xen/include/asm-arm/gic_v3_defs.h > index b01b6ed..8999937 100644 > --- a/xen/include/asm-arm/gic_v3_defs.h > +++ b/xen/include/asm-arm/gic_v3_defs.h > @@ -155,6 +155,16 @@ > #define LPI_PROP_RES1 (1 << 1) > #define LPI_PROP_ENABLED (1 << 0) > > +/* > + * PIDR2: Only bits[7:4] are not implementation defined. We are > + * emulating a GICv3 ([7:4] = 0x3). > + * > + * We don't emulate a specific registers scheme so implement the others > + * bits as RES0 as recommended by the spec (see 8.1.13 in ARM IHI 0069A). > + */ > +#define GICV3_GICD_PIDR2 0x30 > +#define GICV3_GICR_PIDR2 GICV3_GICD_PIDR2 > + Those values should not be defined in gic_v3_defs.h but a vgic headers. My rationale is, those value are implementation defined (e.g depends on the emulation). > #define GICH_VMCR_EOI (1 << 9) > #define GICH_VMCR_VENG1 (1 << 1) > > @@ -198,6 +208,15 @@ struct rdist_region { > bool single_rdist; > }; > > +/* > + * 64 bits registers can be accessible using 32-bit and 64-bit unless > + * stated otherwise (See 8.1.3 ARM IHI 0069A). > + */ > +static inline bool vgic_reg64_check_access(struct hsr_dabt dabt) > +{ > + return ( dabt.size == DABT_DOUBLE_WORD || dabt.size == DABT_WORD ); > +} This is the wrong place for this helper. I think this should go in vgic-emul.h. > + > #endif /* __ASM_ARM_GIC_V3_DEFS_H__ */ > > /* > diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h > index d8165c4..b2a9586 100644 > --- a/xen/include/asm-arm/gic_v3_its.h > +++ b/xen/include/asm-arm/gic_v3_its.h > @@ -35,6 +35,7 @@ > #define GITS_BASER5 0x128 > #define GITS_BASER6 0x130 > #define GITS_BASER7 0x138 > +#define GITS_PIDR2 GICR_PIDR2 > > /* Register bits */ > #define GITS_VALID_BIT BIT_ULL(63) > @@ -56,6 +57,7 @@ > #define GITS_TYPER_ITT_SIZE_MASK (0xfUL << GITS_TYPER_ITT_SIZE_SHIFT) > #define GITS_TYPER_ITT_SIZE(r) ((((r) & GITS_TYPER_ITT_SIZE_MASK) >> \ > GITS_TYPER_ITT_SIZE_SHIFT) + 1) > +#define GITS_TYPER_PHYSICAL (1U << 0) > > #define GITS_IIDR_VALUE 0x34c > > Regards,
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile index 6be85ab..49e1fb2 100644 --- a/xen/arch/arm/Makefile +++ b/xen/arch/arm/Makefile @@ -47,6 +47,7 @@ obj-y += traps.o obj-y += vgic.o obj-y += vgic-v2.o obj-$(CONFIG_HAS_GICV3) += vgic-v3.o +obj-$(CONFIG_HAS_ITS) += vgic-v3-its.o obj-y += vm_event.o obj-y += vtimer.o obj-y += vpsci.o diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c new file mode 100644 index 0000000..fd3b9a1 --- /dev/null +++ b/xen/arch/arm/vgic-v3-its.c @@ -0,0 +1,547 @@ +/* + * xen/arch/arm/vgic-v3-its.c + * + * ARM Interrupt Translation Service (ITS) emulation + * + * Andre Przywara <andre.przywara@arm.com> + * Copyright (c) 2016,2017 ARM Ltd. + * + * 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; under version 2 of the License. + * + * 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 <xen/bitops.h> +#include <xen/config.h> +#include <xen/domain_page.h> +#include <xen/lib.h> +#include <xen/init.h> +#include <xen/softirq.h> +#include <xen/irq.h> +#include <xen/sched.h> +#include <xen/sizes.h> +#include <asm/current.h> +#include <asm/mmio.h> +#include <asm/gic_v3_defs.h> +#include <asm/gic_v3_its.h> +#include <asm/vgic.h> +#include <asm/vgic-emul.h> + +/* Data structure to describe a virtual ITS */ +#define VIRT_ITS_ENABLED 0 +#define VIRT_ITS_COLL_VALID 1 +#define VIRT_ITS_DEV_VALID 2 +#define VIRT_ITS_CMDBUF_VALID 3 +struct virt_its { + struct domain *d; + spinlock_t vcmd_lock; /* Protects the virtual command buffer. */ + uint64_t cbaser; + uint64_t cwriter; + uint64_t creadr; + spinlock_t its_lock; /* Protects the collection and device tables. */ + uint64_t baser_dev, baser_coll; + unsigned int max_collections; + unsigned int max_devices; + unsigned int devid_bits; + unsigned int intid_bits; + unsigned long flags; +}; + +/* + * An Interrupt Translation Table Entry: this is indexed by a + * DeviceID/EventID pair and is located in guest memory. + */ +struct vits_itte +{ + uint32_t vlpi; + uint16_t collection; + uint16_t pad; +}; + +static bool its_is_enabled(struct virt_its *its) +{ + return test_bit(VIRT_ITS_ENABLED, &its->flags); +} + +/************************************** + * Functions that handle ITS commands * + **************************************/ + +static uint64_t its_cmd_mask_field(uint64_t *its_cmd, unsigned int word, + unsigned int shift, unsigned int size) +{ + return (le64_to_cpu(its_cmd[word]) >> shift) & (BIT(size) - 1); +} + +#define its_cmd_get_command(cmd) its_cmd_mask_field(cmd, 0, 0, 8) +#define its_cmd_get_deviceid(cmd) its_cmd_mask_field(cmd, 0, 32, 32) +#define its_cmd_get_size(cmd) its_cmd_mask_field(cmd, 1, 0, 5) +#define its_cmd_get_id(cmd) its_cmd_mask_field(cmd, 1, 0, 32) +#define its_cmd_get_physical_id(cmd) its_cmd_mask_field(cmd, 1, 32, 32) +#define its_cmd_get_collection(cmd) its_cmd_mask_field(cmd, 2, 0, 16) +#define its_cmd_get_target_addr(cmd) its_cmd_mask_field(cmd, 2, 16, 32) +#define its_cmd_get_validbit(cmd) its_cmd_mask_field(cmd, 2, 63, 1) + +#define ITS_CMD_BUFFER_SIZE(baser) ((((baser) & 0xff) + 1) << 12) + +static int vgic_its_handle_cmds(struct domain *d, struct virt_its *its, + uint32_t writer) +{ + paddr_t cmdbuf_addr = its->cbaser & GENMASK_ULL(51, 12); + void *cmdbuf = NULL; + uint64_t *cmdptr; + + if ( writer >= ITS_CMD_BUFFER_SIZE(its->cbaser) ) + return -1; + + spin_lock(&its->vcmd_lock); + + while ( its->creadr != writer ) + { + int ret; + + ret = 0; + + /* + * If this is the first command we handle or we cross a page boundary, + * we need to (re)map the command buffer. + */ + if ( !cmdbuf || (its->creadr & ~PAGE_MASK) == 0 ) + { + if ( cmdbuf ) + unmap_one_guest_page(cmdbuf); + cmdbuf = map_one_guest_page(d, + (cmdbuf_addr + its->creadr) & PAGE_MASK); + if ( !cmdbuf ) + return -EFAULT; + } + cmdptr = cmdbuf + (its->creadr & ~PAGE_MASK); + + switch ( its_cmd_get_command(cmdptr) ) + { + case GITS_CMD_SYNC: + /* We handle ITS commands synchronously, so we ignore SYNC. */ + break; + default: + gdprintk(XENLOG_WARNING, "ITS: unhandled ITS command %lu\n", + its_cmd_get_command(cmdptr)); + break; + } + + its->creadr += ITS_CMD_SIZE; + if ( its->creadr == ITS_CMD_BUFFER_SIZE(its->cbaser) ) + its->creadr = 0; + + if ( ret ) + gdprintk(XENLOG_WARNING, + "ITS: ITS command error %d while handling command %lu\n", + ret, its_cmd_get_command(cmdptr)); + } + its->cwriter = writer; + + spin_unlock(&its->vcmd_lock); + + if ( cmdbuf ) + unmap_one_guest_page(cmdbuf); + + return 0; +} + +/***************************** + * ITS registers read access * + *****************************/ + +static int vgic_v3_its_mmio_read(struct vcpu *v, mmio_info_t *info, + register_t *r, void *priv) +{ + struct virt_its *its = priv; + uint64_t reg; + + switch ( info->gpa & 0xffff ) + { + case VREG32(GITS_CTLR): + if ( info->dabt.size != DABT_WORD ) goto bad_width; + if ( its_is_enabled(its) ) + reg = GITS_CTLR_ENABLE | BIT(31); + else + reg = BIT(31); + *r = vgic_reg32_extract(reg, info); + break; + case VREG32(GITS_IIDR): + if ( info->dabt.size != DABT_WORD ) goto bad_width; + *r = vgic_reg32_extract(GITS_IIDR_VALUE, info); + break; + case VREG64(GITS_TYPER): + if ( !vgic_reg64_check_access(info->dabt) ) goto bad_width; + + reg = GITS_TYPER_PHYSICAL; + reg |= (sizeof(struct vits_itte) - 1) << GITS_TYPER_ITT_SIZE_SHIFT; + reg |= (its->intid_bits - 1) << GITS_TYPER_IDBITS_SHIFT; + reg |= (its->devid_bits - 1) << GITS_TYPER_DEVIDS_SHIFT; + *r = vgic_reg64_extract(reg, info); + break; + case VREG64(GITS_CBASER): + if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width; + *r = vgic_reg64_extract(its->cbaser, info); + break; + case VREG64(GITS_CWRITER): + if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width; + *r = vgic_reg64_extract(its->cwriter, info); + break; + case VREG64(GITS_CREADR): + if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width; + *r = vgic_reg64_extract(its->creadr, info); + break; + case VREG64(GITS_BASER0): + if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width; + *r = vgic_reg64_extract(its->baser_dev, info); + break; + case VREG64(GITS_BASER1): + if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width; + *r = vgic_reg64_extract(its->baser_coll, info); + break; + case VRANGE64(GITS_BASER2, GITS_BASER7): + if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width; + *r = vgic_reg64_extract(0, info); + break; + case VREG32(GITS_PIDR2): + if ( info->dabt.size != DABT_WORD ) goto bad_width; + *r = vgic_reg32_extract(GICV3_GICD_PIDR2, info); + break; + } + + return 1; + +bad_width: + domain_crash_synchronous(); + + return 0; +} + +/****************************** + * ITS registers write access * + ******************************/ + +static int its_baser_table_size(uint64_t baser) +{ + int page_size = 0; + + switch ( (baser >> 8) & 3 ) + { + case 0: page_size = SZ_4K; break; + case 1: page_size = SZ_16K; break; + case 2: + case 3: page_size = SZ_64K; break; + } + + return page_size * ((baser & GENMASK_ULL(7, 0)) + 1); +} + +static int its_baser_nr_entries(uint64_t baser) +{ + int entry_size = ((baser & GENMASK_ULL(52, 48)) >> 48) + 1; + + return its_baser_table_size(baser) / entry_size; +} + +static int vgic_its_map_cmdbuf(struct virt_its *its) +{ + if ( !(its->cbaser & GITS_VALID_BIT) ) + return -EBUSY; + + return get_guest_pages(its->d, its->cbaser & GENMASK_ULL(51, 12), + (its->cbaser & 0xff) + 1); +} + +static void vgic_its_unmap_cmdbuf(struct virt_its *its) +{ + int nr_pages = (its->cbaser & 0xff) + 1; + + put_guest_pages(its->d, its->cbaser & GENMASK_ULL(51, 12), nr_pages); +} + +static int vgic_its_map_its_table(struct virt_its *its, uint64_t reg) +{ + unsigned int i, table_size = its_baser_table_size(reg); + paddr_t guest_addr = get_baser_phys_addr(reg); + + if ( !(reg & GITS_VALID_BIT) ) + return -EINVAL; + + get_guest_pages(its->d, guest_addr, table_size >> PAGE_SHIFT); + /* Map each page one by one to check and clear it. */ + for ( i = 0; i < table_size >> PAGE_SHIFT; i++ ) + { + void *ptr = map_one_guest_page(its->d, guest_addr + (i << PAGE_SHIFT)); + + if ( !ptr ) + return -EFAULT; + + memset(ptr, 0, table_size); + unmap_one_guest_page(ptr); + } + + return 0; +} + +static void vgic_its_unmap_its_table(struct domain *d, uint64_t reg) +{ + put_guest_pages(d, get_baser_phys_addr(reg), + its_baser_table_size(reg) >> PAGE_SHIFT); +} + +static bool vgic_v3_its_change_its_status(struct virt_its *its, bool status) +{ + bool ret = true; + + if ( !status ) + { + clear_bit(VIRT_ITS_ENABLED, &its->flags); + return false; + } + + if ( !vgic_its_map_cmdbuf(its) ) + set_bit(VIRT_ITS_CMDBUF_VALID, &its->flags); + else + { + clear_bit(VIRT_ITS_CMDBUF_VALID, &its->flags); + ret = false; + } + + if ( !vgic_its_map_its_table(its, its->baser_dev) ) + set_bit(VIRT_ITS_DEV_VALID, &its->flags); + else + { + clear_bit(VIRT_ITS_DEV_VALID, &its->flags); + ret = false; + } + + if ( !vgic_its_map_its_table(its, its->baser_coll) ) + set_bit(VIRT_ITS_COLL_VALID, &its->flags); + else + { + clear_bit(VIRT_ITS_COLL_VALID, &its->flags); + ret = false; + } + + if ( ret ) + set_bit(VIRT_ITS_ENABLED, &its->flags); + else + clear_bit(VIRT_ITS_ENABLED, &its->flags); + + return ret; +} + +static void sanitize_its_base_reg(uint64_t *reg) +{ + uint64_t r = *reg; + + /* Avoid outer shareable. */ + switch ( (r >> GITS_BASER_SHAREABILITY_SHIFT) & 0x03 ) + { + case GIC_BASER_OuterShareable: + r = r & ~GITS_BASER_SHAREABILITY_MASK; + r |= GIC_BASER_InnerShareable << GITS_BASER_SHAREABILITY_SHIFT; + break; + default: + break; + } + + /* Avoid any inner non-cacheable mapping. */ + switch ( (r >> GITS_BASER_INNER_CACHEABILITY_SHIFT) & 0x07 ) + { + case GIC_BASER_CACHE_nCnB: + case GIC_BASER_CACHE_nC: + r = r & ~GITS_BASER_INNER_CACHEABILITY_MASK; + r |= GIC_BASER_CACHE_RaWb << GITS_BASER_INNER_CACHEABILITY_SHIFT; + break; + default: + break; + } + + /* Only allow non-cacheable or same-as-inner. */ + switch ( (r >> GITS_BASER_OUTER_CACHEABILITY_SHIFT) & 0x07 ) + { + case GIC_BASER_CACHE_SameAsInner: + case GIC_BASER_CACHE_nC: + break; + default: + r = r & ~GITS_BASER_OUTER_CACHEABILITY_MASK; + r |= GIC_BASER_CACHE_nC << GITS_BASER_OUTER_CACHEABILITY_SHIFT; + break; + } + + *reg = r; +} + +static int vgic_v3_its_mmio_write(struct vcpu *v, mmio_info_t *info, + register_t r, void *priv) +{ + struct domain *d = v->domain; + struct virt_its *its = priv; + uint64_t reg; + uint32_t reg32, ctlr; + + switch ( info->gpa & 0xffff ) + { + case VREG32(GITS_CTLR): + if ( info->dabt.size != DABT_WORD ) goto bad_width; + + ctlr = its_is_enabled(its) ? GITS_CTLR_ENABLE : 0; + reg32 = ctlr; + vgic_reg32_update(®32, r, info); + + if ( ctlr ^ reg32 ) + vgic_v3_its_change_its_status(its, reg32 & GITS_CTLR_ENABLE); + return 1; + + case VREG32(GITS_IIDR): + goto write_ignore_32; + case VREG32(GITS_TYPER): + goto write_ignore_32; + case VREG64(GITS_CBASER): + if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width; + + /* Changing base registers with the ITS enabled is UNPREDICTABLE. */ + if ( its_is_enabled(its) ) + { + gdprintk(XENLOG_WARNING, "ITS: Domain %d tried to change CBASER with the ITS enabled.\n", d->domain_id); + return 1; + } + + reg = its->cbaser; + vgic_reg64_update(®, r, info); + sanitize_its_base_reg(®); + + vgic_its_unmap_cmdbuf(its); + its->cbaser = reg; + + return 1; + + case VREG64(GITS_CWRITER): + if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width; + reg = its->cwriter & 0xfffe0; + vgic_reg64_update(®, r, info); + its->cwriter = reg & 0xfffe0; + + if ( its_is_enabled(its) ) + vgic_its_handle_cmds(d, its, reg); + + return 1; + + case VREG64(GITS_CREADR): + goto write_ignore_64; + case VREG64(GITS_BASER0): + if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width; + + /* Changing base registers with the ITS enabled is UNPREDICTABLE. */ + if ( its_is_enabled(its) ) + { + gdprintk(XENLOG_WARNING, "ITS: Domain %d tried to change BASER with the ITS enabled.\n", + d->domain_id); + + return 1; + } + + reg = its->baser_dev; + vgic_reg64_update(®, r, info); + + reg &= ~GITS_BASER_RO_MASK; + reg |= (sizeof(uint64_t) - 1) << GITS_BASER_ENTRY_SIZE_SHIFT; + reg |= GITS_BASER_TYPE_DEVICE << GITS_BASER_TYPE_SHIFT; + sanitize_its_base_reg(®); + + /* Has the table address been changed or invalidated? */ + if ( !(reg & GITS_VALID_BIT) || + get_baser_phys_addr(reg) != get_baser_phys_addr(its->baser_dev) ) + { + vgic_its_unmap_its_table(its->d, its->baser_dev); + clear_bit(VIRT_ITS_DEV_VALID, &its->flags); + } + + if ( reg & GITS_VALID_BIT ) + its->max_devices = its_baser_nr_entries(reg); + else + its->max_devices = 0; + + its->baser_dev = reg; + return 1; + case VREG64(GITS_BASER1): + if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width; + + /* Changing base registers with the ITS enabled is UNPREDICTABLE. */ + if ( its_is_enabled(its) ) + { + gdprintk(XENLOG_INFO, "ITS: Domain %d tried to change BASER with the ITS enabled.\n", + d->domain_id); + return 1; + } + + reg = its->baser_coll; + vgic_reg64_update(®, r, info); + reg &= ~GITS_BASER_RO_MASK; + reg |= (sizeof(uint16_t) - 1) << GITS_BASER_ENTRY_SIZE_SHIFT; + reg |= GITS_BASER_TYPE_COLLECTION << GITS_BASER_TYPE_SHIFT; + sanitize_its_base_reg(®); + + if ( !(reg & GITS_VALID_BIT) || + get_baser_phys_addr(reg) != get_baser_phys_addr(its->baser_coll) ) + { + vgic_its_unmap_its_table(its->d, its->baser_coll); + clear_bit(VIRT_ITS_COLL_VALID, &its->flags); + } + + if ( reg & GITS_VALID_BIT ) + its->max_collections = its_baser_nr_entries(reg); + else + its->max_collections = 0; + its->baser_coll = reg; + return 1; + case VRANGE64(GITS_BASER2, GITS_BASER7): + goto write_ignore_64; + default: + gdprintk(XENLOG_G_WARNING, "ITS: unhandled ITS register 0x%lx\n", + info->gpa & 0xffff); + return 0; + } + + return 1; + +write_ignore_64: + if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width; + return 1; + +write_ignore_32: + if ( info->dabt.size != DABT_WORD ) goto bad_width; + return 1; + +bad_width: + printk(XENLOG_G_ERR "%pv vGICR: bad read width %d r%d offset %#08lx\n", + v, info->dabt.size, info->dabt.reg, info->gpa & 0xffff); + + domain_crash_synchronous(); + + return 0; +} + +static const struct mmio_handler_ops vgic_its_mmio_handler = { + .read = vgic_v3_its_mmio_read, + .write = vgic_v3_its_mmio_write, +}; + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c index 2c6b317..ebcfc16 100644 --- a/xen/arch/arm/vgic-v3.c +++ b/xen/arch/arm/vgic-v3.c @@ -158,15 +158,6 @@ static void vgic_store_irouter(struct domain *d, struct vgic_irq_rank *rank, write_atomic(&rank->vcpu[offset], new_vcpu->vcpu_id); } -static inline bool vgic_reg64_check_access(struct hsr_dabt dabt) -{ - /* - * 64 bits registers can be accessible using 32-bit and 64-bit unless - * stated otherwise (See 8.1.3 ARM IHI 0069A). - */ - return ( dabt.size == DABT_DOUBLE_WORD || dabt.size == DABT_WORD ); -} - static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info, uint32_t gicr_reg, register_t *r) diff --git a/xen/include/asm-arm/gic_v3_defs.h b/xen/include/asm-arm/gic_v3_defs.h index b01b6ed..8999937 100644 --- a/xen/include/asm-arm/gic_v3_defs.h +++ b/xen/include/asm-arm/gic_v3_defs.h @@ -155,6 +155,16 @@ #define LPI_PROP_RES1 (1 << 1) #define LPI_PROP_ENABLED (1 << 0) +/* + * PIDR2: Only bits[7:4] are not implementation defined. We are + * emulating a GICv3 ([7:4] = 0x3). + * + * We don't emulate a specific registers scheme so implement the others + * bits as RES0 as recommended by the spec (see 8.1.13 in ARM IHI 0069A). + */ +#define GICV3_GICD_PIDR2 0x30 +#define GICV3_GICR_PIDR2 GICV3_GICD_PIDR2 + #define GICH_VMCR_EOI (1 << 9) #define GICH_VMCR_VENG1 (1 << 1) @@ -198,6 +208,15 @@ struct rdist_region { bool single_rdist; }; +/* + * 64 bits registers can be accessible using 32-bit and 64-bit unless + * stated otherwise (See 8.1.3 ARM IHI 0069A). + */ +static inline bool vgic_reg64_check_access(struct hsr_dabt dabt) +{ + return ( dabt.size == DABT_DOUBLE_WORD || dabt.size == DABT_WORD ); +} + #endif /* __ASM_ARM_GIC_V3_DEFS_H__ */ /* diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h index d8165c4..b2a9586 100644 --- a/xen/include/asm-arm/gic_v3_its.h +++ b/xen/include/asm-arm/gic_v3_its.h @@ -35,6 +35,7 @@ #define GITS_BASER5 0x128 #define GITS_BASER6 0x130 #define GITS_BASER7 0x138 +#define GITS_PIDR2 GICR_PIDR2 /* Register bits */ #define GITS_VALID_BIT BIT_ULL(63) @@ -56,6 +57,7 @@ #define GITS_TYPER_ITT_SIZE_MASK (0xfUL << GITS_TYPER_ITT_SIZE_SHIFT) #define GITS_TYPER_ITT_SIZE(r) ((((r) & GITS_TYPER_ITT_SIZE_MASK) >> \ GITS_TYPER_ITT_SIZE_SHIFT) + 1) +#define GITS_TYPER_PHYSICAL (1U << 0) #define GITS_IIDR_VALUE 0x34c
Create a new file to hold the emulation code for the ITS widget. For now we emulate the memory mapped ITS registers and provide a stub to introduce the ITS command handling framework (but without actually emulating any commands at this time). Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- xen/arch/arm/Makefile | 1 + xen/arch/arm/vgic-v3-its.c | 547 ++++++++++++++++++++++++++++++++++++++ xen/arch/arm/vgic-v3.c | 9 - xen/include/asm-arm/gic_v3_defs.h | 19 ++ xen/include/asm-arm/gic_v3_its.h | 2 + 5 files changed, 569 insertions(+), 9 deletions(-) create mode 100644 xen/arch/arm/vgic-v3-its.c