Gleb Natapov wrote on 2013-01-20:
> On Thu, Jan 17, 2013 at 01:26:03AM +0000, Zhang, Yang Z wrote:
>> Previous patch is stale. Resend the new patch.
>> The only change is clear EOI and SELF-IPI reg in msr bitmap when vid is
>> enabled.
>>
>> ------------------------
>> @@ -340,6 +325,8 @@ static inline int apic_find_highest_irr(struct kvm_lapic
> *apic)
>> {
>> int result;
>> + /* Note that irr_pending is just a hint. It will be always
>> + * true with virtual interrupt delivery enabled. */
> This is not correct format for multi-line comments.
Sure, will correct it here and below.
>> +static void vmx_check_ioapic_entry(struct kvm_vcpu *vcpu,
>> + struct kvm_lapic_irq *irq)
>> +{
>> + struct kvm_lapic **dst;
>> + struct kvm_apic_map *map;
>> + unsigned long bitmap = 1;
>> + int i;
>> +
>> + rcu_read_lock();
>> + map = rcu_dereference(vcpu->kvm->arch.apic_map);
>> +
>> + if (unlikely(!map)) {
>> + set_eoi_exitmap_one(vcpu, irq->vector);
>> + goto out;
>> + }
>> +
>> + if (irq->dest_mode == 0) { /* physical mode */
>> + if (irq->delivery_mode == APIC_DM_LOWEST ||
>> + irq->dest_id == 0xff) {
>> + set_eoi_exitmap_one(vcpu, irq->vector);
>> + goto out;
>> + }
>> + dst = &map->phys_map[irq->dest_id & 0xff];
>> + } else {
>> + u32 mda = irq->dest_id << (32 - map->ldr_bits);
>> +
>> + dst = map->logical_map[apic_cluster_id(map, mda)];
>> +
>> + bitmap = apic_logical_id(map, mda);
>> + }
>> +
>> + for_each_set_bit(i, &bitmap, 16) {
>> + if (!dst[i])
>> + continue;
>> + if (dst[i]->vcpu == vcpu) {
>> + set_eoi_exitmap_one(vcpu, irq->vector);
>> + break;
>> + }
>> + }
>> +
>> +out:
>> + rcu_read_unlock();
>> +}
> The logic in this function belongs to lapic code. The only thing
> that is specific to vmx in the function is setting of the bit in
> vmx->eoi_exit_bitmap, but since eoi_exit_bitmap is calculated and
> loaded during same vcpu entry we do not need vmx->eoi_exit_bitmap at
> all. Declare it on a stack in vmx_update_eoi_exitmap() and pass it to
> set_eoi_exitmap() and vmx_load_eoi_exitmap().
IIRC, this logic is in lapic before v7. And you suggested to move the whole
function into vmx code.
So, it better to move back to lapic file?
>> @@ -115,6 +116,42 @@ static void update_handled_vectors(struct kvm_ioapic
> *ioapic)
>> smp_wmb();
>> }
>> +void set_eoi_exitmap(struct kvm_vcpu *vcpu)
>> +{
> This function is exported from the file and need to have more unique
> name. kvm_ioapic_calculate_eoi_exitmap() for instance.
Ok.
>> @@ -156,6 +193,7 @@ static void ioapic_write_indirect(struct kvm_ioapic
> *ioapic, u32 val)
>> if (e->fields.trig_mode == IOAPIC_LEVEL_TRIG
>> && ioapic->irr & (1 << index))
>> ioapic_service(ioapic, index);
>> + ioapic_update_eoi_exitmap(ioapic->kvm);
> ioapic_write_indirect() is called under ioapic->lock,
> ioapic_update_eoi_exitmap() takes the same lock. Have you tested the
> code?
ioapic_update_eoi_exitmap doesn't take any lock.
I will do a full testing for every patch before sending out. It covers both
windows and Linux guest.
Best regards,
Yang
--
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
