> -----Original Message----- > From: Yang Zhang [mailto:yang.zhang.wz@xxxxxxxxx] > Sent: Thursday, January 21, 2016 1:43 PM > To: Wu, Feng <feng.wu@xxxxxxxxx>; pbonzini@xxxxxxxxxx; > rkrcmar@xxxxxxxxxx > Cc: linux-kernel@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx > Subject: Re: [PATCH v3 2/4] KVM: x86: Use vector-hashing to deliver lowest- > priority interrupts > > On 2016/1/21 13:33, Wu, Feng wrote: > > > > > >> -----Original Message----- > >> From: linux-kernel-owner@xxxxxxxxxxxxxxx [mailto:linux-kernel- > >> owner@xxxxxxxxxxxxxxx] On Behalf Of Yang Zhang > >> Sent: Thursday, January 21, 2016 1:24 PM > >> To: Wu, Feng <feng.wu@xxxxxxxxx>; pbonzini@xxxxxxxxxx; > >> rkrcmar@xxxxxxxxxx > >> Cc: linux-kernel@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx > >> Subject: Re: [PATCH v3 2/4] KVM: x86: Use vector-hashing to deliver > lowest- > >> priority interrupts > >> > >> On 2016/1/20 9:42, Feng Wu wrote: > >>> Use vector-hashing to deliver lowest-priority interrupts, As an > >>> example, modern Intel CPUs in server platform use this method to > >>> handle lowest-priority interrupts. > >>> > >>> Signed-off-by: Feng Wu <feng.wu@xxxxxxxxx> > >>> --- > >>> bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic > >> *src, > >>> struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map) > >>> { > >>> @@ -727,21 +743,51 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm > >> *kvm, struct kvm_lapic *src, > >>> > >>> dst = map->logical_map[cid]; > >>> > >>> - if (kvm_lowest_prio_delivery(irq)) { > >>> + if (!kvm_lowest_prio_delivery(irq)) > >>> + goto set_irq; > >>> + > >>> + if (!kvm_vector_hashing_enabled()) { > >>> int l = -1; > >>> for_each_set_bit(i, &bitmap, 16) { > >>> if (!dst[i]) > >>> continue; > >>> if (l < 0) > >>> l = i; > >>> - else if (kvm_apic_compare_prio(dst[i]->vcpu, > >> dst[l]->vcpu) < 0) > >>> + else if (kvm_apic_compare_prio(dst[i]->vcpu, > >>> + dst[l]->vcpu) < 0) > >>> l = i; > >>> } > >>> - > >>> bitmap = (l >= 0) ? 1 << l : 0; > >>> + } else { > >>> + int idx = 0; > >>> + unsigned int dest_vcpus = 0; > >>> + > >>> + dest_vcpus = hweight16(bitmap); > >>> + if (dest_vcpus == 0) > >>> + goto out; > >>> + > >>> + idx = kvm_vector_2_index(irq->vector, > >>> + dest_vcpus, &bitmap, 16); > >>> + > >>> + /* > >>> + * We may find a hardware disabled LAPIC here, if > >> that > >>> + * is the case, print out a error message once for each > >>> + * guest and return. > >>> + */ > >>> + if (!dst[idx-1] && > >>> + (kvm->arch.disabled_lapic_found == 0)) { > >>> + kvm->arch.disabled_lapic_found = 1; > >>> + printk(KERN_ERR > >>> + "Disabled LAPIC found during irq > >> injection\n"); > >>> + goto out; > >> > >> What does "goto out" mean? Inject successfully or fail? According the > >> value of ret which is set to ture here, it means inject successfully but > >> i = -1. > >> > > > > Oh, I didn't notice 'ret' is initialized to true, I thought it was initialized > > to false like another function, I should add a "ret = false' here. We should > > failed to inject the interrupt since hardware disabled LAPIC is found. > > I remember we have discussed that even the LAPIC is software disabled, > it still can respond to some interrupts like INIT, NMI, SMI, and SIPI > messages. Isn't current logic still problematically? I don't think there are problems, here we only cover lowest-priority mode. Thanks, Feng > > -- > best regards > yang -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
