On Sat, 2022-10-01 at 00:59 +0000, Sean Christopherson wrote:
> Iterate over all target logical IDs in the AVIC kick fastpath instead of
> bailing if there is more than one target. Now that KVM inhibits AVIC if
> vCPUs aren't mapped 1:1 with logical IDs, each bit in the destination is
> guaranteed to match to at most one vCPU, i.e. iterating over the bitmap
> is guaranteed to kick each valid target exactly once.
>
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
> arch/x86/kvm/svm/avic.c | 112 ++++++++++++++++++++++------------------
> 1 file changed, 63 insertions(+), 49 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index a9e4e09f83fc..17e64b056e4e 100644
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
> @@ -327,6 +327,50 @@ static void avic_kick_vcpu(struct kvm_vcpu *vcpu, u32 icrl)
> icrl & APIC_VECTOR_MASK);
> }
>
> +static void avic_kick_vcpu_by_physical_id(struct kvm *kvm, u32 physical_id,
> + u32 icrl)
> +{
> + /*
> + * KVM inhibits AVIC if any vCPU ID diverges from the vCPUs APIC ID,
> + * i.e. APIC ID == vCPU ID.
> + */
> + struct kvm_vcpu *target_vcpu = kvm_get_vcpu_by_id(kvm, physical_id);
> +
> + /* Once again, nothing to do if the target vCPU doesn't exist. */
> + if (unlikely(!target_vcpu))
> + return;
> +
> + avic_kick_vcpu(target_vcpu, icrl);
> +}
> +
> +static void avic_kick_vcpu_by_logical_id(struct kvm *kvm, u32 *avic_logical_id_table,
> + u32 logid_index, u32 icrl)
> +{
> + u32 physical_id;
> +
> + if (avic_logical_id_table) {
> + u32 logid_entry = avic_logical_id_table[logid_index];
> +
> + /* Nothing to do if the logical destination is invalid. */
> + if (unlikely(!(logid_entry & AVIC_LOGICAL_ID_ENTRY_VALID_MASK)))
> + return;
> +
> + physical_id = logid_entry &
> + AVIC_LOGICAL_ID_ENTRY_GUEST_PHYSICAL_ID_MASK;
> + } else {
> + /*
> + * For x2APIC, the logical APIC ID is a read-only value that is
> + * derived from the x2APIC ID, thus the x2APIC ID can be found
> + * by reversing the calculation (stored in logid_index). Note,
> + * bits 31:20 of the x2APIC ID aren't propagated to the logical
> + * ID, but KVM limits the x2APIC ID limited to KVM_MAX_VCPU_IDS.
> + */
> + physical_id = logid_index;
> + }
> +
> + avic_kick_vcpu_by_physical_id(kvm, physical_id, icrl);
> +}
> +
> /*
> * A fast-path version of avic_kick_target_vcpus(), which attempts to match
> * destination APIC ID to vCPU without looping through all vCPUs.
> @@ -334,11 +378,10 @@ static void avic_kick_vcpu(struct kvm_vcpu *vcpu, u32 icrl)
> static int avic_kick_target_vcpus_fast(struct kvm *kvm, struct kvm_lapic *source,
> u32 icrl, u32 icrh, u32 index)
> {
> - u32 l1_physical_id, dest;
> - struct kvm_vcpu *target_vcpu;
> int dest_mode = icrl & APIC_DEST_MASK;
> int shorthand = icrl & APIC_SHORT_MASK;
> struct kvm_svm *kvm_svm = to_kvm_svm(kvm);
> + u32 dest;
>
> if (shorthand != APIC_DEST_NOSHORT)
> return -EINVAL;
> @@ -355,14 +398,14 @@ static int avic_kick_target_vcpus_fast(struct kvm *kvm, struct kvm_lapic *source
> if (!apic_x2apic_mode(source) && dest == APIC_BROADCAST)
> return -EINVAL;
>
> - l1_physical_id = dest;
> -
> - if (WARN_ON_ONCE(l1_physical_id != index))
> + if (WARN_ON_ONCE(dest != index))
> return -EINVAL;
>
> + avic_kick_vcpu_by_physical_id(kvm, dest, icrl);
> } else {
> - u32 bitmap, cluster;
> - int logid_index;
> + u32 *avic_logical_id_table;
> + unsigned long bitmap, i;
> + u32 cluster;
>
> if (apic_x2apic_mode(source)) {
> /* 16 bit dest mask, 16 bit cluster id */
> @@ -382,50 +425,21 @@ static int avic_kick_target_vcpus_fast(struct kvm *kvm, struct kvm_lapic *source
> if (unlikely(!bitmap))
> return 0;
>
> - if (!is_power_of_2(bitmap))
> - /* multiple logical destinations, use slow path */
> - return -EINVAL;
> -
> - logid_index = cluster + __ffs(bitmap);
> -
> - if (apic_x2apic_mode(source)) {
> - /*
> - * For x2APIC, the logical APIC ID is a read-only value
> - * that is derived from the x2APIC ID, thus the x2APIC
> - * ID can be found by reversing the calculation (done
> - * above). Note, bits 31:20 of the x2APIC ID are not
> - * propagated to the logical ID, but KVM limits the
> - * x2APIC ID limited to KVM_MAX_VCPU_IDS.
> - */
> - l1_physical_id = logid_index;
> - } else {
> - u32 *avic_logical_id_table =
> - page_address(kvm_svm->avic_logical_id_table_page);
> -
> - u32 logid_entry = avic_logical_id_table[logid_index];
> -
> - if (WARN_ON_ONCE(index != logid_index))
> - return -EINVAL;
> -
> - /* Nothing to do if the logical destination is invalid. */
> - if (unlikely(!(logid_entry & AVIC_LOGICAL_ID_ENTRY_VALID_MASK)))
> - return 0;
> -
> - l1_physical_id = logid_entry &
> - AVIC_LOGICAL_ID_ENTRY_GUEST_PHYSICAL_ID_MASK;
> - }
> + if (apic_x2apic_mode(source))
> + avic_logical_id_table = NULL;
> + else
> + avic_logical_id_table = page_address(kvm_svm->avic_logical_id_table_page);
> +
> + /*
> + * AVIC is inhibited if vCPUs aren't mapped 1:1 with logical
> + * IDs, thus each bit in the destination is guaranteed to map
> + * to at most one vCPU.
> + */
> + for_each_set_bit(i, &bitmap, 16)
> + avic_kick_vcpu_by_logical_id(kvm, avic_logical_id_table,
> + cluster + i, icrl);
> }
>
> - /*
> - * KVM inhibits AVIC if any vCPU ID diverges from the vCPUs APIC ID,
> - * i.e. APIC ID == vCPU ID. Once again, nothing to do if the target
> - * vCPU doesn't exist.
> - */
> - target_vcpu = kvm_get_vcpu_by_id(kvm, l1_physical_id);
> - if (unlikely(!target_vcpu))
> - return 0;
> -
> - avic_kick_vcpu(target_vcpu, icrl);
> return 0;
> }
>
Looks OK, but I might have missed something.
Reviewed-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx>
Best regards,
Maxim Levitsky
