linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jan Kiszka <jan.kiszka@siemens.com>
To: Bandan Das <bsd@redhat.com>, Wanpeng Li <wanpeng.li@linux.intel.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Gleb Natapov <gleb@kernel.org>, Hu Robert <robert.hu@intel.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] KVM: nVMX: Fix IRQs inject to L2 which belong to L1 since race
Date: Thu, 03 Jul 2014 09:33:52 +0200	[thread overview]
Message-ID: <53B50760.3040708@siemens.com> (raw)
In-Reply-To: <jpg61jf9fis.fsf@redhat.com>

On 2014-07-03 07:29, Bandan Das wrote:
> Wanpeng Li <wanpeng.li@linux.intel.com> writes:
> 
>> Hi Bandan,
>> On Wed, Jul 02, 2014 at 12:27:59PM -0400, Bandan Das wrote:
>>> Wanpeng Li <wanpeng.li@linux.intel.com> writes:
>>>
>>>> This patch fix bug https://bugzilla.kernel.org/show_bug.cgi?id=72381 
>>> I can also reproduce this easily with Linux as L1 by "slowing it down"
>>> eg. running with ept = 0
>>>
>>> I suggest changing the subject to -
>>> KVM: nVMX: Fix race that incorrectly injects L1's irq to L2
>>>
>>
>> Ok, I will fold this to next version. ;-)
>>
>>>> If we didn't inject a still-pending event to L1 since nested_run_pending,
>>>> KVM_REQ_EVENT should be requested after the vmexit in order to inject the 
>>>> event to L1. However, current log blindly request a KVM_REQ_EVENT even if 
>>>
>>> What's current "log" ? Do you mean current "code" ?
>>>
>>
>> Yeah, it's a typo. I mean "logic".
>>
>> [...]
>>> Also, I am wondering isn't it enough to just do this to avoid this race ?
>>>
>>> static int vmx_interrupt_allowed(struct kvm_vcpu *vcpu)
>>> {
>>> -       return (!to_vmx(vcpu)->nested.nested_run_pending &&
>>> +       return (!is_guest_mode(vcpu) &&
>>> +               !to_vmx(vcpu)->nested.nested_run_pending &&
>>>                vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_IF) &&
>>>                !(vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) &
>>>
>>
>> I don't think you fix the root cause of the race, and there are two cases which 
>> I concern about your proposal:
>>
>> - If there is a special L1 which don't ask to exit on external intrs, you will 
>>   lose the intrs which L0 inject to L2.  
> 
> Oh didn't think about that case :), thanks for the pointing this out.
> It's easy to check this with Xen as L1, I suppose.

Xen most probably intercepts external interrupts, but Jailhouse
definitely does not. We also have a unit test for that, but I will
likely not expose the issue of lost events.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

      reply	other threads:[~2014-07-03  7:34 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-02  6:54 [PATCH] KVM: nVMX: Fix IRQs inject to L2 which belong to L1 since race Wanpeng Li
2014-07-02  7:20 ` Hu, Robert
2014-07-02  9:03   ` Jan Kiszka
2014-07-02  9:13     ` Hu, Robert
2014-07-02  9:16       ` Jan Kiszka
2014-07-02  9:01 ` Jan Kiszka
2014-07-03  2:59   ` Wanpeng Li
2014-07-03  5:15   ` Bandan Das
2014-07-03  6:59     ` Wanpeng Li
2014-07-03 17:27       ` Bandan Das
2014-07-04  2:52         ` Wanpeng Li
2014-07-04  5:43           ` Jan Kiszka
2014-07-04  6:08             ` Wanpeng Li
2014-07-04  7:19               ` Jan Kiszka
2014-07-04  7:39                 ` Wanpeng Li
2014-07-04  7:46                   ` Paolo Bonzini
2014-07-04  7:59                     ` Wanpeng Li
2014-07-04  8:14                       ` Paolo Bonzini
2014-07-04  7:42             ` Paolo Bonzini
2014-07-04  9:33             ` Jan Kiszka
2014-07-04  9:38               ` Paolo Bonzini
2014-07-04 10:52                 ` Jan Kiszka
2014-07-04 11:07                   ` Jan Kiszka
2014-07-04 11:28                     ` Paolo Bonzini
2014-07-04  6:17     ` Wanpeng Li
2014-07-04  7:21       ` Jan Kiszka
2014-07-07  0:56       ` Bandan Das
2014-07-07  8:46         ` Wanpeng Li
2014-07-07 13:03           ` Paolo Bonzini
2014-07-07 17:31             ` Bandan Das
2014-07-07 17:34               ` Paolo Bonzini
2014-07-07 17:38                 ` Bandan Das
2014-07-07 23:14                   ` Wanpeng Li
2014-07-08  4:35                     ` Bandan Das
2014-07-07 23:38             ` Wanpeng Li
2014-07-08  5:49               ` Paolo Bonzini
2014-07-02 16:27 ` Bandan Das
2014-07-03  5:11   ` Wanpeng Li
2014-07-03  5:29     ` Bandan Das
2014-07-03  7:33       ` Jan Kiszka [this message]

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=53B50760.3040708@siemens.com \
    --to=jan.kiszka@siemens.com \
    --cc=bsd@redhat.com \
    --cc=gleb@kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=robert.hu@intel.com \
    --cc=wanpeng.li@linux.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 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).