diff mbox

[1/2] KVM: VMX: do not change SN bit in vmx_update_pi_irte()

Message ID 20170918015650.16973-2-haozhong.zhang@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Haozhong Zhang Sept. 18, 2017, 1:56 a.m. UTC
In kvm_vcpu_trigger_posted_interrupt() and pi_pre_block(), KVM
assumes that PI notification events should not be suppressed when the
target vCPU is not blocked.

vmx_update_pi_irte() sets the SN field before changing an interrupt
from posting to remapping, but it does not check the vCPU mode.
Therefore, the change of SN field may break above the assumption.
Besides, I don't see reasons to suppress notification events here, so
remove the changes of SN field to avoid race condition.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
Reported-by: "Ramamurthy, Venkatesh" <venkatesh.ramamurthy@intel.com>
Reported-by: Dan Williams <dan.j.williams@intel.com>
---
 arch/x86/kvm/vmx.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

Comments

Dan Williams Sept. 26, 2017, 12:33 a.m. UTC | #1
On Sun, Sep 17, 2017 at 6:56 PM, Haozhong Zhang
<haozhong.zhang@intel.com> wrote:
> In kvm_vcpu_trigger_posted_interrupt() and pi_pre_block(), KVM
> assumes that PI notification events should not be suppressed when the
> target vCPU is not blocked.
>
> vmx_update_pi_irte() sets the SN field before changing an interrupt
> from posting to remapping, but it does not check the vCPU mode.
> Therefore, the change of SN field may break above the assumption.
> Besides, I don't see reasons to suppress notification events here, so
> remove the changes of SN field to avoid race condition.
>
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

Missing a "Fixes:" line and a "Cc: <stable@vger.kernel.org>"?
Haozhong Zhang Sept. 26, 2017, 12:45 a.m. UTC | #2
On 09/25/17 17:33 -0700, Dan Williams wrote:
> On Sun, Sep 17, 2017 at 6:56 PM, Haozhong Zhang
> <haozhong.zhang@intel.com> wrote:
> > In kvm_vcpu_trigger_posted_interrupt() and pi_pre_block(), KVM
> > assumes that PI notification events should not be suppressed when the
> > target vCPU is not blocked.
> >
> > vmx_update_pi_irte() sets the SN field before changing an interrupt
> > from posting to remapping, but it does not check the vCPU mode.
> > Therefore, the change of SN field may break above the assumption.
> > Besides, I don't see reasons to suppress notification events here, so
> > remove the changes of SN field to avoid race condition.
> >
> > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> 
> Missing a "Fixes:" line and a "Cc: <stable@vger.kernel.org>"?

"Fixes" was added when these two patches were committed. I cc'ed to
stable mailing list when sent these two patches.

Haozhong
Dan Williams Sept. 26, 2017, 12:54 a.m. UTC | #3
On Mon, Sep 25, 2017 at 5:45 PM, Haozhong Zhang
<haozhong.zhang@intel.com> wrote:
> On 09/25/17 17:33 -0700, Dan Williams wrote:
>> On Sun, Sep 17, 2017 at 6:56 PM, Haozhong Zhang
>> <haozhong.zhang@intel.com> wrote:
>> > In kvm_vcpu_trigger_posted_interrupt() and pi_pre_block(), KVM
>> > assumes that PI notification events should not be suppressed when the
>> > target vCPU is not blocked.
>> >
>> > vmx_update_pi_irte() sets the SN field before changing an interrupt
>> > from posting to remapping, but it does not check the vCPU mode.
>> > Therefore, the change of SN field may break above the assumption.
>> > Besides, I don't see reasons to suppress notification events here, so
>> > remove the changes of SN field to avoid race condition.
>> >
>> > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
>>
>> Missing a "Fixes:" line and a "Cc: <stable@vger.kernel.org>"?
>
> "Fixes" was added when these two patches were committed. I cc'ed to
> stable mailing list when sent these two patches.

Yes, but that doesn't notify the stable team to do the backport. You
actually need to put "Cc: <stable@vger.kernel.org>" in the patch
directly so the automatic scripts pick it up when it hits mainline.

Otherwise, you need to send the patch to the stable with the upstream
commit id noted at the top of the patch.  See the options in
Documentation/process/stable-kernel-rules.rst.
Haozhong Zhang Sept. 26, 2017, 1:40 a.m. UTC | #4
On 09/25/17 17:54 -0700, Dan Williams wrote:
> On Mon, Sep 25, 2017 at 5:45 PM, Haozhong Zhang
> <haozhong.zhang@intel.com> wrote:
> > On 09/25/17 17:33 -0700, Dan Williams wrote:
> >> On Sun, Sep 17, 2017 at 6:56 PM, Haozhong Zhang
> >> <haozhong.zhang@intel.com> wrote:
> >> > In kvm_vcpu_trigger_posted_interrupt() and pi_pre_block(), KVM
> >> > assumes that PI notification events should not be suppressed when the
> >> > target vCPU is not blocked.
> >> >
> >> > vmx_update_pi_irte() sets the SN field before changing an interrupt
> >> > from posting to remapping, but it does not check the vCPU mode.
> >> > Therefore, the change of SN field may break above the assumption.
> >> > Besides, I don't see reasons to suppress notification events here, so
> >> > remove the changes of SN field to avoid race condition.
> >> >
> >> > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> >>
> >> Missing a "Fixes:" line and a "Cc: <stable@vger.kernel.org>"?
> >
> > "Fixes" was added when these two patches were committed. I cc'ed to
> > stable mailing list when sent these two patches.
> 
> Yes, but that doesn't notify the stable team to do the backport. You
> actually need to put "Cc: <stable@vger.kernel.org>" in the patch
> directly so the automatic scripts pick it up when it hits mainline.
> 
> Otherwise, you need to send the patch to the stable with the upstream
> commit id noted at the top of the patch.  See the options in
> Documentation/process/stable-kernel-rules.rst.

Got it. I'll send them to the stable mailing list with upstream commit ids.

Thanks,
Haozhong
diff mbox

Patch

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index a406afbb6d21..fb611847ea99 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -11911,12 +11911,8 @@  static int vmx_update_pi_irte(struct kvm *kvm, unsigned int host_irq,
 
 		if (set)
 			ret = irq_set_vcpu_affinity(host_irq, &vcpu_info);
-		else {
-			/* suppress notification event before unposting */
-			pi_set_sn(vcpu_to_pi_desc(vcpu));
+		else
 			ret = irq_set_vcpu_affinity(host_irq, NULL);
-			pi_clear_sn(vcpu_to_pi_desc(vcpu));
-		}
 
 		if (ret < 0) {
 			printk(KERN_INFO "%s: failed to update PI IRTE\n",