RE: [PATCH v4 5/7] KVM: Recalculate destination vcpu map

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Gleb Natapov wrote on 2013-03-21:
> On Thu, Mar 21, 2013 at 05:30:32AM +0000, Zhang, Yang Z wrote:
>> Gleb Natapov wrote on 2013-03-21:
>>> On Thu, Mar 21, 2013 at 03:42:46AM +0000, Zhang, Yang Z wrote:
>>>> Gleb Natapov wrote on 2013-03-20:
>>>>> On Wed, Mar 20, 2013 at 07:36:17PM +0800, Yang Zhang wrote:
>>>>>> From: Yang Zhang <yang.z.zhang@xxxxxxxxx>
>>>>>> 
>>>>>> Update RTC interrrupt's destination vcpu map when ioapic entry of RTC
>>>>>> or apic register (id, ldr, dfr) is changed.
>>>>>> 
>>>>>> Signed-off-by: Yang Zhang <yang.z.zhang@xxxxxxxxx>
>>>>>> ---
>>>>>>  virt/kvm/ioapic.c |    9 +++++++--
>>>>>>  1 files changed, 7 insertions(+), 2 deletions(-)
>>>>>> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
>>>>>> index ddf9414..91b4c08 100644
>>>>>> --- a/virt/kvm/ioapic.c
>>>>>> +++ b/virt/kvm/ioapic.c
>>>>>> @@ -121,6 +121,7 @@ void kvm_ioapic_scan_entry(struct kvm_vcpu
> *vcpu,
>>>>>>  { 	struct kvm_ioapic *ioapic = vcpu->kvm->arch.vioapic; 	union
>>>>>>  kvm_ioapic_redirect_entry *e; +	unsigned long *rtc_map =
>>>>>>  ioapic->rtc_status.vcpu_map; 	struct kvm_lapic_irq irqe; 	int index;
>>>>>> @@ -130,15 +131,19 @@ void kvm_ioapic_scan_entry(struct kvm_vcpu
>>> *vcpu,
>>>>>>  		if (!e->fields.mask &&
>>>>>>  			(e->fields.trig_mode == IOAPIC_LEVEL_TRIG ||
>>>>>>  			 kvm_irq_has_notifier(ioapic->kvm, KVM_IRQCHIP_IOAPIC,
>>>>>> -				 index))) {
>>>>>> +				 index) || index == 8)) {
>>>>>>  			irqe.dest_id = e->fields.dest_id;
>>>>>>  			irqe.vector = e->fields.vector;
>>>>>>  			irqe.dest_mode = e->fields.dest_mode;
>>>>>>  			irqe.shorthand = 0;
>>>>>>  
>>>>>>  			if (kvm_apic_match_dest(vcpu, NULL, irqe.shorthand,
>>>>>> -						irqe.dest_id, irqe.dest_mode))
>>>>>> +						irqe.dest_id, irqe.dest_mode)) {
>>>>>>  				__set_bit(irqe.vector, eoi_exit_bitmap);
>>>>>> +				if (index == 8)
>>>>>> +					__set_bit(vcpu->vcpu_id, rtc_map);
>>>>>> +			} else if (index == 8)
>>>>>> +				__clear_bit(vcpu->vcpu_id, rtc_map);
>>>>> rtc_map bitmap is accessed from different vcpus simultaneously so access
>>>>> has to be atomic. We also have a race:
>>>>> 
>>>>> vcpu0                               iothread
>>>>> ioapic config changes
>>>>> request scan ioapic
>>>>>                                      inject rtc interrupt
>>>>>                                      use old vcpu mask
>>>>> scan_ioapic()
>>>>> recalculate vcpu mask
>>>>> 
>>>>> So this approach (suggested by me :() will not work.
>>>>> 
>>>>> Need to think about it some more. May be your idea of building a bitmap
>>>>> while injecting the interrupt is the way to go indeed: pass a pointer to
>>>>> a bitmap to kvm_irq_delivery_to_apic() and build it there. Pass NULL
>>>>> pointer if caller does not need to track vcpus.
>>>> Or, we can block inject rtc interrupt during recalculate vcpu map.
>>>> 
>>>> if(need_eoi > 0 && in_recalculating)
>>>> return coalesced
>>>> 
>>> This should be ||. Then you need to maintain in_recalculating and
>>> recalculations requests may overlap. Too complex and fragile.
>> It should not be too complex. How about the following logic?
>> 
>> when make scan ioapic request:
>> kvm_vcpu_scan_ioapic()
>> {
>> kvm_for_each_vcpu()
>> 	in_recalculating++;
>> }
>> 
>> Then on each vcpu's request handler:
>> vcpu_scan_ioapic()
>> {
>> in_recalculating--;
>> }
>> 
> kvm_vcpu_scan_ioapic() can be called more often then vcpu_scan_ioapic()
Ok. I see your point. Maybe we need to rollback to old idea.

Can you pick the first two patches? If rollback to old way, it will not touch those code.

> 
>> And when delivering RTC interrupt:
>> if(need_eoi > 0 || in_recalculating)
>> 	return coalesced
>> 
>> 
>>> 
>>> --
>>> 			Gleb.
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> 
>> 
>> Best regards,
>> Yang
>> 
> 
> --
> 			Gleb.


Best regards,
Yang


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux