From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCH v4 5/7] KVM: Recalculate destination vcpu map Date: Thu, 21 Mar 2013 07:34:22 +0200 Message-ID: <20130321053422.GE9382@redhat.com> References: <1363779379-20212-1-git-send-email-yang.z.zhang@intel.com> <1363779379-20212-6-git-send-email-yang.z.zhang@intel.com> <20130320152245.GL3889@redhat.com> <20130321050808.GD9382@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: "kvm@vger.kernel.org" , "mtosatti@redhat.com" , "Zhang, Xiantao" To: "Zhang, Yang Z" Return-path: Received: from mx1.redhat.com ([209.132.183.28]:20084 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752496Ab3CUFeY (ORCPT ); Thu, 21 Mar 2013 01:34:24 -0400 Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: 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 > >>>> > >>>> 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 > >>>> --- > >>>> 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() > 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@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > Best regards, > Yang > -- Gleb.