| Message ID | 06930ddb-6c40-ff74-0062-cec236ce2455@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Aug 30, 2017 at 11:28:08AM +0100, Marc Zyngier wrote: > On 26/08/17 20:48, Christoffer Dall wrote: > > On Mon, Jul 31, 2017 at 06:26:19PM +0100, Marc Zyngier wrote: > >> Let's use the irq bypass mechanism introduced for platform device > >> interrupts to intercept the virtual PCIe endpoint configuration > >> and establish our LPI->VLPI mapping. > >> > >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > >> --- > >> include/kvm/arm_vgic.h | 8 ++++ > >> virt/kvm/arm/arm.c | 27 ++++++++---- > >> virt/kvm/arm/vgic/vgic-v4.c | 103 ++++++++++++++++++++++++++++++++++++++++++++ > >> 3 files changed, 130 insertions(+), 8 deletions(-) > >> > >> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h > >> index 359eeffe9857..050f78d4fb42 100644 > >> --- a/include/kvm/arm_vgic.h > >> +++ b/include/kvm/arm_vgic.h > >> @@ -367,4 +367,12 @@ int kvm_vgic_set_forwarding(struct kvm *kvm, unsigned int host_irq, > >> void kvm_vgic_unset_forwarding(struct kvm *kvm, unsigned int host_irq, > >> unsigned int vintid); > >> > >> +struct kvm_kernel_irq_routing_entry; > >> + > >> +int kvm_vgic_v4_set_forwarding(struct kvm *kvm, int irq, > >> + struct kvm_kernel_irq_routing_entry *irq_entry); > >> + > >> +int kvm_vgic_v4_unset_forwarding(struct kvm *kvm, int irq, > >> + struct kvm_kernel_irq_routing_entry *irq_entry); > >> + > >> #endif /* __KVM_ARM_VGIC_H */ > >> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c > >> index ebab6c29e3be..6803ea27c47d 100644 > >> --- a/virt/kvm/arm/arm.c > >> +++ b/virt/kvm/arm/arm.c > >> @@ -1457,11 +1457,16 @@ int kvm_arch_irq_bypass_add_producer(struct irq_bypass_consumer *cons, > >> struct kvm_kernel_irqfd *irqfd = > >> container_of(cons, struct kvm_kernel_irqfd, consumer); > >> > >> - if (prod->type != IRQ_BYPASS_VFIO_PLATFORM) > >> + switch (prod->type) { > >> + case IRQ_BYPASS_VFIO_PLATFORM: > >> + return kvm_vgic_set_forwarding(irqfd->kvm, prod->irq, > >> + irqfd->gsi + VGIC_NR_PRIVATE_IRQS); > >> + case IRQ_BYPASS_VFIO_PCI_MSI: > >> + return kvm_vgic_v4_set_forwarding(irqfd->kvm, prod->irq, > >> + &irqfd->irq_entry); > >> + default: > >> return 0; > >> - > >> - return kvm_vgic_set_forwarding(irqfd->kvm, prod->irq, > >> - irqfd->gsi + VGIC_NR_PRIVATE_IRQS); > >> + } > >> } > >> void kvm_arch_irq_bypass_del_producer(struct irq_bypass_consumer *cons, > >> struct irq_bypass_producer *prod) > >> @@ -1469,11 +1474,17 @@ void kvm_arch_irq_bypass_del_producer(struct irq_bypass_consumer *cons, > >> struct kvm_kernel_irqfd *irqfd = > >> container_of(cons, struct kvm_kernel_irqfd, consumer); > >> > >> - if (prod->type != IRQ_BYPASS_VFIO_PLATFORM) > >> - return; > >> + switch (prod->type) { > >> + case IRQ_BYPASS_VFIO_PLATFORM: > >> + kvm_vgic_unset_forwarding(irqfd->kvm, prod->irq, > >> + irqfd->gsi + VGIC_NR_PRIVATE_IRQS); > >> + break; > >> > >> - kvm_vgic_unset_forwarding(irqfd->kvm, prod->irq, > >> - irqfd->gsi + VGIC_NR_PRIVATE_IRQS); > >> + case IRQ_BYPASS_VFIO_PCI_MSI: > >> + kvm_vgic_v4_unset_forwarding(irqfd->kvm, prod->irq, > >> + &irqfd->irq_entry); > >> + break; > >> + } > >> } > >> > >> void kvm_arch_irq_bypass_stop(struct irq_bypass_consumer *cons) > >> diff --git a/virt/kvm/arm/vgic/vgic-v4.c b/virt/kvm/arm/vgic/vgic-v4.c > >> index 207e1fda0dcd..338c86c5159f 100644 > >> --- a/virt/kvm/arm/vgic/vgic-v4.c > >> +++ b/virt/kvm/arm/vgic/vgic-v4.c > >> @@ -72,3 +72,106 @@ void vgic_v4_teardown(struct kvm *kvm) > >> its_vm->nr_vpes = 0; > >> its_vm->vpes = NULL; > >> } > >> + > >> +static struct vgic_its *vgic_get_its(struct kvm *kvm, > >> + struct kvm_kernel_irq_routing_entry *irq_entry) > >> +{ > >> + struct kvm_msi msi = (struct kvm_msi) { > >> + .address_lo = irq_entry->msi.address_lo, > >> + .address_hi = irq_entry->msi.address_hi, > >> + .data = irq_entry->msi.data, > >> + .flags = irq_entry->msi.flags, > >> + .devid = irq_entry->msi.devid, > >> + }; > >> + > >> + /* > >> + * Get a reference on the LPI. If NULL, this is not a valid > >> + * translation for any of our vITSs. > >> + */ > >> + return vgic_msi_to_its(kvm, &msi); > >> +} > >> + > >> +int kvm_vgic_v4_set_forwarding(struct kvm *kvm, int virq, > >> + struct kvm_kernel_irq_routing_entry *irq_entry) > >> +{ > >> + struct vgic_its *its; > >> + struct vgic_irq *irq; > >> + struct its_vlpi_map map; > >> + int ret; > >> + > >> + if (!vgic_is_v4_capable(kvm)) > >> + return 0; > >> + > >> + /* > >> + * Get the ITS, and escape early on error (not a valid > >> + * doorbell for any of our vITSs). > >> + */ > >> + its = vgic_get_its(kvm, irq_entry); > >> + if (IS_ERR(its)) > >> + return 0; > >> + > >> + mutex_lock(&its->its_lock); > >> + > >> + /* Perform then actual DevID/EventID -> LPI translation. */ > >> + ret = vgic_its_resolve_lpi(kvm, its, irq_entry->msi.devid, > >> + irq_entry->msi.data, &irq); > >> + if (ret) > >> + goto out; > >> + > >> + /* > >> + * Emit the mapping request. If it fails, the ITS probably > >> + * isn't v4 compatible, so let's silently bail out. Holding > >> + * the ITS lock should ensure that nothing can modify the > >> + * target vcpu. > >> + */ > >> + map = (struct its_vlpi_map) { > >> + .vm = &kvm->arch.vgic.its_vm, > >> + .vintid = irq->intid, > >> + .db_enabled = true, > >> + .vpe_idx = irq->target_vcpu->vcpu_id, > > This is just wrong. We cannot assume that the vcpu_id has anything to do > with the vpe_idx. It happens to be the same thing now, but the two things > should be clearly disconnected. > > I suggest the following (untested): > > diff --git a/virt/kvm/arm/vgic/vgic-v4.c b/virt/kvm/arm/vgic/vgic-v4.c > index cf5d6e2de6b8..0146e004401a 100644 > --- a/virt/kvm/arm/vgic/vgic-v4.c > +++ b/virt/kvm/arm/vgic/vgic-v4.c > @@ -251,13 +251,27 @@ static void dump_routing(int virq, struct kvm_kernel_irq_routing_entry *irq_entr > > } > > +static int vgic_v4_vcpu_to_index(struct its_vm *its_vm, struct kvm_vcpu *vcpu) > +{ > + int i; > + > + for (i = 0; i < its_vm->nr_vpes; i++) { > + struct its_vpe *vpe = &vcpu->arch.vgic_cpu.vgic_v3.its_vpe; > + > + if (its_vm->vpes[i] == vpe) > + return i; > + } > + > + return -ENODEV; > +} > + Stupid question: Can we change the struct its_vlpi_map to contain a vpe pointer or in stead of or in addition to the index? Alternatively, can you look it up using vcpu->arch.vgic_cpu.vgic_v3.its_vpe.irq() and another 'ioctl' in irq_set_vcpu_affinity() somehow instead of looping around and testing? > int kvm_vgic_v4_set_forwarding(struct kvm *kvm, int virq, > struct kvm_kernel_irq_routing_entry *irq_entry) > { > struct vgic_its *its; > struct vgic_irq *irq; > struct its_vlpi_map map; > - int ret; > + int ret, idx; > > dump_routing(virq, irq_entry, true); > > @@ -280,6 +294,11 @@ int kvm_vgic_v4_set_forwarding(struct kvm *kvm, int virq, > if (ret) > goto out; > > + idx = vgic_v4_vcpu_to_index(&kvm->arch.vgic.its_vm, > + irq->target_vcpu); > + if (idx < 0) > + goto out; > + > /* > * Emit the mapping request. If it fails, the ITS probably > * isn't v4 compatible, so let's silently bail out. Holding > @@ -290,7 +309,7 @@ int kvm_vgic_v4_set_forwarding(struct kvm *kvm, int virq, > .vm = &kvm->arch.vgic.its_vm, > .vintid = irq->intid, > .db_enabled = true, > - .vpe_idx = irq->target_vcpu->vcpu_id, > + .vpe_idx = idx, > }; > > if (its_map_vlpi(virq, &map)) > > Another option would be to just pass the pointer to the vpe instead, > and let the irq code to figure out the index, which can easily be > derived the doorbells (vpe->vpe_db_lpi - vpe->its_vm->db_lpi_base). > > I'll work something out for the next version. Ah, yeah, so I guess that is an option. It should be pretty quick to loop over even hundreds of vcpus, but that kind of thing just always seems wrong, somehow. Thanks, -Christoffer
diff --git a/virt/kvm/arm/vgic/vgic-v4.c b/virt/kvm/arm/vgic/vgic-v4.c index cf5d6e2de6b8..0146e004401a 100644 --- a/virt/kvm/arm/vgic/vgic-v4.c +++ b/virt/kvm/arm/vgic/vgic-v4.c @@ -251,13 +251,27 @@ static void dump_routing(int virq, struct kvm_kernel_irq_routing_entry *irq_entr } +static int vgic_v4_vcpu_to_index(struct its_vm *its_vm, struct kvm_vcpu *vcpu) +{ + int i; + + for (i = 0; i < its_vm->nr_vpes; i++) { + struct its_vpe *vpe = &vcpu->arch.vgic_cpu.vgic_v3.its_vpe; + + if (its_vm->vpes[i] == vpe) + return i; + } + + return -ENODEV; +} + int kvm_vgic_v4_set_forwarding(struct kvm *kvm, int virq, struct kvm_kernel_irq_routing_entry *irq_entry) { struct vgic_its *its; struct vgic_irq *irq; struct its_vlpi_map map; - int ret; + int ret, idx; dump_routing(virq, irq_entry, true); @@ -280,6 +294,11 @@ int kvm_vgic_v4_set_forwarding(struct kvm *kvm, int virq, if (ret) goto out; + idx = vgic_v4_vcpu_to_index(&kvm->arch.vgic.its_vm, + irq->target_vcpu); + if (idx < 0) + goto out; + /* * Emit the mapping request. If it fails, the ITS probably * isn't v4 compatible, so let's silently bail out. Holding @@ -290,7 +309,7 @@ int kvm_vgic_v4_set_forwarding(struct kvm *kvm, int virq, .vm = &kvm->arch.vgic.its_vm, .vintid = irq->intid, .db_enabled = true, - .vpe_idx = irq->target_vcpu->vcpu_id, + .vpe_idx = idx, }; if (its_map_vlpi(virq, &map))