| Message ID | 1432893209-27313-13-git-send-email-andre.przywara@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello Andre, On 05/29/2015 11:53 AM, Andre Przywara wrote: > When userland wants to inject a MSI into the guest, we have to use > our data structures to find the LPI number and the VCPU to receivce receive > the interrupt. > Use the wrapper functions to iterate the linked lists and find the > proper Interrupt Translation Table Entry. Then set the pending bit > in this ITTE to be later picked up by the LR handling code. Kick > the VCPU which is meant to handle this interrupt. > We provide a VGIC emulation model specific routine for the actual > MSI injection. The wrapper functions return an error for models not > (yet) implementing MSIs (like the GICv2 emulation). > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > include/kvm/arm_vgic.h | 1 + > virt/kvm/arm/its-emul.c | 49 +++++++++++++++++++++++++++++++++++++++++++++ > virt/kvm/arm/its-emul.h | 2 ++ > virt/kvm/arm/vgic-v3-emul.c | 1 + > 4 files changed, 53 insertions(+) > > diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h > index de19c34..6bb138d 100644 > --- a/include/kvm/arm_vgic.h > +++ b/include/kvm/arm_vgic.h > @@ -149,6 +149,7 @@ struct vgic_vm_ops { > int (*map_resources)(struct kvm *, const struct vgic_params *); > bool (*queue_lpis)(struct kvm_vcpu *); > void (*unqueue_lpi)(struct kvm_vcpu *, int irq); > + int (*inject_msi)(struct kvm *, struct kvm_msi *); > }; > > struct vgic_io_device { > diff --git a/virt/kvm/arm/its-emul.c b/virt/kvm/arm/its-emul.c > index 574cf05..35e886c 100644 > --- a/virt/kvm/arm/its-emul.c > +++ b/virt/kvm/arm/its-emul.c > @@ -340,6 +340,55 @@ static bool handle_mmio_gits_idregs(struct kvm_vcpu *vcpu, > } > > /* > + * Translates an incoming MSI request into the redistributor (=VCPU) and > + * the associated LPI number. Sets the LPI pending bit and also marks the > + * VCPU as having a pending interrupt. > + */ > +int vits_inject_msi(struct kvm *kvm, struct kvm_msi *msi) > +{ > + struct vgic_dist *dist = &kvm->arch.vgic; > + struct vgic_its *its = &dist->its; > + struct its_itte *itte; > + int cpuid; > + bool inject = false; > + int ret = 0; > + > + if (!vgic_has_its(kvm)) > + return -ENODEV; > + > + if (!(msi->flags & KVM_MSI_VALID_DEVID)) > + return -EINVAL; > + > + spin_lock(&its->lock); > + > + if (!its->enabled || !dist->lpis_enabled) { > + ret = -EAGAIN; > + goto out_unlock; > + } > + > + itte = find_itte(kvm, msi->devid, msi->data); > + /* Triggering an unmapped IRQ gets silently dropped. */ > + if (!itte || !itte->collection) > + goto out_unlock; > + > + cpuid = itte->collection->target_addr; > + set_bit(cpuid, itte->pending); so now the internal state is different from the pending state in ext memory. I don't really understand where the ext mem is used? > + inject = itte->enabled; > + > +out_unlock: > + spin_unlock(&its->lock); > + > + if (inject) { > + spin_lock(&dist->lock); > + set_bit(cpuid, dist->irq_pending_on_cpu); isn't it atomic op? Best Regards Eric > + spin_unlock(&dist->lock); > + kvm_vcpu_kick(kvm_get_vcpu(kvm, cpuid)); > + } > + > + return ret; > +} > + > +/* > * Find all enabled and pending LPIs and queue them into the list > * registers. > * The dist lock is held by the caller. > diff --git a/virt/kvm/arm/its-emul.h b/virt/kvm/arm/its-emul.h > index 6152d04..cac1406 100644 > --- a/virt/kvm/arm/its-emul.h > +++ b/virt/kvm/arm/its-emul.h > @@ -33,6 +33,8 @@ void vgic_enable_lpis(struct kvm_vcpu *vcpu); > int vits_init(struct kvm *kvm); > void vits_destroy(struct kvm *kvm); > > +int vits_inject_msi(struct kvm *kvm, struct kvm_msi *msi); > + > bool vits_queue_lpis(struct kvm_vcpu *vcpu); > void vits_unqueue_lpi(struct kvm_vcpu *vcpu, int irq); > > diff --git a/virt/kvm/arm/vgic-v3-emul.c b/virt/kvm/arm/vgic-v3-emul.c > index 66640c2fa..4513551 100644 > --- a/virt/kvm/arm/vgic-v3-emul.c > +++ b/virt/kvm/arm/vgic-v3-emul.c > @@ -901,6 +901,7 @@ void vgic_v3_init_emulation(struct kvm *kvm) > dist->vm_ops.init_model = vgic_v3_init_model; > dist->vm_ops.destroy_model = vgic_v3_destroy_model; > dist->vm_ops.map_resources = vgic_v3_map_resources; > + dist->vm_ops.inject_msi = vits_inject_msi; > dist->vm_ops.queue_lpis = vits_queue_lpis; > dist->vm_ops.unqueue_lpi = vits_unqueue_lpi; > > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Eric, .... >> diff --git a/virt/kvm/arm/its-emul.c b/virt/kvm/arm/its-emul.c >> index 574cf05..35e886c 100644 >> --- a/virt/kvm/arm/its-emul.c >> +++ b/virt/kvm/arm/its-emul.c >> @@ -340,6 +340,55 @@ static bool handle_mmio_gits_idregs(struct kvm_vcpu *vcpu, >> } >> >> /* >> + * Translates an incoming MSI request into the redistributor (=VCPU) and >> + * the associated LPI number. Sets the LPI pending bit and also marks the >> + * VCPU as having a pending interrupt. >> + */ >> +int vits_inject_msi(struct kvm *kvm, struct kvm_msi *msi) >> +{ >> + struct vgic_dist *dist = &kvm->arch.vgic; >> + struct vgic_its *its = &dist->its; >> + struct its_itte *itte; >> + int cpuid; >> + bool inject = false; >> + int ret = 0; >> + >> + if (!vgic_has_its(kvm)) >> + return -ENODEV; >> + >> + if (!(msi->flags & KVM_MSI_VALID_DEVID)) >> + return -EINVAL; >> + >> + spin_lock(&its->lock); >> + >> + if (!its->enabled || !dist->lpis_enabled) { >> + ret = -EAGAIN; >> + goto out_unlock; >> + } >> + >> + itte = find_itte(kvm, msi->devid, msi->data); >> + /* Triggering an unmapped IRQ gets silently dropped. */ >> + if (!itte || !itte->collection) >> + goto out_unlock; >> + >> + cpuid = itte->collection->target_addr; >> + set_bit(cpuid, itte->pending); > so now the internal state is different from the pending state in ext > memory. I don't really understand where the ext mem is used? This is expected, as the ITS is allowed to cache the state. In a hardware ITS implementation the external memory is used to provide actual storage for the ITS, something we do not need for the emulation, as we have cheaper (host) memory for that. The only thing we have to model though is that the guest may use the external storage to take a snapshot of the current state, but it may only do so after having flushed the ITS "cache", which means we synchronize our internal data structures to that "external" memory. >> + inject = itte->enabled; >> + >> +out_unlock: >> + spin_unlock(&its->lock); >> + >> + if (inject) { >> + spin_lock(&dist->lock); >> + set_bit(cpuid, dist->irq_pending_on_cpu); > isn't it atomic op? It is, but that's not what the lock protects. It's there to avoid stepping on someone else's toes, who might deal with the ITS data structures at the same time and would not expect a value to change in the middle (think about code iterating over dist->irq_pending_on_cpu). Cheers, Andre. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 06, 2015 at 05:46:37PM +0100, Andre Przywara wrote: [...] > > >> + inject = itte->enabled; > >> + > >> +out_unlock: > >> + spin_unlock(&its->lock); > >> + > >> + if (inject) { > >> + spin_lock(&dist->lock); > >> + set_bit(cpuid, dist->irq_pending_on_cpu); > > isn't it atomic op? > > It is, but that's not what the lock protects. It's there to avoid > stepping on someone else's toes, who might deal with the ITS data > structures at the same time and would not expect a value to change in > the middle (think about code iterating over dist->irq_pending_on_cpu). > then should it be __set_bit ? -Christoffer -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h index de19c34..6bb138d 100644 --- a/include/kvm/arm_vgic.h +++ b/include/kvm/arm_vgic.h @@ -149,6 +149,7 @@ struct vgic_vm_ops { int (*map_resources)(struct kvm *, const struct vgic_params *); bool (*queue_lpis)(struct kvm_vcpu *); void (*unqueue_lpi)(struct kvm_vcpu *, int irq); + int (*inject_msi)(struct kvm *, struct kvm_msi *); }; struct vgic_io_device { diff --git a/virt/kvm/arm/its-emul.c b/virt/kvm/arm/its-emul.c index 574cf05..35e886c 100644 --- a/virt/kvm/arm/its-emul.c +++ b/virt/kvm/arm/its-emul.c @@ -340,6 +340,55 @@ static bool handle_mmio_gits_idregs(struct kvm_vcpu *vcpu, } /* + * Translates an incoming MSI request into the redistributor (=VCPU) and + * the associated LPI number. Sets the LPI pending bit and also marks the + * VCPU as having a pending interrupt. + */ +int vits_inject_msi(struct kvm *kvm, struct kvm_msi *msi) +{ + struct vgic_dist *dist = &kvm->arch.vgic; + struct vgic_its *its = &dist->its; + struct its_itte *itte; + int cpuid; + bool inject = false; + int ret = 0; + + if (!vgic_has_its(kvm)) + return -ENODEV; + + if (!(msi->flags & KVM_MSI_VALID_DEVID)) + return -EINVAL; + + spin_lock(&its->lock); + + if (!its->enabled || !dist->lpis_enabled) { + ret = -EAGAIN; + goto out_unlock; + } + + itte = find_itte(kvm, msi->devid, msi->data); + /* Triggering an unmapped IRQ gets silently dropped. */ + if (!itte || !itte->collection) + goto out_unlock; + + cpuid = itte->collection->target_addr; + set_bit(cpuid, itte->pending); + inject = itte->enabled; + +out_unlock: + spin_unlock(&its->lock); + + if (inject) { + spin_lock(&dist->lock); + set_bit(cpuid, dist->irq_pending_on_cpu); + spin_unlock(&dist->lock); + kvm_vcpu_kick(kvm_get_vcpu(kvm, cpuid)); + } + + return ret; +} + +/* * Find all enabled and pending LPIs and queue them into the list * registers. * The dist lock is held by the caller. diff --git a/virt/kvm/arm/its-emul.h b/virt/kvm/arm/its-emul.h index 6152d04..cac1406 100644 --- a/virt/kvm/arm/its-emul.h +++ b/virt/kvm/arm/its-emul.h @@ -33,6 +33,8 @@ void vgic_enable_lpis(struct kvm_vcpu *vcpu); int vits_init(struct kvm *kvm); void vits_destroy(struct kvm *kvm); +int vits_inject_msi(struct kvm *kvm, struct kvm_msi *msi); + bool vits_queue_lpis(struct kvm_vcpu *vcpu); void vits_unqueue_lpi(struct kvm_vcpu *vcpu, int irq); diff --git a/virt/kvm/arm/vgic-v3-emul.c b/virt/kvm/arm/vgic-v3-emul.c index 66640c2fa..4513551 100644 --- a/virt/kvm/arm/vgic-v3-emul.c +++ b/virt/kvm/arm/vgic-v3-emul.c @@ -901,6 +901,7 @@ void vgic_v3_init_emulation(struct kvm *kvm) dist->vm_ops.init_model = vgic_v3_init_model; dist->vm_ops.destroy_model = vgic_v3_destroy_model; dist->vm_ops.map_resources = vgic_v3_map_resources; + dist->vm_ops.inject_msi = vits_inject_msi; dist->vm_ops.queue_lpis = vits_queue_lpis; dist->vm_ops.unqueue_lpi = vits_unqueue_lpi;
When userland wants to inject a MSI into the guest, we have to use our data structures to find the LPI number and the VCPU to receivce the interrupt. Use the wrapper functions to iterate the linked lists and find the proper Interrupt Translation Table Entry. Then set the pending bit in this ITTE to be later picked up by the LR handling code. Kick the VCPU which is meant to handle this interrupt. We provide a VGIC emulation model specific routine for the actual MSI injection. The wrapper functions return an error for models not (yet) implementing MSIs (like the GICv2 emulation). Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- include/kvm/arm_vgic.h | 1 + virt/kvm/arm/its-emul.c | 49 +++++++++++++++++++++++++++++++++++++++++++++ virt/kvm/arm/its-emul.h | 2 ++ virt/kvm/arm/vgic-v3-emul.c | 1 + 4 files changed, 53 insertions(+)