Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts
@ 2019-10-09 12:52 Roger Pau Monne
  2019-10-09 13:35 ` Jan Beulich
  0 siblings, 1 reply; 3+ messages in thread
From: Roger Pau Monne @ 2019-10-09 12:52 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, Kevin Tian, Wei Liu, Andrew Cooper, Joe Jin,
	Jan Beulich, Roger Pau Monne

When using posted interrupts and the guest migrates MSI from vCPUs Xen
needs to flush any pending PIRR vectors on the previous vCPU, or else
those vectors could get wrongly injected at a later point when the MSI
fields are already updated.

Rename sync_pir_to_irr to vlapic_sync_pir_to_irr and export it so it
can be called when updating the posted interrupt descriptor field in
pi_update_irte. While there also remove the unlock_out from
pi_update_irte, it's used in a single goto and removing it makes the
function smaller.

Note that PIRR is synced to IRR both in pt_irq_destroy_bind and
pt_irq_create_bind when the interrupt delivery data is being updated.

Also store the vCPU ID in multi-destination mode when using posted
interrupts and the interrupt is bound to a single vCPU in order for
posted interrupts to be used.

While there guard pi_update_irte with CONFIG_HVM since it's only used
with HVM guests.

Reported-by: Joe Jin <joe.jin@oracle.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Joe Jin <joe.jin@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
---
I would like to see a bug fix for this issue in 4.13. The fix here only
affects posted interrupts, hence I think the risk of breaking anything
else is low.
---
Changes since v1:
 - Store the vcpu id also in multi-dest mode if the interrupt is bound
   to a vcpu for posted delivery.
 - s/#if/#ifdef/.
---
 xen/arch/x86/hvm/vlapic.c              |  6 +++---
 xen/drivers/passthrough/io.c           | 13 ++++++++++---
 xen/drivers/passthrough/vtd/intremap.c | 15 ++++++++-------
 xen/include/asm-x86/hvm/vlapic.h       |  2 ++
 xen/include/asm-x86/iommu.h            |  2 +-
 5 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index 9466258d6f..d255ad8db7 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -106,7 +106,7 @@ static void vlapic_clear_irr(int vector, struct vlapic *vlapic)
     vlapic_clear_vector(vector, &vlapic->regs->data[APIC_IRR]);
 }
 
-static void sync_pir_to_irr(struct vcpu *v)
+void vlapic_sync_pir_to_irr(struct vcpu *v)
 {
     if ( hvm_funcs.sync_pir_to_irr )
         alternative_vcall(hvm_funcs.sync_pir_to_irr, v);
@@ -114,7 +114,7 @@ static void sync_pir_to_irr(struct vcpu *v)
 
 static int vlapic_find_highest_irr(struct vlapic *vlapic)
 {
-    sync_pir_to_irr(vlapic_vcpu(vlapic));
+    vlapic_sync_pir_to_irr(vlapic_vcpu(vlapic));
 
     return vlapic_find_highest_vector(&vlapic->regs->data[APIC_IRR]);
 }
@@ -1493,7 +1493,7 @@ static int lapic_save_regs(struct vcpu *v, hvm_domain_context_t *h)
     if ( !has_vlapic(v->domain) )
         return 0;
 
-    sync_pir_to_irr(v);
+    vlapic_sync_pir_to_irr(v);
 
     return hvm_save_entry(LAPIC_REGS, v->vcpu_id, h, vcpu_vlapic(v)->regs);
 }
diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/io.c
index b292e79382..5bf1877726 100644
--- a/xen/drivers/passthrough/io.c
+++ b/xen/drivers/passthrough/io.c
@@ -341,7 +341,7 @@ int pt_irq_create_bind(
     {
         uint8_t dest, delivery_mode;
         bool dest_mode;
-        int dest_vcpu_id;
+        int dest_vcpu_id, prev_vcpu_id = -1;
         const struct vcpu *vcpu;
         uint32_t gflags = pt_irq_bind->u.msi.gflags &
                           ~XEN_DOMCTL_VMSI_X86_UNMASKED;
@@ -411,6 +411,7 @@ int pt_irq_create_bind(
 
                 pirq_dpci->gmsi.gvec = pt_irq_bind->u.msi.gvec;
                 pirq_dpci->gmsi.gflags = gflags;
+                prev_vcpu_id = pirq_dpci->gmsi.dest_vcpu_id;
             }
         }
         /* Calculate dest_vcpu_id for MSI-type pirq migration. */
@@ -432,7 +433,10 @@ int pt_irq_create_bind(
                 vcpu = vector_hashing_dest(d, dest, dest_mode,
                                            pirq_dpci->gmsi.gvec);
             if ( vcpu )
+            {
                 pirq_dpci->gmsi.posted = true;
+                pirq_dpci->gmsi.dest_vcpu_id = vcpu->vcpu_id;
+            }
         }
         if ( vcpu && is_iommu_enabled(d) )
             hvm_migrate_pirq(pirq_dpci, vcpu);
@@ -440,7 +444,8 @@ int pt_irq_create_bind(
         /* Use interrupt posting if it is supported. */
         if ( iommu_intpost )
             pi_update_irte(vcpu ? &vcpu->arch.hvm.vmx.pi_desc : NULL,
-                           info, pirq_dpci->gmsi.gvec);
+                           info, pirq_dpci->gmsi.gvec,
+                           prev_vcpu_id >= 0 ? d->vcpu[prev_vcpu_id] : NULL);
 
         if ( pt_irq_bind->u.msi.gflags & XEN_DOMCTL_VMSI_X86_UNMASKED )
         {
@@ -729,7 +734,9 @@ int pt_irq_destroy_bind(
             what = "bogus";
     }
     else if ( pirq_dpci && pirq_dpci->gmsi.posted )
-        pi_update_irte(NULL, pirq, 0);
+        pi_update_irte(NULL, pirq, 0,
+                       pirq_dpci->gmsi.dest_vcpu_id >= 0
+                       ? d->vcpu[pirq_dpci->gmsi.dest_vcpu_id] : NULL);
 
     if ( pirq_dpci && (pirq_dpci->flags & HVM_IRQ_DPCI_MAPPED) &&
          list_empty(&pirq_dpci->digl_list) )
diff --git a/xen/drivers/passthrough/vtd/intremap.c b/xen/drivers/passthrough/vtd/intremap.c
index bf846195c4..07c1c1627a 100644
--- a/xen/drivers/passthrough/vtd/intremap.c
+++ b/xen/drivers/passthrough/vtd/intremap.c
@@ -946,12 +946,13 @@ void intel_iommu_disable_eim(void)
         disable_qinval(drhd->iommu);
 }
 
+#ifdef CONFIG_HVM
 /*
  * This function is used to update the IRTE for posted-interrupt
  * when guest changes MSI/MSI-X information.
  */
 int pi_update_irte(const struct pi_desc *pi_desc, const struct pirq *pirq,
-    const uint8_t gvec)
+    const uint8_t gvec, struct vcpu *prev)
 {
     struct irq_desc *desc;
     struct msi_desc *msi_desc;
@@ -964,8 +965,8 @@ int pi_update_irte(const struct pi_desc *pi_desc, const struct pirq *pirq,
     msi_desc = desc->msi_desc;
     if ( !msi_desc )
     {
-        rc = -ENODEV;
-        goto unlock_out;
+        spin_unlock_irq(&desc->lock);
+        return -ENODEV;
     }
     msi_desc->pi_desc = pi_desc;
     msi_desc->gvec = gvec;
@@ -974,10 +975,10 @@ int pi_update_irte(const struct pi_desc *pi_desc, const struct pirq *pirq,
 
     ASSERT(pcidevs_locked());
 
-    return msi_msg_write_remap_rte(msi_desc, &msi_desc->msg);
-
- unlock_out:
-    spin_unlock_irq(&desc->lock);
+    rc = msi_msg_write_remap_rte(msi_desc, &msi_desc->msg);
+    if ( !rc && prev )
+         vlapic_sync_pir_to_irr(prev);
 
     return rc;
 }
+#endif
diff --git a/xen/include/asm-x86/hvm/vlapic.h b/xen/include/asm-x86/hvm/vlapic.h
index dde66b4f0f..b0017d1dae 100644
--- a/xen/include/asm-x86/hvm/vlapic.h
+++ b/xen/include/asm-x86/hvm/vlapic.h
@@ -150,4 +150,6 @@ bool_t vlapic_match_dest(
     const struct vlapic *target, const struct vlapic *source,
     int short_hand, uint32_t dest, bool_t dest_mode);
 
+void vlapic_sync_pir_to_irr(struct vcpu *v);
+
 #endif /* __ASM_X86_HVM_VLAPIC_H__ */
diff --git a/xen/include/asm-x86/iommu.h b/xen/include/asm-x86/iommu.h
index 85741f7c96..314dcfbe47 100644
--- a/xen/include/asm-x86/iommu.h
+++ b/xen/include/asm-x86/iommu.h
@@ -119,7 +119,7 @@ static inline void iommu_disable_x2apic(void)
 extern bool untrusted_msi;
 
 int pi_update_irte(const struct pi_desc *pi_desc, const struct pirq *pirq,
-                   const uint8_t gvec);
+                   const uint8_t gvec, struct vcpu *prev);
 
 #endif /* !__ARCH_X86_IOMMU_H__ */
 /*
-- 
2.23.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts
  2019-10-09 12:52 [Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts Roger Pau Monne
@ 2019-10-09 13:35 ` Jan Beulich
  2019-10-14 14:36   ` Joe Jin
  0 siblings, 1 reply; 3+ messages in thread
From: Jan Beulich @ 2019-10-09 13:35 UTC (permalink / raw)
  To: Roger Pau Monne
  Cc: Juergen Gross, Kevin Tian, Wei Liu, Andrew Cooper, Joe Jin, xen-devel

On 09.10.2019 14:52, Roger Pau Monne wrote:
> When using posted interrupts and the guest migrates MSI from vCPUs Xen
> needs to flush any pending PIRR vectors on the previous vCPU, or else
> those vectors could get wrongly injected at a later point when the MSI
> fields are already updated.
> 
> Rename sync_pir_to_irr to vlapic_sync_pir_to_irr and export it so it
> can be called when updating the posted interrupt descriptor field in
> pi_update_irte. While there also remove the unlock_out from
> pi_update_irte, it's used in a single goto and removing it makes the
> function smaller.
> 
> Note that PIRR is synced to IRR both in pt_irq_destroy_bind and
> pt_irq_create_bind when the interrupt delivery data is being updated.
> 
> Also store the vCPU ID in multi-destination mode when using posted
> interrupts and the interrupt is bound to a single vCPU in order for
> posted interrupts to be used.
> 
> While there guard pi_update_irte with CONFIG_HVM since it's only used
> with HVM guests.
> 
> Reported-by: Joe Jin <joe.jin@oracle.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Like for the other patch I'd prefer to wait a little with committing
(even if the VT-d ack appeared quickly) until hopefully a Tested-by
could be provided.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts
  2019-10-09 13:35 ` Jan Beulich
@ 2019-10-14 14:36   ` Joe Jin
  0 siblings, 0 replies; 3+ messages in thread
From: Joe Jin @ 2019-10-14 14:36 UTC (permalink / raw)
  To: Jan Beulich, Roger Pau Monne
  Cc: Juergen Gross, xen-devel, Kevin Tian, Wei Liu, Andrew Cooper

On 10/9/19 6:35 AM, Jan Beulich wrote:
> On 09.10.2019 14:52, Roger Pau Monne wrote:
>> When using posted interrupts and the guest migrates MSI from vCPUs Xen
>> needs to flush any pending PIRR vectors on the previous vCPU, or else
>> those vectors could get wrongly injected at a later point when the MSI
>> fields are already updated.
>>
>> Rename sync_pir_to_irr to vlapic_sync_pir_to_irr and export it so it
>> can be called when updating the posted interrupt descriptor field in
>> pi_update_irte. While there also remove the unlock_out from
>> pi_update_irte, it's used in a single goto and removing it makes the
>> function smaller.
>>
>> Note that PIRR is synced to IRR both in pt_irq_destroy_bind and
>> pt_irq_create_bind when the interrupt delivery data is being updated.
>>
>> Also store the vCPU ID in multi-destination mode when using posted
>> interrupts and the interrupt is bound to a single vCPU in order for
>> posted interrupts to be used.
>>
>> While there guard pi_update_irte with CONFIG_HVM since it's only used
>> with HVM guests.
>>
>> Reported-by: Joe Jin <joe.jin@oracle.com>
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> Like for the other patch I'd prefer to wait a little with committing
> (even if the VT-d ack appeared quickly) until hopefully a Tested-by
> could be provided.

My test env has not been fixed yet, not sure when it can be fixed, once
it available I'll test it.

Thanks,
Joe


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-09 12:52 [Xen-devel] [PATCH v2] x86/passthrough: fix migration of MSI when using posted interrupts Roger Pau Monne
2019-10-09 13:35 ` Jan Beulich
2019-10-14 14:36   ` Joe Jin

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@lists.xen.org xen-devel@archiver.kernel.org
	public-inbox-index xen-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.xenproject.lists.xen-devel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox