RE: [PATCH] KVM: x86: Add lowest-priority support for vt-d posted-interrupts

[Wu, Feng]

> -----Original Message-----
> From: Radim Krčmář [mailto:rkrcmar@xxxxxxxxxx]
> Sent: Tuesday, November 17, 2015 3:03 AM
> To: Wu, Feng <feng.wu@xxxxxxxxx>
> Cc: pbonzini@xxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] KVM: x86: Add lowest-priority support for vt-d posted-
> interrupts
> 
> 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 ...)
> 
> > 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?)

Sorry for the late response, I try to get more information about vector
hashing before getting back to you. Here is the response from our
hardware architect:

"I don't think we do any vector hashing on our client parts.  This may be why the customer is not able to detect this on Skylake client silicon.
The vector hashing is micro-architectural and something we had done on server parts.

If you look at the haswell server CPU spec (https://www-ssl.intel.com/content/dam/www/public/us/en/documents/datasheets/xeon-e5-v3-datasheet-vol-2.pdf)
In section 4.1.2, you will see an IntControl register (this is a register controlled/configured by BIOS) - see below.

If you look at bits 6:4 in that register, you see the option we offer in hardware for what kind of redirection is applied to lowest priority interrupts.
There are three options:
1.	Fixed priority  
2.	Redirect last 
3.	Hash Vector

If picking vector hash, then bits 10:8 specifies the APIC-ID bits used for the hashing."

Thanks,
Feng


> 
> > +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 linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/