[PATCH v4 0/5] Make x86 IOMMU driver support configurable

[PATCH v4 0/5] Make x86 IOMMU driver support configurable

[Xenia Ragiadakou]

This series aims to provide a means to render the iommu driver support for x86
configurable. Currently, irrespectively of the target platform, both AMD and
Intel iommu drivers are built. This is the case because the existent Kconfig
infrastructure does not provide any facilities for finer-grained configuration.

The series adds two new Kconfig options, AMD_IOMMU and INTEL_IOMMU, that can be
used to generate a tailored iommu configuration for a given platform.

This version of the series is rebased on top of the current staging and
addresses the comments made on version 3.
Patches
"[v3 1/8] x86/iommu: amd_iommu_perdev_intremap is AMD-Vi specific"
"[v3 2/8] x86/iommu: iommu_igfx and iommu_qinval are Intel VT-d specific"
"[v3 4/8] x86/acpi: separate AMD-Vi and VT-d specific functions"
are not included in this series because they have been already merged.

Xenia Ragiadakou (5):
  x86/iommu: snoop control is allowed only by Intel VT-d
  x86/iommu: make code addressing CVE-2011-1898 no VT-d specific
  x86/iommu: call pi_update_irte through an hvm_function callback
  x86/dpci: move hvm_dpci_isairq_eoi() to generic HVM code
  x86/iommu: make AMD-Vi and Intel VT-d support configurable

 xen/arch/x86/hvm/vmx/vmx.c               | 41 +++++++++++++++
 xen/arch/x86/include/asm/hvm/hvm.h       | 10 ++++
 xen/arch/x86/include/asm/iommu.h         |  4 --
 xen/drivers/passthrough/Kconfig          | 22 +++++++-
 xen/drivers/passthrough/vtd/intremap.c   | 36 -------------
 xen/drivers/passthrough/vtd/iommu.c      |  5 +-
 xen/drivers/passthrough/vtd/x86/Makefile |  1 -
 xen/drivers/passthrough/vtd/x86/hvm.c    | 64 ------------------------
 xen/drivers/passthrough/x86/hvm.c        | 50 ++++++++++++++++--
 xen/drivers/passthrough/x86/iommu.c      |  5 ++
 xen/include/xen/iommu.h                  |  8 ++-
 11 files changed, 131 insertions(+), 115 deletions(-)
 delete mode 100644 xen/drivers/passthrough/vtd/x86/hvm.c

-- 
2.37.2

[PATCH v4 1/5] x86/iommu: snoop control is allowed only by Intel VT-d

[Xenia Ragiadakou]

The AMD-Vi driver forces coherent accesses by hardwiring the FC bit to 1.
Therefore, given that iommu_snoop is used only when the iommu is enabled,
when Xen is configured with only the AMD iommu enabled, iommu_snoop can be
reduced to a #define to true.

No functional change intended.

Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---

No changes in v4

Depends on Jan's "x86/shadow: make iommu_snoop usage consistent with HAP's"
being applied first.

 xen/include/xen/iommu.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 4f22fc1bed..626731941b 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -74,7 +74,12 @@ extern enum __packed iommu_intremap {
    iommu_intremap_restricted,
    iommu_intremap_full,
 } iommu_intremap;
-extern bool iommu_igfx, iommu_qinval, iommu_snoop;
+extern bool iommu_igfx, iommu_qinval;
+#ifdef CONFIG_INTEL_IOMMU
+extern bool iommu_snoop;
+#else
+# define iommu_snoop true
+#endif /* CONFIG_INTEL_IOMMU */
 #else
 # define iommu_intremap false
 # define iommu_snoop false
-- 
2.37.2

[PATCH v4 2/5] x86/iommu: make code addressing CVE-2011-1898 no VT-d specific

[Xenia Ragiadakou]

The variable untrusted_msi indicates whether the system is vulnerable to
CVE-2011-1898 due to the absence of interrupt remapping support.
Although AMD iommus with interrupt remapping disabled are also affected,
this case is not handled yet. Given that the issue is not VT-d specific,
and to accommodate future use of the flag for covering also the AMD iommu
case, move the definition of the flag out of the VT-d specific code to the
common x86 iommu code.

Also, since the current implementation assumes that only PV guests are prone
to this attack, take the opportunity to define untrusted_msi only when PV is
enabled.

No functional change intended.

Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---

Changes in v4:
  - in vtd code, guard with CONFIG_PV the use of untrusted_msi
  - mention in commit log that CVE-2011-1898 currently is not addressed for
    AMD iommus with disabled intremap
  - add Jan's Reviewed-by tag

 xen/drivers/passthrough/vtd/iommu.c | 5 ++---
 xen/drivers/passthrough/x86/iommu.c | 5 +++++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 62e143125d..e97b1fe8cd 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -54,9 +54,6 @@
                                  ? dom_iommu(d)->arch.vtd.pgd_maddr \
                                  : (pdev)->arch.vtd.pgd_maddr)
 
-/* Possible unfiltered LAPIC/MSI messages from untrusted sources? */
-bool __read_mostly untrusted_msi;
-
 bool __read_mostly iommu_igfx = true;
 bool __read_mostly iommu_qinval = true;
 #ifndef iommu_snoop
@@ -2770,6 +2767,7 @@ static int cf_check reassign_device_ownership(
         if ( !has_arch_pdevs(target) )
             vmx_pi_hooks_assign(target);
 
+#ifdef CONFIG_PV
         /*
          * Devices assigned to untrusted domains (here assumed to be any domU)
          * can attempt to send arbitrary LAPIC/MSI messages. We are unprotected
@@ -2778,6 +2776,7 @@ static int cf_check reassign_device_ownership(
         if ( !iommu_intremap && !is_hardware_domain(target) &&
              !is_system_domain(target) )
             untrusted_msi = true;
+#endif
 
         ret = domain_context_mapping(target, devfn, pdev);
 
diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c
index f671b0f2bb..c5021ea023 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -36,6 +36,11 @@ bool __initdata iommu_superpages = true;
 
 enum iommu_intremap __read_mostly iommu_intremap = iommu_intremap_full;
 
+#ifdef CONFIG_PV
+/* Possible unfiltered LAPIC/MSI messages from untrusted sources? */
+bool __read_mostly untrusted_msi;
+#endif
+
 #ifndef iommu_intpost
 /*
  * In the current implementation of VT-d posted interrupts, in some extreme
-- 
2.37.2

[PATCH v4 3/5] x86/iommu: call pi_update_irte through an hvm_function callback

[Xenia Ragiadakou]

Posted interrupt support in Xen is currently implemented only for the
Intel platforms. Instead of calling directly pi_update_irte() from the
common hvm code, add a pi_update_irte callback to the hvm_function_table.
Then, create a wrapper function hvm_pi_update_irte() to be used by the
common hvm code.

In the pi_update_irte callback prototype, pass the vcpu as first parameter
instead of the posted-interrupt descriptor that is platform specific, and
remove the const qualifier from the parameter gvec since it is not needed
and because it does not compile with the alternative code patching in use.

Since the posted interrupt descriptor is Intel VT-x specific while
msi_msg_write_remap_rte is iommu specific, open code pi_update_irte() inside
vmx_pi_update_irte() but replace msi_msg_write_remap_rte() with generic
iommu_update_ire_from_msi(). That way vmx_pi_update_irte() is not bound to
Intel VT-d anymore.

Remove the now unused pi_update_irte() implementation.

No functional change intended.

Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---

Changes in v4:
  - remove the now unused asm/hvm/vmx/vmcs.h

 xen/arch/x86/hvm/vmx/vmx.c             | 41 ++++++++++++++++++++++++++
 xen/arch/x86/include/asm/hvm/hvm.h     | 10 +++++++
 xen/arch/x86/include/asm/iommu.h       |  4 ---
 xen/drivers/passthrough/vtd/intremap.c | 36 ----------------------
 xen/drivers/passthrough/x86/hvm.c      |  5 ++--
 5 files changed, 53 insertions(+), 43 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index ade2a25ce7..270bc98195 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -397,6 +397,43 @@ void vmx_pi_hooks_deassign(struct domain *d)
     domain_unpause(d);
 }
 
+/*
+ * This function is used to update the IRTE for posted-interrupt
+ * when guest changes MSI/MSI-X information.
+ */
+static int cf_check vmx_pi_update_irte(const struct vcpu *v,
+                                       const struct pirq *pirq, uint8_t gvec)
+{
+    const struct pi_desc *pi_desc = v ? &v->arch.hvm.vmx.pi_desc : NULL;
+    struct irq_desc *desc;
+    struct msi_desc *msi_desc;
+    int rc;
+
+    desc = pirq_spin_lock_irq_desc(pirq, NULL);
+    if ( !desc )
+        return -EINVAL;
+
+    msi_desc = desc->msi_desc;
+    if ( !msi_desc )
+    {
+        rc = -ENODEV;
+        goto unlock_out;
+    }
+    msi_desc->pi_desc = pi_desc;
+    msi_desc->gvec = gvec;
+
+    spin_unlock_irq(&desc->lock);
+
+    ASSERT(pcidevs_locked());
+
+    return iommu_update_ire_from_msi(msi_desc, &msi_desc->msg);
+
+ unlock_out:
+    spin_unlock_irq(&desc->lock);
+
+    return rc;
+}
+
 static const struct lbr_info {
     u32 base, count;
 } p4_lbr[] = {
@@ -2986,8 +3023,12 @@ const struct hvm_function_table * __init start_vmx(void)
     {
         alloc_direct_apic_vector(&posted_intr_vector, pi_notification_interrupt);
         if ( iommu_intpost )
+        {
             alloc_direct_apic_vector(&pi_wakeup_vector, pi_wakeup_interrupt);
 
+            vmx_function_table.pi_update_irte = vmx_pi_update_irte;
+        }
+
         vmx_function_table.deliver_posted_intr = vmx_deliver_posted_intr;
         vmx_function_table.sync_pir_to_irr     = vmx_sync_pir_to_irr;
         vmx_function_table.test_pir            = vmx_test_pir;
diff --git a/xen/arch/x86/include/asm/hvm/hvm.h b/xen/arch/x86/include/asm/hvm/hvm.h
index 65768c797e..80e4565bd2 100644
--- a/xen/arch/x86/include/asm/hvm/hvm.h
+++ b/xen/arch/x86/include/asm/hvm/hvm.h
@@ -28,6 +28,8 @@
 #include <asm/x86_emulate.h>
 #include <asm/hvm/asid.h>
 
+struct pirq; /* needed by pi_update_irte */
+
 #ifdef CONFIG_HVM_FEP
 /* Permit use of the Forced Emulation Prefix in HVM guests */
 extern bool_t opt_hvm_fep;
@@ -213,6 +215,8 @@ struct hvm_function_table {
     void (*sync_pir_to_irr)(struct vcpu *v);
     bool (*test_pir)(const struct vcpu *v, uint8_t vector);
     void (*handle_eoi)(uint8_t vector, int isr);
+    int (*pi_update_irte)(const struct vcpu *v, const struct pirq *pirq,
+                          uint8_t gvec);
 
     /*Walk nested p2m  */
     int (*nhvm_hap_walk_L1_p2m)(struct vcpu *v, paddr_t L2_gpa,
@@ -776,6 +780,12 @@ static inline void hvm_set_nonreg_state(struct vcpu *v,
         alternative_vcall(hvm_funcs.set_nonreg_state, v, nrs);
 }
 
+static inline int hvm_pi_update_irte(const struct vcpu *v,
+                                     const struct pirq *pirq, uint8_t gvec)
+{
+    return alternative_call(hvm_funcs.pi_update_irte, v, pirq, gvec);
+}
+
 #else  /* CONFIG_HVM */
 
 #define hvm_enabled false
diff --git a/xen/arch/x86/include/asm/iommu.h b/xen/arch/x86/include/asm/iommu.h
index fc0afe35bf..586c7434f2 100644
--- a/xen/arch/x86/include/asm/iommu.h
+++ b/xen/arch/x86/include/asm/iommu.h
@@ -21,7 +21,6 @@
 #include <asm/apicdef.h>
 #include <asm/cache.h>
 #include <asm/processor.h>
-#include <asm/hvm/vmx/vmcs.h>
 
 #define DEFAULT_DOMAIN_ADDRESS_WIDTH 48
 
@@ -129,9 +128,6 @@ void iommu_identity_map_teardown(struct domain *d);
 
 extern bool untrusted_msi;
 
-int pi_update_irte(const struct pi_desc *pi_desc, const struct pirq *pirq,
-                   const uint8_t gvec);
-
 extern bool iommu_non_coherent, iommu_superpages;
 
 static inline void iommu_sync_cache(const void *addr, unsigned int size)
diff --git a/xen/drivers/passthrough/vtd/intremap.c b/xen/drivers/passthrough/vtd/intremap.c
index 1512e4866b..b39bc83282 100644
--- a/xen/drivers/passthrough/vtd/intremap.c
+++ b/xen/drivers/passthrough/vtd/intremap.c
@@ -866,39 +866,3 @@ void cf_check intel_iommu_disable_eim(void)
     for_each_drhd_unit ( drhd )
         disable_qinval(drhd->iommu);
 }
-
-/*
- * 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)
-{
-    struct irq_desc *desc;
-    struct msi_desc *msi_desc;
-    int rc;
-
-    desc = pirq_spin_lock_irq_desc(pirq, NULL);
-    if ( !desc )
-        return -EINVAL;
-
-    msi_desc = desc->msi_desc;
-    if ( !msi_desc )
-    {
-        rc = -ENODEV;
-        goto unlock_out;
-    }
-    msi_desc->pi_desc = pi_desc;
-    msi_desc->gvec = gvec;
-
-    spin_unlock_irq(&desc->lock);
-
-    ASSERT(pcidevs_locked());
-
-    return msi_msg_write_remap_rte(msi_desc, &msi_desc->msg);
-
- unlock_out:
-    spin_unlock_irq(&desc->lock);
-
-    return rc;
-}
diff --git a/xen/drivers/passthrough/x86/hvm.c b/xen/drivers/passthrough/x86/hvm.c
index a16e0e5344..e720461a14 100644
--- a/xen/drivers/passthrough/x86/hvm.c
+++ b/xen/drivers/passthrough/x86/hvm.c
@@ -381,8 +381,7 @@ 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);
+            hvm_pi_update_irte(vcpu, info, pirq_dpci->gmsi.gvec);
 
         if ( pt_irq_bind->u.msi.gflags & XEN_DOMCTL_VMSI_X86_UNMASKED )
         {
@@ -672,7 +671,7 @@ int pt_irq_destroy_bind(
             what = "bogus";
     }
     else if ( pirq_dpci && pirq_dpci->gmsi.posted )
-        pi_update_irte(NULL, pirq, 0);
+        hvm_pi_update_irte(NULL, pirq, 0);
 
     if ( pirq_dpci && (pirq_dpci->flags & HVM_IRQ_DPCI_MAPPED) &&
          list_empty(&pirq_dpci->digl_list) )
-- 
2.37.2

[PATCH v4 4/5] x86/dpci: move hvm_dpci_isairq_eoi() to generic HVM code

[Xenia Ragiadakou]

The function hvm_dpci_isairq_eoi() has no dependencies on VT-d driver code
and can be moved from xen/drivers/passthrough/vtd/x86/hvm.c to
xen/drivers/passthrough/x86/hvm.c, along with the corresponding copyrights.

Remove the now empty xen/drivers/passthrough/vtd/x86/hvm.c.

Since the function is used only in this file, declare it static.

No functional change intended.

Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---

No changes in v4

 xen/drivers/passthrough/vtd/x86/Makefile |  1 -
 xen/drivers/passthrough/vtd/x86/hvm.c    | 64 ------------------------
 xen/drivers/passthrough/x86/hvm.c        | 45 +++++++++++++++++
 xen/include/xen/iommu.h                  |  1 -
 4 files changed, 45 insertions(+), 66 deletions(-)
 delete mode 100644 xen/drivers/passthrough/vtd/x86/hvm.c

diff --git a/xen/drivers/passthrough/vtd/x86/Makefile b/xen/drivers/passthrough/vtd/x86/Makefile
index 4ef00a4c5b..fe20a0b019 100644
--- a/xen/drivers/passthrough/vtd/x86/Makefile
+++ b/xen/drivers/passthrough/vtd/x86/Makefile
@@ -1,3 +1,2 @@
 obj-y += ats.o
-obj-$(CONFIG_HVM) += hvm.o
 obj-y += vtd.o
diff --git a/xen/drivers/passthrough/vtd/x86/hvm.c b/xen/drivers/passthrough/vtd/x86/hvm.c
deleted file mode 100644
index bc776cf7da..0000000000
--- a/xen/drivers/passthrough/vtd/x86/hvm.c
+++ /dev/null
@@ -1,64 +0,0 @@
-/*
- * Copyright (c) 2008, Intel Corporation.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
- * more details.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program; If not, see <http://www.gnu.org/licenses/>.
- *
- * Copyright (C) Allen Kay <allen.m.kay@intel.com>
- * Copyright (C) Weidong Han <weidong.han@intel.com>
- */
-
-#include <xen/iommu.h>
-#include <xen/irq.h>
-#include <xen/sched.h>
-
-static int cf_check _hvm_dpci_isairq_eoi(
-    struct domain *d, struct hvm_pirq_dpci *pirq_dpci, void *arg)
-{
-    struct hvm_irq *hvm_irq = hvm_domain_irq(d);
-    unsigned int isairq = (long)arg;
-    const struct dev_intx_gsi_link *digl;
-
-    list_for_each_entry ( digl, &pirq_dpci->digl_list, list )
-    {
-        unsigned int link = hvm_pci_intx_link(digl->device, digl->intx);
-
-        if ( hvm_irq->pci_link.route[link] == isairq )
-        {
-            hvm_pci_intx_deassert(d, digl->device, digl->intx);
-            if ( --pirq_dpci->pending == 0 )
-                pirq_guest_eoi(dpci_pirq(pirq_dpci));
-        }
-    }
-
-    return 0;
-}
-
-void hvm_dpci_isairq_eoi(struct domain *d, unsigned int isairq)
-{
-    struct hvm_irq_dpci *dpci = NULL;
-
-    ASSERT(isairq < NR_ISAIRQS);
-    if ( !is_iommu_enabled(d) )
-        return;
-
-    write_lock(&d->event_lock);
-
-    dpci = domain_get_irq_dpci(d);
-
-    if ( dpci && test_bit(isairq, dpci->isairq_map) )
-    {
-        /* Multiple mirq may be mapped to one isa irq */
-        pt_pirq_iterate(d, _hvm_dpci_isairq_eoi, (void *)(long)isairq);
-    }
-    write_unlock(&d->event_lock);
-}
diff --git a/xen/drivers/passthrough/x86/hvm.c b/xen/drivers/passthrough/x86/hvm.c
index e720461a14..6bbd04bf3d 100644
--- a/xen/drivers/passthrough/x86/hvm.c
+++ b/xen/drivers/passthrough/x86/hvm.c
@@ -14,6 +14,7 @@
  * this program; If not, see <http://www.gnu.org/licenses/>.
  *
  * Copyright (C) Allen Kay <allen.m.kay@intel.com>
+ * Copyright (C) Weidong Han <weidong.han@intel.com>
  * Copyright (C) Xiaohui Xin <xiaohui.xin@intel.com>
  */
 
@@ -924,6 +925,50 @@ static void hvm_gsi_eoi(struct domain *d, unsigned int gsi)
     hvm_pirq_eoi(pirq);
 }
 
+static int cf_check _hvm_dpci_isairq_eoi(
+    struct domain *d, struct hvm_pirq_dpci *pirq_dpci, void *arg)
+{
+    const struct hvm_irq *hvm_irq = hvm_domain_irq(d);
+    unsigned int isairq = (long)arg;
+    const struct dev_intx_gsi_link *digl;
+
+    list_for_each_entry ( digl, &pirq_dpci->digl_list, list )
+    {
+        unsigned int link = hvm_pci_intx_link(digl->device, digl->intx);
+
+        if ( hvm_irq->pci_link.route[link] == isairq )
+        {
+            hvm_pci_intx_deassert(d, digl->device, digl->intx);
+            if ( --pirq_dpci->pending == 0 )
+                pirq_guest_eoi(dpci_pirq(pirq_dpci));
+        }
+    }
+
+    return 0;
+}
+
+static void hvm_dpci_isairq_eoi(struct domain *d, unsigned int isairq)
+{
+    const struct hvm_irq_dpci *dpci = NULL;
+
+    ASSERT(isairq < NR_ISAIRQS);
+
+    if ( !is_iommu_enabled(d) )
+        return;
+
+    write_lock(&d->event_lock);
+
+    dpci = domain_get_irq_dpci(d);
+
+    if ( dpci && test_bit(isairq, dpci->isairq_map) )
+    {
+        /* Multiple mirq may be mapped to one isa irq */
+        pt_pirq_iterate(d, _hvm_dpci_isairq_eoi, (void *)(long)isairq);
+    }
+
+    write_unlock(&d->event_lock);
+}
+
 void hvm_dpci_eoi(struct domain *d, unsigned int guest_gsi)
 {
     const struct hvm_irq_dpci *hvm_irq_dpci;
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 626731941b..405db59971 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -201,7 +201,6 @@ int hvm_do_IRQ_dpci(struct domain *, struct pirq *);
 int pt_irq_create_bind(struct domain *, const struct xen_domctl_bind_pt_irq *);
 int pt_irq_destroy_bind(struct domain *, const struct xen_domctl_bind_pt_irq *);
 
-void hvm_dpci_isairq_eoi(struct domain *d, unsigned int isairq);
 struct hvm_irq_dpci *domain_get_irq_dpci(const struct domain *);
 void free_hvm_irq_dpci(struct hvm_irq_dpci *dpci);
 
-- 
2.37.2

[PATCH v4 5/5] x86/iommu: make AMD-Vi and Intel VT-d support configurable

[Xenia Ragiadakou]

Provide the user with configuration control over the IOMMU support by making
AMD_IOMMU and INTEL_IOMMU options user selectable and able to be turned off.

However, there are cases where the IOMMU support is required, for instance for
a system with more than 254 CPUs. In order to prevent users from unknowingly
disabling it and ending up with a broken hypervisor, make the support user
selectable only if EXPERT is enabled.

To preserve the current default configuration of an x86 system, both options
depend on X86 and default to Y.

Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---

No changes in v4

 xen/drivers/passthrough/Kconfig | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/passthrough/Kconfig b/xen/drivers/passthrough/Kconfig
index 5c65567744..864fcf3b0c 100644
--- a/xen/drivers/passthrough/Kconfig
+++ b/xen/drivers/passthrough/Kconfig
@@ -38,10 +38,28 @@ config IPMMU_VMSA
 endif
 
 config AMD_IOMMU
-	def_bool y if X86
+	bool "AMD IOMMU" if EXPERT
+	depends on X86
+	default y
+	help
+	  Enables I/O virtualization on platforms that implement the
+	  AMD I/O Virtualization Technology (IOMMU).
+
+	  If your system includes an IOMMU implementing AMD-Vi, say Y.
+	  This is required if your system has more than 254 CPUs.
+	  If in doubt, say Y.
 
 config INTEL_IOMMU
-	def_bool y if X86
+	bool "Intel VT-d" if EXPERT
+	depends on X86
+	default y
+	help
+	  Enables I/O virtualization on platforms that implement the
+	  Intel Virtualization Technology for Directed I/O (Intel VT-d).
+
+	  If your system includes an IOMMU implementing Intel VT-d, say Y.
+	  This is required if your system has more than 254 CPUs.
+	  If in doubt, say Y.
 
 config IOMMU_FORCE_PT_SHARE
 	bool
-- 
2.37.2

Re: [PATCH v4 3/5] x86/iommu: call pi_update_irte through an hvm_function callback

[Jan Beulich]

On 24.01.2023 13:41, Xenia Ragiadakou wrote:
> Posted interrupt support in Xen is currently implemented only for the
> Intel platforms. Instead of calling directly pi_update_irte() from the
> common hvm code, add a pi_update_irte callback to the hvm_function_table.
> Then, create a wrapper function hvm_pi_update_irte() to be used by the
> common hvm code.
> 
> In the pi_update_irte callback prototype, pass the vcpu as first parameter
> instead of the posted-interrupt descriptor that is platform specific, and
> remove the const qualifier from the parameter gvec since it is not needed
> and because it does not compile with the alternative code patching in use.
> 
> Since the posted interrupt descriptor is Intel VT-x specific while
> msi_msg_write_remap_rte is iommu specific, open code pi_update_irte() inside
> vmx_pi_update_irte() but replace msi_msg_write_remap_rte() with generic
> iommu_update_ire_from_msi(). That way vmx_pi_update_irte() is not bound to
> Intel VT-d anymore.
> 
> Remove the now unused pi_update_irte() implementation.
> 
> No functional change intended.
> 
> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>

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

RE: [PATCH v4 2/5] x86/iommu: make code addressing CVE-2011-1898 no VT-d specific

[Tian, Kevin]

> From: Xenia Ragiadakou <burzalodowa@gmail.com>
> Sent: Tuesday, January 24, 2023 8:42 PM
> 
> The variable untrusted_msi indicates whether the system is vulnerable to
> CVE-2011-1898 due to the absence of interrupt remapping support.
> Although AMD iommus with interrupt remapping disabled are also affected,
> this case is not handled yet. Given that the issue is not VT-d specific,
> and to accommodate future use of the flag for covering also the AMD iommu
> case, move the definition of the flag out of the VT-d specific code to the
> common x86 iommu code.
> 
> Also, since the current implementation assumes that only PV guests are
> prone
> to this attack, take the opportunity to define untrusted_msi only when PV is
> enabled.
> 

I'm fine with this change given no functional change.

But I'm curious about the statement here that the current code only
applies to PV guest. I didn't see such statement in original mail [1]
and in concept a HVM guest with passthrough device can also do such
attack w/o interrupt remapping.

Any more context?

[1] http://old-list-archives.xenproject.org/archives/html/xen-devel/2011-05/msg00687.html

RE: [PATCH v4 3/5] x86/iommu: call pi_update_irte through an hvm_function callback

[Tian, Kevin]

> From: Xenia Ragiadakou <burzalodowa@gmail.com>
> Sent: Tuesday, January 24, 2023 8:42 PM
> 
> Posted interrupt support in Xen is currently implemented only for the
> Intel platforms. Instead of calling directly pi_update_irte() from the
> common hvm code, add a pi_update_irte callback to the hvm_function_table.
> Then, create a wrapper function hvm_pi_update_irte() to be used by the
> common hvm code.
> 
> In the pi_update_irte callback prototype, pass the vcpu as first parameter
> instead of the posted-interrupt descriptor that is platform specific, and
> remove the const qualifier from the parameter gvec since it is not needed
> and because it does not compile with the alternative code patching in use.
> 
> Since the posted interrupt descriptor is Intel VT-x specific while
> msi_msg_write_remap_rte is iommu specific, open code pi_update_irte()
> inside
> vmx_pi_update_irte() but replace msi_msg_write_remap_rte() with generic
> iommu_update_ire_from_msi(). That way vmx_pi_update_irte() is not
> bound to
> Intel VT-d anymore.
> 
> Remove the now unused pi_update_irte() implementation.
> 
> No functional change intended.
> 
> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

RE: [PATCH v4 4/5] x86/dpci: move hvm_dpci_isairq_eoi() to generic HVM code

[Tian, Kevin]

> From: Xenia Ragiadakou <burzalodowa@gmail.com>
> Sent: Tuesday, January 24, 2023 8:42 PM
> 
> The function hvm_dpci_isairq_eoi() has no dependencies on VT-d driver
> code
> and can be moved from xen/drivers/passthrough/vtd/x86/hvm.c to
> xen/drivers/passthrough/x86/hvm.c, along with the corresponding
> copyrights.
> 
> Remove the now empty xen/drivers/passthrough/vtd/x86/hvm.c.
> 
> Since the function is used only in this file, declare it static.
> 
> No functional change intended.
> 
> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

Re: [PATCH v4 2/5] x86/iommu: make code addressing CVE-2011-1898 no VT-d specific

[Xenia Ragiadakou]

On 2/1/23 07:07, Tian, Kevin wrote:
>> From: Xenia Ragiadakou <burzalodowa@gmail.com>
>> Sent: Tuesday, January 24, 2023 8:42 PM
>>
>> The variable untrusted_msi indicates whether the system is vulnerable to
>> CVE-2011-1898 due to the absence of interrupt remapping support.
>> Although AMD iommus with interrupt remapping disabled are also affected,
>> this case is not handled yet. Given that the issue is not VT-d specific,
>> and to accommodate future use of the flag for covering also the AMD iommu
>> case, move the definition of the flag out of the VT-d specific code to the
>> common x86 iommu code.
>>
>> Also, since the current implementation assumes that only PV guests are
>> prone
>> to this attack, take the opportunity to define untrusted_msi only when PV is
>> enabled.
>>
> 
> I'm fine with this change given no functional change.
> 
> But I'm curious about the statement here that the current code only
> applies to PV guest. I didn't see such statement in original mail [1]
> and in concept a HVM guest with passthrough device can also do such
> attack w/o interrupt remapping.
> 
> Any more context?

I agree. I phrased it that way because currently the mitigation 
addresses only maliciously injected PV traps.

> 
> [1] http://old-list-archives.xenproject.org/archives/html/xen-devel/2011-05/msg00687.html

-- 
Xenia

Re: [PATCH v4 2/5] x86/iommu: make code addressing CVE-2011-1898 no VT-d specific

[Jan Beulich]

On 01.02.2023 06:07, Tian, Kevin wrote:
>> From: Xenia Ragiadakou <burzalodowa@gmail.com>
>> Sent: Tuesday, January 24, 2023 8:42 PM
>>
>> The variable untrusted_msi indicates whether the system is vulnerable to
>> CVE-2011-1898 due to the absence of interrupt remapping support.
>> Although AMD iommus with interrupt remapping disabled are also affected,
>> this case is not handled yet. Given that the issue is not VT-d specific,
>> and to accommodate future use of the flag for covering also the AMD iommu
>> case, move the definition of the flag out of the VT-d specific code to the
>> common x86 iommu code.
>>
>> Also, since the current implementation assumes that only PV guests are
>> prone
>> to this attack, take the opportunity to define untrusted_msi only when PV is
>> enabled.
>>
> 
> I'm fine with this change given no functional change.
> 
> But I'm curious about the statement here that the current code only
> applies to PV guest. I didn't see such statement in original mail [1]
> and in concept a HVM guest with passthrough device can also do such
> attack w/o interrupt remapping.
> 
> Any more context?

Isn't this simply because we don't allow HVM to have devices assigned
without intremap? (I'm not sure, but even for PV allowing this may
have been limited to the xend tool stack.)

Jan

RE: [PATCH v4 2/5] x86/iommu: make code addressing CVE-2011-1898 no VT-d specific

[Tian, Kevin]

> From: Jan Beulich <jbeulich@suse.com>
> Sent: Wednesday, February 1, 2023 5:30 PM
> 
> On 01.02.2023 06:07, Tian, Kevin wrote:
> >> From: Xenia Ragiadakou <burzalodowa@gmail.com>
> >> Sent: Tuesday, January 24, 2023 8:42 PM
> >>
> >> The variable untrusted_msi indicates whether the system is vulnerable to
> >> CVE-2011-1898 due to the absence of interrupt remapping support.
> >> Although AMD iommus with interrupt remapping disabled are also
> affected,
> >> this case is not handled yet. Given that the issue is not VT-d specific,
> >> and to accommodate future use of the flag for covering also the AMD
> iommu
> >> case, move the definition of the flag out of the VT-d specific code to the
> >> common x86 iommu code.
> >>
> >> Also, since the current implementation assumes that only PV guests are
> >> prone
> >> to this attack, take the opportunity to define untrusted_msi only when PV
> is
> >> enabled.
> >>
> >
> > I'm fine with this change given no functional change.
> >
> > But I'm curious about the statement here that the current code only
> > applies to PV guest. I didn't see such statement in original mail [1]
> > and in concept a HVM guest with passthrough device can also do such
> > attack w/o interrupt remapping.
> >
> > Any more context?
> 
> Isn't this simply because we don't allow HVM to have devices assigned
> without intremap? (I'm not sure, but even for PV allowing this may
> have been limited to the xend tool stack.)
> 

OK, this is what I'm seeking.

Reviewed-by: Kevin Tian <kevin.tian@intel.com>