| Message ID | 7fec7586-a7c6-59dd-7653-2f88de75fbe4@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> -----Original Message----- > From: Yang Zhang [mailto:yang.zhang.wz@gmail.com] > Sent: Friday, May 20, 2016 3:51 PM > To: Paolo Bonzini <pbonzini@redhat.com>; rkrcmar@redhat.com; Wu, Feng > <feng.wu@intel.com> > Cc: kvm@vger.kernel.org > Subject: [PATCH] KVM: VMX: check apicv is active before using VT-d posted > interrupt > > VT-d posted interrupt is relying on the CPU side's posted interrupt. > Need to check whether VCPU's APICv is active before enabing VT-d posted > interrupt. Yes, the extra check is good. Just curious, Do you see some hardware with VT-d PI but no CPU side PI. I remember I was told this cannot happen in real world. Thanks, feng > > Signed-off-by: Yang Zhang <yang.zhang.wz@gmail.com> > Signed-off-by: Shengge Ding <shengge.dsg@alibaba-inc.com> > --- > arch/x86/kvm/vmx.c | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 7ebf27b..8ba87f3 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -2072,7 +2072,8 @@ static void vmx_vcpu_pi_load(struct kvm_vcpu > *vcpu, int cpu) > unsigned int dest; > > if (!kvm_arch_has_assigned_device(vcpu->kvm) || > - !irq_remapping_cap(IRQ_POSTING_CAP)) > + !irq_remapping_cap(IRQ_POSTING_CAP) || > + !kvm_vcpu_apicv_active(vcpu)) > return; > > do { > @@ -2180,7 +2181,8 @@ static void vmx_vcpu_pi_put(struct kvm_vcpu *vcpu) > struct pi_desc *pi_desc = vcpu_to_pi_desc(vcpu); > > if (!kvm_arch_has_assigned_device(vcpu->kvm) || > - !irq_remapping_cap(IRQ_POSTING_CAP)) > + !irq_remapping_cap(IRQ_POSTING_CAP) || > + !kvm_vcpu_apicv_active(vcpu)) > return; > > /* Set SN when the vCPU is preempted */ > @@ -10698,7 +10700,8 @@ static int vmx_pre_block(struct kvm_vcpu *vcpu) > struct pi_desc *pi_desc = vcpu_to_pi_desc(vcpu); > > if (!kvm_arch_has_assigned_device(vcpu->kvm) || > - !irq_remapping_cap(IRQ_POSTING_CAP)) > + !irq_remapping_cap(IRQ_POSTING_CAP) || > + !kvm_vcpu_apicv_active(vcpu)) > return 0; > > vcpu->pre_pcpu = vcpu->cpu; > @@ -10764,7 +10767,8 @@ static void vmx_post_block(struct kvm_vcpu *vcpu) > unsigned long flags; > > if (!kvm_arch_has_assigned_device(vcpu->kvm) || > - !irq_remapping_cap(IRQ_POSTING_CAP)) > + !irq_remapping_cap(IRQ_POSTING_CAP) || > + !kvm_vcpu_apicv_active(vcpu)) > return; > > do { > @@ -10817,7 +10821,8 @@ static int vmx_update_pi_irte(struct kvm *kvm, > unsigned int host_irq, > int idx, ret = -EINVAL; > > if (!kvm_arch_has_assigned_device(kvm) || > - !irq_remapping_cap(IRQ_POSTING_CAP)) > + !irq_remapping_cap(IRQ_POSTING_CAP) || > + !kvm_vcpu_apicv_active(kvm->vcpus[0])) > return 0; > > idx = srcu_read_lock(&kvm->irq_srcu); > -- > 1.8.2.1
On 2016/5/20 16:22, Wu, Feng wrote: > > >> -----Original Message----- >> From: Yang Zhang [mailto:yang.zhang.wz@gmail.com] >> Sent: Friday, May 20, 2016 3:51 PM >> To: Paolo Bonzini <pbonzini@redhat.com>; rkrcmar@redhat.com; Wu, Feng >> <feng.wu@intel.com> >> Cc: kvm@vger.kernel.org >> Subject: [PATCH] KVM: VMX: check apicv is active before using VT-d posted >> interrupt >> >> VT-d posted interrupt is relying on the CPU side's posted interrupt. >> Need to check whether VCPU's APICv is active before enabing VT-d posted >> interrupt. > > Yes, the extra check is good. Just curious, Do you see some hardware with > VT-d PI but no CPU side PI. I remember I was told this cannot happen in > real world. Hardware is ok. The problem is user can turn off the APICv manually which cause the device fails to work inside guest. > > Thanks, > feng > >> >> Signed-off-by: Yang Zhang <yang.zhang.wz@gmail.com> >> Signed-off-by: Shengge Ding <shengge.dsg@alibaba-inc.com> >> --- >> arch/x86/kvm/vmx.c | 15 ++++++++++----- >> 1 file changed, 10 insertions(+), 5 deletions(-) >> >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> index 7ebf27b..8ba87f3 100644 >> --- a/arch/x86/kvm/vmx.c >> +++ b/arch/x86/kvm/vmx.c >> @@ -2072,7 +2072,8 @@ static void vmx_vcpu_pi_load(struct kvm_vcpu >> *vcpu, int cpu) >> unsigned int dest; >> >> if (!kvm_arch_has_assigned_device(vcpu->kvm) || >> - !irq_remapping_cap(IRQ_POSTING_CAP)) >> + !irq_remapping_cap(IRQ_POSTING_CAP) || >> + !kvm_vcpu_apicv_active(vcpu)) >> return; >> >> do { >> @@ -2180,7 +2181,8 @@ static void vmx_vcpu_pi_put(struct kvm_vcpu *vcpu) >> struct pi_desc *pi_desc = vcpu_to_pi_desc(vcpu); >> >> if (!kvm_arch_has_assigned_device(vcpu->kvm) || >> - !irq_remapping_cap(IRQ_POSTING_CAP)) >> + !irq_remapping_cap(IRQ_POSTING_CAP) || >> + !kvm_vcpu_apicv_active(vcpu)) >> return; >> >> /* Set SN when the vCPU is preempted */ >> @@ -10698,7 +10700,8 @@ static int vmx_pre_block(struct kvm_vcpu *vcpu) >> struct pi_desc *pi_desc = vcpu_to_pi_desc(vcpu); >> >> if (!kvm_arch_has_assigned_device(vcpu->kvm) || >> - !irq_remapping_cap(IRQ_POSTING_CAP)) >> + !irq_remapping_cap(IRQ_POSTING_CAP) || >> + !kvm_vcpu_apicv_active(vcpu)) >> return 0; >> >> vcpu->pre_pcpu = vcpu->cpu; >> @@ -10764,7 +10767,8 @@ static void vmx_post_block(struct kvm_vcpu *vcpu) >> unsigned long flags; >> >> if (!kvm_arch_has_assigned_device(vcpu->kvm) || >> - !irq_remapping_cap(IRQ_POSTING_CAP)) >> + !irq_remapping_cap(IRQ_POSTING_CAP) || >> + !kvm_vcpu_apicv_active(vcpu)) >> return; >> >> do { >> @@ -10817,7 +10821,8 @@ static int vmx_update_pi_irte(struct kvm *kvm, >> unsigned int host_irq, >> int idx, ret = -EINVAL; >> >> if (!kvm_arch_has_assigned_device(kvm) || >> - !irq_remapping_cap(IRQ_POSTING_CAP)) >> + !irq_remapping_cap(IRQ_POSTING_CAP) || >> + !kvm_vcpu_apicv_active(kvm->vcpus[0])) >> return 0; >> >> idx = srcu_read_lock(&kvm->irq_srcu); >> -- >> 1.8.2.1
On 20/05/2016 09:50, Yang Zhang wrote: > VT-d posted interrupt is relying on the CPU side's posted interrupt. > Need to check whether VCPU's APICv is active before enabing VT-d posted > interrupt. > > Signed-off-by: Yang Zhang <yang.zhang.wz@gmail.com> > Signed-off-by: Shengge Ding <shengge.dsg@alibaba-inc.com> Please resend this patch without format=flowed (just use git-send-email), as it cannot be applied otherwise. Thanks, Paolo > --- > arch/x86/kvm/vmx.c | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 7ebf27b..8ba87f3 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -2072,7 +2072,8 @@ static void vmx_vcpu_pi_load(struct kvm_vcpu > *vcpu, int cpu) > unsigned int dest; > > if (!kvm_arch_has_assigned_device(vcpu->kvm) || > - !irq_remapping_cap(IRQ_POSTING_CAP)) > + !irq_remapping_cap(IRQ_POSTING_CAP) || > + !kvm_vcpu_apicv_active(vcpu)) > return; > > do { > @@ -2180,7 +2181,8 @@ static void vmx_vcpu_pi_put(struct kvm_vcpu *vcpu) > struct pi_desc *pi_desc = vcpu_to_pi_desc(vcpu); > > if (!kvm_arch_has_assigned_device(vcpu->kvm) || > - !irq_remapping_cap(IRQ_POSTING_CAP)) > + !irq_remapping_cap(IRQ_POSTING_CAP) || > + !kvm_vcpu_apicv_active(vcpu)) > return; > > /* Set SN when the vCPU is preempted */ > @@ -10698,7 +10700,8 @@ static int vmx_pre_block(struct kvm_vcpu *vcpu) > struct pi_desc *pi_desc = vcpu_to_pi_desc(vcpu); > > if (!kvm_arch_has_assigned_device(vcpu->kvm) || > - !irq_remapping_cap(IRQ_POSTING_CAP)) > + !irq_remapping_cap(IRQ_POSTING_CAP) || > + !kvm_vcpu_apicv_active(vcpu)) > return 0; > > vcpu->pre_pcpu = vcpu->cpu; > @@ -10764,7 +10767,8 @@ static void vmx_post_block(struct kvm_vcpu *vcpu) > unsigned long flags; > > if (!kvm_arch_has_assigned_device(vcpu->kvm) || > - !irq_remapping_cap(IRQ_POSTING_CAP)) > + !irq_remapping_cap(IRQ_POSTING_CAP) || > + !kvm_vcpu_apicv_active(vcpu)) > return; > > do { > @@ -10817,7 +10821,8 @@ static int vmx_update_pi_irte(struct kvm *kvm, > unsigned int host_irq, > int idx, ret = -EINVAL; > > if (!kvm_arch_has_assigned_device(kvm) || > - !irq_remapping_cap(IRQ_POSTING_CAP)) > + !irq_remapping_cap(IRQ_POSTING_CAP) || > + !kvm_vcpu_apicv_active(kvm->vcpus[0])) > return 0; > > idx = srcu_read_lock(&kvm->irq_srcu); -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2016/5/23 22:23, Paolo Bonzini wrote: > On 20/05/2016 09:50, Yang Zhang wrote: >> VT-d posted interrupt is relying on the CPU side's posted interrupt. >> Need to check whether VCPU's APICv is active before enabing VT-d posted >> interrupt. >> >> Signed-off-by: Yang Zhang <yang.zhang.wz@gmail.com> >> Signed-off-by: Shengge Ding <shengge.dsg@alibaba-inc.com> > > Please resend this patch without format=flowed (just use > git-send-email), as it cannot be applied otherwise. Sorry, my git proxy is broken recently. I will resend it ASAP once the problem got fixed. > > Thanks, > > Paolo > >> --- >> arch/x86/kvm/vmx.c | 15 ++++++++++----- >> 1 file changed, 10 insertions(+), 5 deletions(-) >> >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> index 7ebf27b..8ba87f3 100644 >> --- a/arch/x86/kvm/vmx.c >> +++ b/arch/x86/kvm/vmx.c >> @@ -2072,7 +2072,8 @@ static void vmx_vcpu_pi_load(struct kvm_vcpu >> *vcpu, int cpu) >> unsigned int dest; >> >> if (!kvm_arch_has_assigned_device(vcpu->kvm) || >> - !irq_remapping_cap(IRQ_POSTING_CAP)) >> + !irq_remapping_cap(IRQ_POSTING_CAP) || >> + !kvm_vcpu_apicv_active(vcpu)) >> return; >> >> do { >> @@ -2180,7 +2181,8 @@ static void vmx_vcpu_pi_put(struct kvm_vcpu *vcpu) >> struct pi_desc *pi_desc = vcpu_to_pi_desc(vcpu); >> >> if (!kvm_arch_has_assigned_device(vcpu->kvm) || >> - !irq_remapping_cap(IRQ_POSTING_CAP)) >> + !irq_remapping_cap(IRQ_POSTING_CAP) || >> + !kvm_vcpu_apicv_active(vcpu)) >> return; >> >> /* Set SN when the vCPU is preempted */ >> @@ -10698,7 +10700,8 @@ static int vmx_pre_block(struct kvm_vcpu *vcpu) >> struct pi_desc *pi_desc = vcpu_to_pi_desc(vcpu); >> >> if (!kvm_arch_has_assigned_device(vcpu->kvm) || >> - !irq_remapping_cap(IRQ_POSTING_CAP)) >> + !irq_remapping_cap(IRQ_POSTING_CAP) || >> + !kvm_vcpu_apicv_active(vcpu)) >> return 0; >> >> vcpu->pre_pcpu = vcpu->cpu; >> @@ -10764,7 +10767,8 @@ static void vmx_post_block(struct kvm_vcpu *vcpu) >> unsigned long flags; >> >> if (!kvm_arch_has_assigned_device(vcpu->kvm) || >> - !irq_remapping_cap(IRQ_POSTING_CAP)) >> + !irq_remapping_cap(IRQ_POSTING_CAP) || >> + !kvm_vcpu_apicv_active(vcpu)) >> return; >> >> do { >> @@ -10817,7 +10821,8 @@ static int vmx_update_pi_irte(struct kvm *kvm, >> unsigned int host_irq, >> int idx, ret = -EINVAL; >> >> if (!kvm_arch_has_assigned_device(kvm) || >> - !irq_remapping_cap(IRQ_POSTING_CAP)) >> + !irq_remapping_cap(IRQ_POSTING_CAP) || >> + !kvm_vcpu_apicv_active(kvm->vcpus[0])) >> return 0; >> >> idx = srcu_read_lock(&kvm->irq_srcu);
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 7ebf27b..8ba87f3 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -2072,7 +2072,8 @@ static void vmx_vcpu_pi_load(struct kvm_vcpu *vcpu, int cpu) unsigned int dest; if (!kvm_arch_has_assigned_device(vcpu->kvm) || - !irq_remapping_cap(IRQ_POSTING_CAP)) + !irq_remapping_cap(IRQ_POSTING_CAP) || + !kvm_vcpu_apicv_active(vcpu)) return;