On 09/10/2012 04:09 PM, Gleb Natapov wrote:
> Most interrupt are delivered to only one vcpu. Use pre-build tables to
> find interrupt destination instead of looping through all vcpus.
> 
> +
> +static inline int recalculate_apic_map(struct kvm *kvm)
> +{
> +     struct kvm_apic_map *new, *old = NULL;
> +     struct kvm_vcpu *vcpu;
> +     int i;
> +
> +     new = kzalloc(sizeof(struct kvm_apic_map), GFP_KERNEL);
> +
> +     if (!new)
> +             return -ENOMEM;
> +
> +     new->ldr_bits = 8;
> +     new->flat = true;
> +     kvm_for_each_vcpu(i, vcpu, kvm) {
> +             u16 cid, lid;
> +             struct kvm_lapic *apic = vcpu->arch.apic;
> +
> +             if (!kvm_apic_present(vcpu))
> +                     continue;
> +
> +             if (apic_x2apic_mode(apic)) {
> +                     new->ldr_bits = 32;
> +                     new->flat = false;
> +             } else if (kvm_apic_sw_enabled(apic) && new->flat &&
> +                             kvm_apic_get_reg(apic, APIC_DFR) == 
> APIC_DFR_CLUSTER)
> +                     new->flat = false;
> +
> +             new->phys_map[kvm_apic_id(apic)] = apic;
> +             kvm_apic_get_logical_id(kvm_apic_get_reg(apic, APIC_LDR),
> +                             new->flat, new->ldr_bits, &cid, &lid);
> +
> +             if (lid)
> +                     new->logical_map[cid][ffs(lid) - 1] = apic;
> +     }
> +     mutex_lock(&kvm->arch.apic_map_lock);
> +     old = kvm->arch.apic_map;
> +     rcu_assign_pointer(kvm->arch.apic_map, new);
> +     mutex_unlock(&kvm->arch.apic_map_lock);

is this not racy?

cpu 0             cpu 1
----------------  -----------------
new apic id
recalculate

                  new apic id
                  recalculate

                  take lock
                  install map
                  drop lock

take lock
install map
drop lock


> +
> +     if (old)
> +             call_rcu(&old->rcu, rcu_free_apic_map);
> +
> +     return 0;
> +}
> +

> -
>  static const unsigned int apic_lvt_mask[APIC_LVT_NUM] = {
>       LVT_MASK ,      /* part LVTT mask, timer mode mask added at runtime */
>       LVT_MASK | APIC_MODE_MASK,      /* LVTTHMR */
> @@ -477,6 +562,67 @@ int kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct 
> kvm_lapic *source,
>       return result;
>  }
>  
> +bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
> +             struct kvm_lapic_irq *irq, int *r)
> +{
> +     unsigned long bitmap = 1;
> +     struct kvm_lapic **dst;
> +     int i;
> +
> +     if (irq->shorthand == APIC_DEST_SELF) {
> +             *r = kvm_apic_set_irq(src->vcpu, irq);
> +             return true;
> +     }
> +
> +     if (irq->shorthand)
> +             return false;
> +
> +     if (irq->dest_mode == 0) { /* physical mode */
> +             if (irq->delivery_mode == APIC_DM_LOWEST ||
> +                             irq->dest_id == 0xff)
> +                     return false;
> +             rcu_read_lock();

Unmatched rcu_read_lock() inside a block is ugly, please start it earlier.

> +             dst = &kvm->arch.apic_map->phys_map[irq->dest_id & 0xff];

Like mst says, rcu_dereference().

> +     } else {
> +             u16 cid, lid;
> +             u32 mda = irq->dest_id;
> +
> +             rcu_read_lock();
> +             if (kvm->arch.apic_map->ldr_bits == 8)
> +                     mda <<= 24;
> +
> +             kvm_apic_get_logical_id(mda, kvm->arch.apic_map->flat,
> +                             kvm->arch.apic_map->ldr_bits, &cid, &lid);
> +             dst = kvm->arch.apic_map->logical_map[cid];
> +
> +             bitmap = lid;
> +             if (irq->delivery_mode == APIC_DM_LOWEST) {
> +                     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)
> +                                     l = i;
> +                     }
> +
> +                     bitmap = (l >= 0) ? 1 << l : 0;
> +             }
> +     }
> +
> +     for_each_set_bit(i, &bitmap, 16) {
> +             if (!dst[i])
> +                     continue;
> +             if (*r < 0)
> +                     *r = 0;
> +             *r += kvm_apic_set_irq(dst[i]->vcpu, irq);
> +     }
> +
> +     rcu_read_unlock();
> +     return true;
> +}
> +
>  /*
>   * Add a pending IRQ into lapic.
>   * Return 1 if successfully added and 0 if discarded.
> @@ -880,7 +1026,7 @@ static int apic_reg_write(struct kvm_lapic *apic, u32 
> reg, u32 val)
>       switch (reg) {
>       case APIC_ID:           /* Local APIC ID */
>               if (!apic_x2apic_mode(apic))
> -                     apic_set_reg(apic, APIC_ID, val);
> +                     kvm_apic_set_id(apic, val >> 24);
>               else
>                       ret = 1;
>               break;
> @@ -896,15 +1042,16 @@ static int apic_reg_write(struct kvm_lapic *apic, u32 
> reg, u32 val)
>  
>       case APIC_LDR:
>               if (!apic_x2apic_mode(apic))
> -                     apic_set_reg(apic, APIC_LDR, val & APIC_LDR_MASK);
> +                     kvm_apic_set_ldr(apic, val & APIC_LDR_MASK);
>               else
>                       ret = 1;
>               break;
>  
>       case APIC_DFR:
> -             if (!apic_x2apic_mode(apic))
> +             if (!apic_x2apic_mode(apic)) {
>                       apic_set_reg(apic, APIC_DFR, val | 0x0FFFFFFF);
> -             else
> +                     recalculate_apic_map(apic->vcpu->kvm);
> +             } else
>                       ret = 1;

What if these have different settings on different vcpus?

>               break;
>  



-- 
error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to