On 29/01/2015 22:48, Radim Krčmář wrote:
> recalculate_apic_map() uses two passes over all VCPUs.  This is a relic
> from time when we selected a global mode in the first pass and set up
> the optimized table in the second pass (to have a consistent mode).
> 
> Recent changes made mixed mode unoptimized and we can do it in one pass.
> Format of logical MDA is a function of the mode, so we encode it in
> apic_logical_id() and drop obsoleted variables from the struct.
> 
> Signed-off-by: Radim Krčmář <rkrc...@redhat.com>
> ---
>  arch/x86/include/asm/kvm_host.h |  3 --
>  arch/x86/kvm/lapic.c            | 78 
> ++++++++++-------------------------------
>  arch/x86/kvm/lapic.h            | 29 ++++++++-------
>  3 files changed, 36 insertions(+), 74 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index fec3188cabbb..14b6b0fd17b1 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -555,9 +555,6 @@ struct kvm_arch_memory_slot {
>  struct kvm_apic_map {
>       struct rcu_head rcu;
>       u8 mode;
> -     u8 ldr_bits;
> -     /* fields bellow are used to decode ldr values in different modes */
> -     u32 cid_shift, cid_mask, lid_mask;
>       struct kvm_lapic *phys_map[256];
>       /* first index is cluster id second is cpu id in a cluster */
>       struct kvm_lapic *logical_map[16][16];
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 621d9df6ac63..f74557791a77 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -146,49 +146,6 @@ static void recalculate_apic_map(struct kvm *kvm)
>       if (!new)
>               goto out;
>  
> -     new->ldr_bits = 8;
> -     /* flat mode is default */
> -     new->cid_shift = 8;
> -     new->cid_mask = 0;
> -     new->lid_mask = 0xff;
> -
> -     kvm_for_each_vcpu(i, vcpu, kvm) {
> -             struct kvm_lapic *apic = vcpu->arch.apic;
> -
> -             if (!kvm_apic_present(vcpu))
> -                     continue;
> -
> -             if (apic_x2apic_mode(apic)) {
> -                     new->ldr_bits = 32;
> -                     new->cid_shift = 16;
> -                     new->cid_mask = new->lid_mask = 0xffff;
> -                     new->mode |= KVM_APIC_MODE_X2APIC;
> -             } else if (kvm_apic_get_reg(apic, APIC_LDR)) {
> -                     if (kvm_apic_get_reg(apic, APIC_DFR) ==
> -                                                     APIC_DFR_CLUSTER) {
> -                             new->cid_shift = 4;
> -                             new->cid_mask = 0xf;
> -                             new->lid_mask = 0xf;
> -                             new->mode |= KVM_APIC_MODE_XAPIC_CLUSTER;
> -                     } else {
> -                             new->cid_shift = 8;
> -                             new->cid_mask = 0;
> -                             new->lid_mask = 0xff;
> -                             new->mode |= KVM_APIC_MODE_XAPIC_FLAT;
> -                     }
> -             }
> -
> -             /*
> -              * All APICs have to be configured in the same mode by an OS.
> -              * We take advatage of this while building logical id loockup
> -              * table. After reset APICs are in software disabled mode, so if
> -              * we find apic with different setting we assume this is the 
> mode
> -              * OS wants all apics to be in; build lookup table accordingly.
> -              */
> -             if (kvm_apic_sw_enabled(apic))
> -                     break;
> -     }
> -
>       kvm_for_each_vcpu(i, vcpu, kvm) {
>               struct kvm_lapic *apic = vcpu->arch.apic;
>               u16 cid, lid;
> @@ -198,17 +155,22 @@ static void recalculate_apic_map(struct kvm *kvm)
>                       continue;
>  
>               aid = kvm_apic_id(apic);
> -             ldr = kvm_apic_get_reg(apic, APIC_LDR);
> -             cid = apic_cluster_id(new, ldr);
> -             lid = apic_logical_id(new, ldr);
> -
>               if (aid < ARRAY_SIZE(new->phys_map))
>                       new->phys_map[aid] = apic;
>  
> -             /* The logical map is definitely wrong if we have multiple
> -              * modes at the same time.  Physical is still right though.
> -              */
> -             if (hweight8(new->mode) != 1)
> +             ldr = kvm_apic_get_reg(apic, APIC_LDR);
> +
> +             if (apic_x2apic_mode(apic)) {
> +                     new->mode |= KVM_APIC_MODE_X2APIC;
> +             } else if (ldr) {
> +                     ldr = GET_APIC_LOGICAL_ID(ldr);
> +                     if (kvm_apic_get_reg(apic, APIC_DFR) == APIC_DFR_FLAT)
> +                             new->mode |= KVM_APIC_MODE_XAPIC_FLAT;
> +                     else
> +                             new->mode |= KVM_APIC_MODE_XAPIC_CLUSTER;
> +             }
> +
> +             if (!apic_logical_id(new, ldr, &cid, &lid))
>                       continue;
>  
>               if (lid && cid < ARRAY_SIZE(new->logical_map))
> @@ -724,22 +686,20 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, 
> struct kvm_lapic *src,
>  
>               dst = &map->phys_map[irq->dest_id];
>       } else {
> -             u32 mda = irq->dest_id << (32 - map->ldr_bits);
> -             u16 cid = apic_cluster_id(map, mda);
> +             u16 cid;
>  
> -             if (cid >= ARRAY_SIZE(map->logical_map))
> -                     goto out;
> -
> -             if (hweight8(map->mode) != 1) {
> +             if (!apic_logical_id(map, irq->dest_id, &cid, (u16 *)&bitmap))
> +             {
>                       /* Not deliverable with optimized map. */
>                       ret = false;
>                       goto out;
>               }
>  
> +             if (cid >= ARRAY_SIZE(map->logical_map))
> +                     goto out;
> +
>               dst = map->logical_map[cid];
>  
> -             bitmap = apic_logical_id(map, mda);
> -
>               if (irq->delivery_mode == APIC_DM_LOWEST) {
>                       int l = -1;
>                       for_each_set_bit(i, &bitmap, 16) {
> diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
> index fd0197a93862..320716e301a8 100644
> --- a/arch/x86/kvm/lapic.h
> +++ b/arch/x86/kvm/lapic.h
> @@ -151,19 +151,24 @@ static inline bool kvm_apic_vid_enabled(struct kvm *kvm)
>       return kvm_x86_ops->vm_has_apicv(kvm);
>  }
>  
> -static inline u16 apic_cluster_id(struct kvm_apic_map *map, u32 ldr)
> +static inline bool
> +apic_logical_id(struct kvm_apic_map *map, u32 ldr, u16 *cid, u16 *lid)
>  {
> -     u16 cid;
> -     ldr >>= 32 - map->ldr_bits;
> -     cid = (ldr >> map->cid_shift) & map->cid_mask;
> -
> -     return cid;
> -}
> -
> -static inline u16 apic_logical_id(struct kvm_apic_map *map, u32 ldr)
> -{
> -     ldr >>= (32 - map->ldr_bits);
> -     return ldr & map->lid_mask;
> +     switch (map->mode) {
> +     case KVM_APIC_MODE_XAPIC_FLAT:
> +             *cid = 0;
> +             *lid = ldr & 0xff;
> +             return true;
> +     case KVM_APIC_MODE_XAPIC_CLUSTER:
> +             *cid = (ldr >> 4) & 0xf;
> +             *lid = ldr & 0xf;
> +             return true;
> +     case KVM_APIC_MODE_X2APIC:
> +             *cid = ldr >> 16;
> +             *lid = ldr & 0xffff;
> +             return true;
> +     }

We need some optimization here.  You can make the defines equal to the
size of the lid: CLUSTER = 1 << 3, FLAT = 1 << 2, X2APIC = 1 << 4:

        BUILD_BUG_ON(...FLAT != 4);
        BUILD_BUG_ON(...CLUSTER != 8);
        BUILD_BUG_ON(...X2APIC != 16);

        lid_bits = mode;
        cid_bits = mode & (16 | 4);
        lid_mask = (1 << lid_bits) - 1;
        cid_mask = (1 << cid_bits) - 1;

        *cid = (ldr >> lid_bits) & cid_mask;
        *lid = ldr & lid_mask;

Please move this to lapic.c since you are at it.

Paolo

> +     return false;
>  }
>  
>  static inline bool kvm_apic_has_events(struct kvm_vcpu *vcpu)
> 
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to