From: Yang Zhang <yang.zhang.wz@gmail.com>
To: "rkrcmar@redhat.com" <rkrcmar@redhat.com>,
"Wu, Feng" <feng.wu@intel.com>
Cc: "pbonzini@redhat.com" <pbonzini@redhat.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"kvm@vger.kernel.org" <kvm@vger.kernel.org>
Subject: Re: [PATCH v3 2/4] KVM: x86: Use vector-hashing to deliver lowest-priority interrupts
Date: Fri, 22 Jan 2016 12:00:12 +0800 [thread overview]
Message-ID: <56A1A94C.7010703@gmail.com> (raw)
In-Reply-To: <20160121172109.GB17514@potion.brq.redhat.com>
On 2016/1/22 1:21, rkrcmar@redhat.com wrote:
> 2016-01-21 05:33+0000, Wu, Feng:
>>> From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-
>>> owner@vger.kernel.org] On Behalf Of Yang Zhang
>>> On 2016/1/20 9:42, Feng Wu wrote:
>>>> + /*
>>>> + * We may find a hardware disabled LAPIC here, if
>>> that
>>>> + * is the case, print out a error message once for each
>>>> + * guest and return.
>>>> + */
>>>> + if (!dst[idx-1] &&
>>>> + (kvm->arch.disabled_lapic_found == 0)) {
>>>> + kvm->arch.disabled_lapic_found = 1;
>>>> + printk(KERN_ERR
>>>> + "Disabled LAPIC found during irq
>>> injection\n");
>>>> + goto out;
>>>
>>> What does "goto out" mean? Inject successfully or fail? According the
>>> value of ret which is set to ture here, it means inject successfully but
>
> (true actually means that fast path did the job and slow path isn't
> needed.)
>
>>> i = -1.
>
> (I think there isn't a practical difference between *r=-1 and *r=0.)
Currently, if *r == -1, the remote_irr may get set. But it seems wrong.
I need to have a double check to see whether it is a bug in current code.
>
>> Oh, I didn't notice 'ret' is initialized to true, I thought it was initialized
>> to false like another function, I should add a "ret = false' here. We should
>> failed to inject the interrupt since hardware disabled LAPIC is found.
>
> 'ret = true' is the better one. We know that the interrupt is not
> deliverable [1], so there's no point in trying to deliver with the slow
> path. We behave similarly when the interrupt targets a single disabled
> APIC.
>
> ---
> 1: Well ... it's possible that slowpath would deliver it thanks to
> different handling of disabled APICs, but it's undefined behavior,
why it is undefined behavior? Besides, why we will keep two different
handling logic for the fast path and slow path? It looks weird.
> so it doesn't matter matter if we don't try.
>
--
best regards
yang
next prev parent reply other threads:[~2016-01-22 4:00 UTC|newest]
Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-20 1:42 [PATCH v3 0/4] VT-d posted-interrupts follow ups Feng Wu
2016-01-20 1:42 ` [PATCH v3 1/4] KVM: Recover IRTE to remapped mode if the interrupt is not single-destination Feng Wu
2016-01-21 3:05 ` Yang Zhang
2016-01-21 3:14 ` Wu, Feng
2016-01-21 3:34 ` Yang Zhang
2016-01-21 4:42 ` Wu, Feng
2016-01-21 4:54 ` Tian, Kevin
2016-01-21 4:59 ` Yang Zhang
2016-01-21 5:07 ` Wu, Feng
2016-01-21 5:35 ` Yang Zhang
2016-01-21 5:41 ` Wu, Feng
2016-01-21 5:44 ` Yang Zhang
2016-01-21 16:35 ` rkrcmar
2016-01-22 2:03 ` Yang Zhang
2016-01-22 13:31 ` rkrcmar
2016-01-25 1:49 ` Yang Zhang
2016-01-25 13:59 ` rkrcmar
2016-01-26 1:44 ` Yang Zhang
2016-01-26 18:22 ` rkrcmar
2016-01-27 2:07 ` Yang Zhang
2016-01-27 15:05 ` rkrcmar
2016-01-21 16:19 ` Radim Krčmář
2016-01-22 1:49 ` Wu, Feng
2016-01-22 13:05 ` Radim Krcmár
2016-01-25 12:22 ` Paolo Bonzini
2016-01-25 12:26 ` Wu, Feng
2016-01-25 12:38 ` Paolo Bonzini
2016-01-25 12:48 ` Wu, Feng
2016-01-25 14:05 ` Radim Krcmár
2016-01-26 0:57 ` Wu, Feng
2016-01-20 1:42 ` [PATCH v3 2/4] KVM: x86: Use vector-hashing to deliver lowest-priority interrupts Feng Wu
2016-01-21 5:23 ` Yang Zhang
2016-01-21 5:33 ` Wu, Feng
2016-01-21 5:42 ` Yang Zhang
2016-01-21 5:46 ` Wu, Feng
2016-01-21 5:57 ` Yang Zhang
2016-01-21 6:02 ` Wu, Feng
2016-01-21 6:07 ` Yang Zhang
2016-01-21 17:21 ` rkrcmar
2016-01-22 2:01 ` Wu, Feng
2016-01-22 4:00 ` Yang Zhang [this message]
2016-01-22 13:49 ` rkrcmar
2016-01-21 19:49 ` Radim Krčmář
2016-01-22 5:12 ` Wu, Feng
2016-01-22 14:01 ` Radim Krcmár
2016-01-25 12:25 ` Paolo Bonzini
2016-01-25 15:20 ` Radim Krcmár
2016-01-25 16:14 ` Paolo Bonzini
2016-01-26 1:10 ` Wu, Feng
2016-01-20 1:42 ` [PATCH v3 3/4] KVM: x86: Add lowest-priority support for vt-d posted-interrupts Feng Wu
2016-01-21 20:16 ` Radim Krčmář
2016-01-22 5:12 ` Wu, Feng
2016-01-22 14:07 ` Radim Krcmár
2016-01-20 1:42 ` [PATCH v3 4/4] KVM/VMX: Add host irq information in trace event when updating IRTE for posted interrupts Feng Wu
2016-01-21 20:19 ` Radim Krčmář
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=56A1A94C.7010703@gmail.com \
--to=yang.zhang.wz@gmail.com \
--cc=feng.wu@intel.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=rkrcmar@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).