On 3/25/16, 7:14 PM, "kvmarm-bounces@xxxxxxxxxxxxxxxxxxxxx on behalf of Andre Przywara" <kvmarm-bounces@xxxxxxxxxxxxxxxxxxxxx on behalf of andre.przywara@xxxxxxx> wrote: >Add emulation for some basic MMIO registers used in the ITS emulation. >This includes: >- GITS_{CTLR,TYPER,IIDR} >- ID registers >- GITS_{CBASER,CREADR,CWRITER} > those implement the ITS command buffer handling > >Most of the handlers are pretty straight forward, but CWRITER goes >some extra miles to allow fine grained locking. The idea here >is to let only the first instance iterate through the command ring >buffer, CWRITER accesses on other VCPUs meanwhile will be picked up >by that first instance and handled as well. The ITS lock is thus only >hold for very small periods of time and is dropped before the actual >command handler is called. > >Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx> >--- > include/kvm/vgic/vgic.h | 3 + > include/linux/irqchip/arm-gic-v3.h | 8 ++ > virt/kvm/arm/vgic/its-emul.c | 272 ++++++++++++++++++++++++++++++++++++- > virt/kvm/arm/vgic/vgic.h | 6 + > virt/kvm/arm/vgic/vgic_init.c | 2 + > 5 files changed, 284 insertions(+), 7 deletions(-) > >diff --git a/include/kvm/vgic/vgic.h b/include/kvm/vgic/vgic.h >index c79bed5..bafea11 100644 >--- a/include/kvm/vgic/vgic.h >+++ b/include/kvm/vgic/vgic.h >@@ -115,6 +115,9 @@ struct vgic_io_device { > struct vgic_its { > bool enabled; > spinlock_t lock; >+ u64 cbaser; >+ int creadr; >+ int cwriter; > }; > > struct vgic_dist { >diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h >index a813c3e..7011b98 100644 >--- a/include/linux/irqchip/arm-gic-v3.h >+++ b/include/linux/irqchip/arm-gic-v3.h >@@ -179,15 +179,23 @@ > #define GITS_BASER 0x0100 > #define GITS_IDREGS_BASE 0xffd0 > #define GITS_PIDR2 GICR_PIDR2 >+#define GITS_PIDR4 0xffd0 >+#define GITS_CIDR0 0xfff0 >+#define GITS_CIDR1 0xfff4 >+#define GITS_CIDR2 0xfff8 >+#define GITS_CIDR3 0xfffc > > #define GITS_TRANSLATER 0x10040 > > #define GITS_CTLR_ENABLE (1U << 0) > #define GITS_CTLR_QUIESCENT (1U << 31) > >+#define GITS_TYPER_PLPIS (1UL << 0) >+#define GITS_TYPER_IDBITS_SHIFT 8 > #define GITS_TYPER_DEVBITS_SHIFT 13 > #define GITS_TYPER_DEVBITS(r) ((((r) >> GITS_TYPER_DEVBITS_SHIFT) & 0x1f) + 1) > #define GITS_TYPER_PTA (1UL << 19) >+#define GITS_TYPER_HWCOLLCNT_SHIFT 24 > > #define GITS_CBASER_VALID (1UL << 63) > #define GITS_CBASER_nCnB (0UL << 59) >diff --git a/virt/kvm/arm/vgic/its-emul.c b/virt/kvm/arm/vgic/its-emul.c >index 49dd5e4..de8d360 100644 >--- a/virt/kvm/arm/vgic/its-emul.c >+++ b/virt/kvm/arm/vgic/its-emul.c >@@ -31,23 +31,263 @@ > #include "vgic.h" > #include "vgic_mmio.h" > >+#define BASER_BASE_ADDRESS(x) ((x) & 0xfffffffff000ULL) >+ >+static int vgic_mmio_read_its_ctlr(struct kvm_vcpu *vcpu, >+ struct kvm_io_device *this, >+ gpa_t addr, int len, void *val) >+{ >+ struct vgic_its *its = &vcpu->kvm->arch.vgic.its; >+ u32 reg; >+ >+ reg = GITS_CTLR_QUIESCENT; >+ if (its->enabled) >+ reg |= GITS_CTLR_ENABLE; >+ >+ write_mask32(reg, addr & 3, len, val); >+ >+ return 0; >+} >+ >+static int vgic_mmio_write_its_ctlr(struct kvm_vcpu *vcpu, >+ struct kvm_io_device *this, >+ gpa_t addr, int len, const void *val) >+{ >+ struct vgic_its *its = &vcpu->kvm->arch.vgic.its; >+ struct vgic_io_device *iodev = container_of(this, >+ struct vgic_io_device, dev); >+ >+ if (addr - iodev->base_addr == 0) >+ its->enabled = !!(*(u8*)val & GITS_CTLR_ENABLE); >+ >+ return 0; >+} >+ >+static int vgic_mmio_read_its_typer(struct kvm_vcpu *vcpu, >+ struct kvm_io_device *this, >+ gpa_t addr, int len, void *val) >+{ >+ u64 reg = GITS_TYPER_PLPIS; >+ >+ /* >+ * We use linear CPU numbers for redistributor addressing, >+ * so GITS_TYPER.PTA is 0. >+ * To avoid memory waste on the guest side, we keep the >+ * number of IDBits and DevBits low for the time being. >+ * This could later be made configurable by userland. >+ * Since we have all collections in linked list, we claim >+ * that we can hold all of the collection tables in our >+ * own memory and that the ITT entry size is 1 byte (the >+ * smallest possible one). >+ */ How are you planning to handle device ids with VFIO? Will there be a mapping between virtual and physical? Or will it entirely based on virtual and not planning to send the commands to corresponding physical ITS. May be no need. >+ reg |= 0xff << GITS_TYPER_HWCOLLCNT_SHIFT; >+ reg |= 0x0f << GITS_TYPER_DEVBITS_SHIFT; >+ reg |= 0x0f << GITS_TYPER_IDBITS_SHIFT; >+ >+ write_mask64(reg, addr & 7, len, val); >+ >+ return 0; >+} >+ >+static int vgic_mmio_read_its_iidr(struct kvm_vcpu *vcpu, >+ struct kvm_io_device *this, >+ gpa_t addr, int len, void *val) >+{ >+ u32 reg = (PRODUCT_ID_KVM << 24) | (IMPLEMENTER_ARM << 0); >+ Hmm, no other implementor planning to support system emulation. >+ write_mask32(reg, addr & 3, len, val); >+ >+ return 0; >+} >+ >+static int vgic_mmio_read_its_idregs(struct kvm_vcpu *vcpu, >+ struct kvm_io_device *this, >+ gpa_t addr, int len, void *val) >+{ >+ struct vgic_io_device *iodev = container_of(this, >+ struct vgic_io_device, dev); >+ u32 reg = 0; >+ int idreg = (addr & ~3) - iodev->base_addr + GITS_IDREGS_BASE; >+ >+ switch (idreg) { >+ case GITS_PIDR2: >+ reg = GIC_PIDR2_ARCH_GICv3; >+ break; >+ case GITS_PIDR4: >+ /* This is a 64K software visible page */ >+ reg = 0x40; >+ break; >+ /* Those are the ID registers for (any) GIC. */ >+ case GITS_CIDR0: >+ reg = 0x0d; >+ break; >+ case GITS_CIDR1: >+ reg = 0xf0; >+ break; >+ case GITS_CIDR2: >+ reg = 0x05; >+ break; >+ case GITS_CIDR3: >+ reg = 0xb1; >+ break; >+ } >+ >+ write_mask32(reg, addr & 3, len, val); >+ >+ return 0; >+} >+ >+/* >+ * This function is called with both the ITS and the distributor lock dropped, >+ * so the actual command handlers must take the respective locks when needed. >+ */ >+static int vits_handle_command(struct kvm_vcpu *vcpu, u64 *its_cmd) >+{ >+ return -ENODEV; >+} >+ >+static int vgic_mmio_read_its_cbaser(struct kvm_vcpu *vcpu, >+ struct kvm_io_device *this, >+ gpa_t addr, int len, void *val) >+{ >+ struct vgic_its *its = &vcpu->kvm->arch.vgic.its; >+ >+ write_mask64(its->cbaser, addr & 7, len, val); >+ >+ return 0; >+} >+ >+static int vgic_mmio_write_its_cbaser(struct kvm_vcpu *vcpu, >+ struct kvm_io_device *this, >+ gpa_t addr, int len, const void *val) >+{ >+ struct vgic_its *its = &vcpu->kvm->arch.vgic.its; >+ >+ if (its->enabled) >+ return 0; >+ >+ its->cbaser = mask64(its->cbaser, addr & 7, len, val); >+ its->creadr = 0; >+ >+ return 0; >+} >+ >+static int its_cmd_buffer_size(struct kvm *kvm) >+{ >+ struct vgic_its *its = &kvm->arch.vgic.its; >+ >+ return ((its->cbaser & 0xff) + 1) << 12; >+} >+ >+static gpa_t its_cmd_buffer_base(struct kvm *kvm) >+{ >+ struct vgic_its *its = &kvm->arch.vgic.its; >+ >+ return BASER_BASE_ADDRESS(its->cbaser); >+} >+ >+/* >+ * By writing to CWRITER the guest announces new commands to be processed. >+ * Since we cannot read from guest memory inside the ITS spinlock, we >+ * iterate over the command buffer (with the lock dropped) until the read >+ * pointer matches the write pointer. Other VCPUs writing this register in the >+ * meantime will just update the write pointer, leaving the command >+ * processing to the first instance of the function. >+ */ >+static int vgic_mmio_write_its_cwriter(struct kvm_vcpu *vcpu, >+ struct kvm_io_device *this, >+ gpa_t addr, int len, const void *val) >+{ >+ struct vgic_dist *dist = &vcpu->kvm->arch.vgic; >+ struct vgic_its *its = &dist->its; >+ gpa_t cbaser = its_cmd_buffer_base(vcpu->kvm); >+ u64 cmd_buf[4]; >+ u32 reg; >+ bool finished; >+ >+ reg = mask64(its->cwriter & 0xfffe0, addr & 7, len, val); >+ reg &= 0xfffe0; >+ if (reg > its_cmd_buffer_size(vcpu->kvm)) >+ return 0; >+ >+ spin_lock(&its->lock); >+ >+ /* >+ * If there is still another VCPU handling commands, let this >+ * one pick up the new CWRITER and process "our" new commands as well. >+ */ >+ finished = (its->cwriter != its->creadr); >+ its->cwriter = reg; >+ >+ spin_unlock(&its->lock); >+ >+ while (!finished) { >+ int ret = kvm_read_guest(vcpu->kvm, cbaser + its->creadr, >+ cmd_buf, 32); >+ if (ret) { >+ /* >+ * Gah, we are screwed. Reset CWRITER to that command >+ * that we have finished processing and return. >+ */ >+ spin_lock(&its->lock); >+ its->cwriter = its->creadr; Is this correct? >+ spin_unlock(&its->lock); >+ break; >+ } >+ vits_handle_command(vcpu, cmd_buf); >+ >+ spin_lock(&its->lock); >+ its->creadr += 32; >+ if (its->creadr == its_cmd_buffer_size(vcpu->kvm)) >+ its->creadr = 0; >+ finished = (its->creadr == its->cwriter); >+ spin_unlock(&its->lock); >+ } >+ >+ return 0; >+} >+ >+static int vgic_mmio_read_its_cwriter(struct kvm_vcpu *vcpu, >+ struct kvm_io_device *this, >+ gpa_t addr, int len, void *val) >+{ >+ struct vgic_its *its = &vcpu->kvm->arch.vgic.its; >+ u64 reg = its->cwriter & 0xfffe0; >+ >+ write_mask64(reg, addr & 7, len, val); >+ >+ return 0; >+} >+ >+static int vgic_mmio_read_its_creadr(struct kvm_vcpu *vcpu, >+ struct kvm_io_device *this, >+ gpa_t addr, int len, void *val) >+{ >+ struct vgic_its *its = &vcpu->kvm->arch.vgic.its; >+ u64 reg = its->creadr & 0xfffe0; >+ >+ write_mask64(reg, addr & 7, len, val); >+ >+ return 0; >+} >+ > struct vgic_register_region its_registers[] = { > REGISTER_DESC_WITH_LENGTH(GITS_CTLR, >- vgic_mmio_read_raz, vgic_mmio_write_wi, 4), >+ vgic_mmio_read_its_ctlr, vgic_mmio_write_its_ctlr, 4), > REGISTER_DESC_WITH_LENGTH(GITS_IIDR, >- vgic_mmio_read_raz, vgic_mmio_write_wi, 4), >+ vgic_mmio_read_its_iidr, vgic_mmio_write_wi, 4), > REGISTER_DESC_WITH_LENGTH(GITS_TYPER, >- vgic_mmio_read_raz, vgic_mmio_write_wi, 4), >+ vgic_mmio_read_its_typer, vgic_mmio_write_wi, 4), > REGISTER_DESC_WITH_LENGTH(GITS_CBASER, >- vgic_mmio_read_raz, vgic_mmio_write_wi, 8), >+ vgic_mmio_read_its_cbaser, vgic_mmio_write_its_cbaser, 8), > REGISTER_DESC_WITH_LENGTH(GITS_CWRITER, >- vgic_mmio_read_raz, vgic_mmio_write_wi, 8), >+ vgic_mmio_read_its_cwriter, vgic_mmio_write_its_cwriter, 8), > REGISTER_DESC_WITH_LENGTH(GITS_CREADR, >- vgic_mmio_read_raz, vgic_mmio_write_wi, 8), >+ vgic_mmio_read_its_creadr, vgic_mmio_write_wi, 8), > REGISTER_DESC_WITH_LENGTH(GITS_BASER, > vgic_mmio_read_raz, vgic_mmio_write_wi, 0x40), > REGISTER_DESC_WITH_LENGTH(GITS_IDREGS_BASE, >- vgic_mmio_read_raz, vgic_mmio_write_wi, 0x30), >+ vgic_mmio_read_its_idregs, vgic_mmio_write_wi, 0x30), > }; > > /* This is called on setting the LPI enable bit in the redistributor. */ >@@ -59,9 +299,14 @@ int vits_init(struct kvm *kvm) > { > struct vgic_dist *dist = &kvm->arch.vgic; > struct vgic_its *its = &dist->its; >+ int nr_vcpus = atomic_read(&kvm->online_vcpus); > struct vgic_io_device *regions; > int ret, i; > >+ dist->pendbaser = kcalloc(nr_vcpus, sizeof(u64), GFP_KERNEL); >+ if (!dist->pendbaser) >+ return -ENOMEM; >+ > spin_lock_init(&its->lock); > > regions = kmalloc_array(ARRAY_SIZE(its_registers), >@@ -82,3 +327,16 @@ int vits_init(struct kvm *kvm) > > return -ENXIO; > } >+ >+void vits_destroy(struct kvm *kvm) >+{ >+ struct vgic_dist *dist = &kvm->arch.vgic; >+ struct vgic_its *its = &dist->its; >+ >+ if (!vgic_has_its(kvm)) >+ return; >+ >+ kfree(dist->pendbaser); >+ >+ its->enabled = false; >+} >diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h >index 4e7dcb8..08f97d1 100644 >--- a/virt/kvm/arm/vgic/vgic.h >+++ b/virt/kvm/arm/vgic/vgic.h >@@ -63,6 +63,7 @@ int vgic_register_redist_regions(struct kvm *kvm, gpa_t dist_base_address); > > int vits_init(struct kvm *kvm); > void vgic_enable_lpis(struct kvm_vcpu *vcpu); >+void vits_destroy(struct kvm *kvm); > #else > static inline void vgic_v3_irq_change_affinity(struct kvm *kvm, u32 intid, > u64 mpidr) >@@ -137,6 +138,11 @@ static inline void vgic_enable_lpis(struct kvm_vcpu *vcpu) > { > return; > } >+ >+static inline void vits_destroy(struct kvm *kvm) >+{ >+ return; >+} > #endif > > void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); >diff --git a/virt/kvm/arm/vgic/vgic_init.c b/virt/kvm/arm/vgic/vgic_init.c >index dcfb93d..e4459e3 100644 >--- a/virt/kvm/arm/vgic/vgic_init.c >+++ b/virt/kvm/arm/vgic/vgic_init.c >@@ -298,6 +298,8 @@ void kvm_vgic_destroy(struct kvm *kvm) > > kvm_vgic_dist_destroy(kvm); > >+ vits_destroy(kvm); >+ > kvm_for_each_vcpu(i, vcpu, kvm) > kvm_vgic_vcpu_destroy(vcpu); > } >-- >2.7.3 > >_______________________________________________ >kvmarm mailing list >kvmarm@xxxxxxxxxxxxxxxxxxxxx >https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ��.n��������+%������w��{.n�����o�^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�