Message ID | 20170918015650.16973-2-haozhong.zhang@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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>"?
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
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.
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 --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",
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(-)