From: Gleb Natapov <gleb@redhat.com>
To: "Zhang, Yang Z" <yang.z.zhang@intel.com>
Cc: "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
"mtosatti@redhat.com" <mtosatti@redhat.com>,
"Zhang, Xiantao" <xiantao.zhang@intel.com>
Subject: Re: [PATCH v5 3/6] KVM : Calculate destination vcpu on interrupt injection
Date: Thu, 21 Mar 2013 14:03:44 +0200 [thread overview]
Message-ID: <20130321120344.GQ3889@redhat.com> (raw)
In-Reply-To: <A9667DDFB95DB7438FA9D7D576C3D87E099EA1C3@SHSMSX101.ccr.corp.intel.com>
On Thu, Mar 21, 2013 at 11:56:05AM +0000, Zhang, Yang Z wrote:
> Gleb Natapov wrote on 2013-03-21:
> > On Thu, Mar 21, 2013 at 06:49:21PM +0800, Yang Zhang wrote:
> >> From: Yang Zhang <yang.z.zhang@Intel.com>
> >>
> >> Add a new parameter to know vcpus who received the interrupt.
> >>
> >> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> >> ---
> >> arch/x86/kvm/lapic.c | 21 ++++++++++++++++-----
> >> arch/x86/kvm/lapic.h | 5 +++--
> >> virt/kvm/ioapic.c | 2 +-
> >> virt/kvm/ioapic.h | 2 +-
> >> virt/kvm/irq_comm.c | 12 ++++++------
> >> 5 files changed, 27 insertions(+), 15 deletions(-)
> >> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> >> index d3e322a..5f6b1d0 100644
> >> --- a/arch/x86/kvm/lapic.c
> >> +++ b/arch/x86/kvm/lapic.c
> >> @@ -433,10 +433,21 @@ int kvm_lapic_find_highest_irr(struct kvm_vcpu
> > *vcpu)
> >> static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
> >> int vector, int level, int trig_mode);
> >> -int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq)
> >> +static void kvm_set_irq_dest_map(struct kvm_vcpu *vcpu, unsigned long
> >> *dest_map) +{ + if (!kvm_lapic_enabled(vcpu)) + return;
> > Why this check here?
> The vcpu who didn't enable apic should not account as destination vcpu.
> Without this check, if broadcast interrupt, all cpus will treat as destination vcpu, but only those who enabled apic will receive the interrupt.
> There are same check in __apic_accept_irq():
> if (unlikely(!apic_enabled(apic)))
> break;
I see, but you use more strict check that also checks that apic is
emulated by the kernel and we wouldn't be here if it wasn't. Anyway lets
move bitmap update into __apic_accept_irq().
>
> >> + __set_bit(vcpu->vcpu_id, dest_map);
> >> +}
> >> +
> >> +int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
> >> + unsigned long *dest_map)
> >> {
> >> struct kvm_lapic *apic = vcpu->arch.apic;
> >> + if (dest_map)
> >> + kvm_set_irq_dest_map(vcpu, dest_map);
> >> +
> >> return __apic_accept_irq(apic, irq->delivery_mode, irq->vector,
> >> irq->level, irq->trig_mode);
> >> }
> >> @@ -611,7 +622,7 @@ int kvm_apic_match_dest(struct kvm_vcpu *vcpu,
> > struct kvm_lapic *source,
> >> }
> >>
> >> bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
> >> - struct kvm_lapic_irq *irq, int *r)
> >> + struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map)
> >> {
> >> struct kvm_apic_map *map;
> >> unsigned long bitmap = 1;
> >> @@ -622,7 +633,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm,
> > struct kvm_lapic *src,
> >> *r = -1;
> >>
> >> if (irq->shorthand == APIC_DEST_SELF) {
> >> - *r = kvm_apic_set_irq(src->vcpu, irq);
> >> + *r = kvm_apic_set_irq(src->vcpu, irq, dest_map);
> >> return true;
> >> }
> >> @@ -667,7 +678,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm,
> > struct kvm_lapic *src,
> >> continue;
> >> if (*r < 0)
> >> *r = 0;
> >> - *r += kvm_apic_set_irq(dst[i]->vcpu, irq);
> >> + *r += kvm_apic_set_irq(dst[i]->vcpu, irq, dest_map);
> >> }
> >>
> >> ret = true; @@ -852,7 +863,7 @@ static void apic_send_ipi(struct
> >> kvm_lapic *apic) irq.trig_mode, irq.level, irq.dest_mode,
> >> irq.delivery_mode, irq.vector);
> >> - kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq);
> >> + kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq, NULL);
> >> }
> >>
> >> static u32 apic_get_tmcct(struct kvm_lapic *apic)
> >> diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
> >> index 2c721b9..967519c 100644
> >> --- a/arch/x86/kvm/lapic.h
> >> +++ b/arch/x86/kvm/lapic.h
> >> @@ -55,11 +55,12 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu);
> >>
> >> int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest);
> >> int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u8 mda);
> >> -int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq);
> >> +int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
> >> + unsigned long *dest_map);
> >> int kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type);
> >>
> >> bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
> >> - struct kvm_lapic_irq *irq, int *r);
> >> + struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map);
> >>
> >> u64 kvm_get_apic_base(struct kvm_vcpu *vcpu);
> >> void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data);
> >> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
> >> index ed6f111..4767fa6 100644
> >> --- a/virt/kvm/ioapic.c
> >> +++ b/virt/kvm/ioapic.c
> >> @@ -217,7 +217,7 @@ static int ioapic_deliver(struct kvm_ioapic *ioapic, int
> > irq)
> >> irqe.level = 1;
> >> irqe.shorthand = 0;
> >> - return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe);
> >> + return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe, NULL);
> >> }
> >>
> >> int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int irq_source_id,
> >> diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
> >> index 6e5c88f..14e5289 100644
> >> --- a/virt/kvm/ioapic.h
> >> +++ b/virt/kvm/ioapic.h
> >> @@ -88,7 +88,7 @@ int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq,
> > int irq_source_id,
> >> void kvm_ioapic_clear_all(struct kvm_ioapic *ioapic, int irq_source_id);
> >> void kvm_ioapic_reset(struct kvm_ioapic *ioapic);
> >> int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
> >> - struct kvm_lapic_irq *irq);
> >> + struct kvm_lapic_irq *irq, unsigned long *dest_map);
> >> int kvm_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
> >> int kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
> >> void kvm_ioapic_make_eoibitmap_request(struct kvm *kvm);
> >> diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
> >> index e9073cf..2f07d2e 100644
> >> --- a/virt/kvm/irq_comm.c
> >> +++ b/virt/kvm/irq_comm.c
> >> @@ -63,7 +63,7 @@ inline static bool kvm_is_dm_lowest_prio(struct
> > kvm_lapic_irq *irq)
> >> }
> >>
> >> int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
> >> - struct kvm_lapic_irq *irq)
> >> + struct kvm_lapic_irq *irq, unsigned long *dest_map)
> >> {
> >> int i, r = -1;
> >> struct kvm_vcpu *vcpu, *lowest = NULL;
> >> @@ -74,7 +74,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
> > kvm_lapic *src,
> >> irq->delivery_mode = APIC_DM_FIXED;
> >> }
> >> - if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, &r))
> >> + if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, &r, dest_map))
> >> return r;
> >>
> >> kvm_for_each_vcpu(i, vcpu, kvm) {
> >> @@ -88,7 +88,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
> > kvm_lapic *src,
> >> if (!kvm_is_dm_lowest_prio(irq)) {
> >> if (r < 0)
> >> r = 0;
> >> - r += kvm_apic_set_irq(vcpu, irq);
> >> + r += kvm_apic_set_irq(vcpu, irq, dest_map);
> >> } else if (kvm_lapic_enabled(vcpu)) {
> >> if (!lowest)
> >> lowest = vcpu;
> >> @@ -98,7 +98,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
> > kvm_lapic *src,
> >> }
> >>
> >> if (lowest)
> >> - r = kvm_apic_set_irq(lowest, irq);
> >> + r = kvm_apic_set_irq(lowest, irq, dest_map);
> >>
> >> return r;
> >> }
> >> @@ -130,7 +130,7 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry
> >> *e,
> >>
> >> kvm_set_msi_irq(e, &irq);
> >> - return kvm_irq_delivery_to_apic(kvm, NULL, &irq);
> >> + return kvm_irq_delivery_to_apic(kvm, NULL, &irq, NULL);
> >> }
> >>
> >> @@ -142,7 +142,7 @@ static int kvm_set_msi_inatomic(struct
> >> kvm_kernel_irq_routing_entry *e,
> >>
> >> kvm_set_msi_irq(e, &irq);
> >> - if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, &r))
> >> + if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, &r, NULL))
> >> return r;
> >> else
> >> return -EWOULDBLOCK;
> >> --
> >> 1.7.1
> >
> > --
> > Gleb.
>
>
> Best regards,
> Yang
>
--
Gleb.
next prev parent reply other threads:[~2013-03-21 12:03 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-21 10:49 [PATCH v5 0/6] Use eoi to track RTC interrupt delivery status Yang Zhang
2013-03-21 10:49 ` [PATCH v5 1/6] KVM: Add vcpu info to ioapic_update_eoi() Yang Zhang
2013-03-21 10:49 ` [PATCH v5 2/6] KVM: Introduce struct rtc_status Yang Zhang
2013-03-21 12:08 ` Gleb Natapov
2013-03-21 12:15 ` Zhang, Yang Z
2013-03-21 10:49 ` [PATCH v5 3/6] KVM : Calculate destination vcpu on interrupt injection Yang Zhang
2013-03-21 11:48 ` Gleb Natapov
2013-03-21 11:56 ` Zhang, Yang Z
2013-03-21 12:03 ` Gleb Natapov [this message]
2013-03-21 12:12 ` Zhang, Yang Z
2013-03-21 12:21 ` Gleb Natapov
2013-03-21 12:25 ` Zhang, Yang Z
2013-03-21 10:49 ` [PATCH v5 4/6] KVM: Add reset/restore rtc_status support Yang Zhang
2013-03-22 10:16 ` Marcelo Tosatti
2013-03-22 10:47 ` Zhang, Yang Z
2013-03-21 10:49 ` [PATCH v5 5/6] KVM: Force vmexit with virtual interrupt delivery Yang Zhang
2013-03-21 10:49 ` [PATCH v5 6/6] KVM: Use eoi to track RTC interrupt delivery status Yang Zhang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130321120344.GQ3889@redhat.com \
--to=gleb@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=mtosatti@redhat.com \
--cc=xiantao.zhang@intel.com \
--cc=yang.z.zhang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.