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 09:04:57 +0200 Message-ID: <20130321070457.GN3889@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> 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]:35719 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751515Ab3CUHFA (ORCPT ); Thu, 21 Mar 2013 03:05:00 -0400 Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On Thu, Mar 21, 2013 at 07:02:39AM +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. > How about build it in kvm_apic_set_irq()? It should be more straightforward. > Sure, pass a pointer there. Just do not access ioapic directly. -- Gleb.