On 16/11/2015 20:03, Radim Krčmář wrote: > 2015-11-09 10:46+0800, Feng Wu: >> Use vector-hashing to handle lowest-priority interrupts for >> posted-interrupts. As an example, modern Intel CPUs use this >> method to handle lowest-priority interrupts. > > (I don't think it's a good idea that the algorithm differs from non-PI > lowest priority delivery. I'd make them both vector-hashing, which > would be "fun" to explain to people expecting round robin ...) Yup, I would make it a module option. Thanks very much Radim for helping with the review. Paolo >> Signed-off-by: Feng Wu <feng.wu@xxxxxxxxx> >> --- >> diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c >> +/* >> + * This routine handles lowest-priority interrupts using vector-hashing >> + * mechanism. As an example, modern Intel CPUs use this method to handle >> + * lowest-priority interrupts. >> + * >> + * Here is the details about the vector-hashing mechanism: >> + * 1. For lowest-priority interrupts, store all the possible destination >> + * vCPUs in an array. >> + * 2. Use "guest vector % max number of destination vCPUs" to find the right >> + * destination vCPU in the array for the lowest-priority interrupt. >> + */ > > (Is Skylake i7-6700 a modern Intel CPU? > I didn't manage to get hashing ... all interrupts always went to the > lowest APIC ID in the set :/ > Is there a simple way to verify the algorithm?) > >> +struct kvm_vcpu *kvm_intr_vector_hashing_dest(struct kvm *kvm, >> + struct kvm_lapic_irq *irq) >> + >> +{ >> + unsigned long dest_vcpu_bitmap[BITS_TO_LONGS(KVM_MAX_VCPUS)]; >> + unsigned int dest_vcpus = 0; >> + struct kvm_vcpu *vcpu; >> + unsigned int i, mod, idx = 0; >> + >> + vcpu = kvm_intr_vector_hashing_dest_fast(kvm, irq); >> + if (vcpu) >> + return vcpu; > > I think the rest of this function shouldn't be implemented: > - Shorthands are only for IPIs and hence don't need to be handled, > - Lowest priority physical broadcast is not supported, > - Lowest priority cluster logical broadcast is not supported, > - No point in optimizing mixed xAPIC and x2APIC mode, > - The rest is handled by kvm_intr_vector_hashing_dest_fast(). > (Even lowest priority flat logical "broadcast".) > - We do the work twice when vcpu == NULL means that there is no > matching destination. > > Is there a valid case that can be resolved by going through all vcpus? > >> + >> + memset(dest_vcpu_bitmap, 0, sizeof(dest_vcpu_bitmap)); >> + >> + kvm_for_each_vcpu(i, vcpu, kvm) { >> + if (!kvm_apic_present(vcpu)) >> + continue; >> + >> + if (!kvm_apic_match_dest(vcpu, NULL, irq->shorthand, >> + irq->dest_id, irq->dest_mode)) >> + continue; >> + >> + __set_bit(vcpu->vcpu_id, dest_vcpu_bitmap); >> + dest_vcpus++; >> + } >> + >> + if (dest_vcpus == 0) >> + return NULL; >> + >> + mod = irq->vector % dest_vcpus; >> + >> + for (i = 0; i <= mod; i++) { >> + idx = find_next_bit(dest_vcpu_bitmap, KVM_MAX_VCPUS, idx) + 1; >> + BUG_ON(idx >= KVM_MAX_VCPUS); >> + } >> + >> + return kvm_get_vcpu(kvm, idx - 1); >> +} >> +EXPORT_SYMBOL_GPL(kvm_intr_vector_hashing_dest); >> + >> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c >> @@ -816,6 +816,63 @@ out: >> +struct kvm_vcpu *kvm_intr_vector_hashing_dest_fast(struct kvm *kvm, >> + struct kvm_lapic_irq *irq) > > We now have three very similar functions :( > > kvm_irq_delivery_to_apic_fast > kvm_intr_is_single_vcpu_fast > kvm_intr_vector_hashing_dest_fast > > By utilizing the gcc optimizer, they can be merged without introducing > many instructions to the hot path, kvm_irq_delivery_to_apic_fast. > (I would eventually do it, so you can save time by ignoring this.) > > Thanks. > -- 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