[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-changelog] [xen staging] x86: allow stubdom access to irq created for msi



commit 92d9101eabc75bff9184bf7fe5f2e9241897c857
Author:     Simon Gaiser <simon@xxxxxxxxxxxxxxxxxxxxxx>
AuthorDate: Fri Sep 27 15:04:08 2019 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Fri Sep 27 15:04:08 2019 +0200

    x86: allow stubdom access to irq created for msi
    
    Stubdomains need to be given sufficient privilege over the guest which it
    provides emulation for in order for PCI passthrough to work correctly.
    When a HVM domain try to enable MSI, QEMU in stubdomain calls
    PHYSDEVOP_map_pirq, but later it needs to call XEN_DOMCTL_bind_pt_irq as
    part of xc_domain_update_msi_irq. Give the stubdomain enough permissions
    over the mapped interrupt in order to bind it successfully to it's
    target domain.
    
    This is not needed for PCI INTx, because IRQ in that case is known
    beforehand and the stubdomain is given permissions over this IRQ by
    libxl__device_pci_add (there's a do_pci_add against the stubdomain).
    
    create_irq() already grant IRQ access to hardware_domain, with
    assumption the device model lives there.
    Modify create_irq() to take additional parameter, whether to grant
    permissions to the domain creating the IRQ, which may be dom0 or a
    stubdomain. Do this instead of granting access always to
    hardware_domain. Save ID of the domain given permission, to revoke it in
    destroy_irq() - easier and cleaner than replaying logic of create_irq()
    parameter. Use domid instead of actual reference to the domain,
    because it might get destroyed before destroying IRQ (stubdomain is
    destroyed before its target domain). And it is not an issue,
    because IRQ permissions live within domain structure, so destroying
    a domain also implicitly revoke the permission.  Potential domid
    reuse is detected by checking if that domain does have permission
    over the IRQ being destroyed.
    
    Then, adjust all callers to provide the parameter. In case of Xen
    internal allocations, set it to false, but for domain accessible
    interrupt set it to true.
    
    Inspired by 
https://github.com/OpenXT/xenclient-oe/blob/5e0e7304a5a3c75ef01240a1e3673665b2aaf05e/recipes-extended/xen/files/stubdomain-msi-irq-access.patch
 by Eric Chanudet <chanudete@xxxxxxxxxxxx>.
    
    Signed-off-by: Simon Gaiser <simon@xxxxxxxxxxxxxxxxxxxxxx>
    Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
    Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
    Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 xen/arch/x86/hpet.c                      |  3 ++-
 xen/arch/x86/irq.c                       | 44 ++++++++++++++++++++++----------
 xen/drivers/char/ns16550.c               |  2 +-
 xen/drivers/passthrough/amd/iommu_init.c |  2 +-
 xen/drivers/passthrough/vtd/iommu.c      |  3 ++-
 xen/include/asm-x86/irq.h                | 11 +++++++-
 6 files changed, 47 insertions(+), 18 deletions(-)

diff --git a/xen/arch/x86/hpet.c b/xen/arch/x86/hpet.c
index 4b08488ef1..57f68fa81b 100644
--- a/xen/arch/x86/hpet.c
+++ b/xen/arch/x86/hpet.c
@@ -11,6 +11,7 @@
 #include <xen/softirq.h>
 #include <xen/irq.h>
 #include <xen/numa.h>
+#include <xen/sched.h>
 #include <asm/fixmap.h>
 #include <asm/div64.h>
 #include <asm/hpet.h>
@@ -368,7 +369,7 @@ static int __init hpet_assign_irq(struct hpet_event_channel 
*ch)
 {
     int irq;
 
-    if ( (irq = create_irq(NUMA_NO_NODE)) < 0 )
+    if ( (irq = create_irq(NUMA_NO_NODE, false)) < 0 )
         return irq;
 
     ch->msi.irq = irq;
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index d96cf43542..e9b65b1d64 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -254,7 +254,8 @@ void __init clear_irq_vector(int irq)
 /*
  * Dynamic irq allocate and deallocation for MSI
  */
-int create_irq(nodeid_t node)
+
+int create_irq(nodeid_t node, bool grant_access)
 {
     int irq, ret;
     struct irq_desc *desc;
@@ -282,18 +283,25 @@ int create_irq(nodeid_t node)
         }
         ret = assign_irq_vector(irq, mask);
     }
+
+    ASSERT(desc->arch.creator_domid == DOMID_INVALID);
+
     if (ret < 0)
     {
         desc->arch.used = IRQ_UNUSED;
         irq = ret;
     }
-    else if ( hardware_domain )
+    else if ( grant_access )
     {
-        ret = irq_permit_access(hardware_domain, irq);
+        struct domain *currd = current->domain;
+
+        ret = irq_permit_access(currd, irq);
         if ( ret )
             printk(XENLOG_G_ERR
-                   "Could not grant Dom0 access to IRQ%d (error %d)\n",
-                   irq, ret);
+                   "Could not grant %pd access to IRQ%d (error %d)\n",
+                   currd, irq, ret);
+        else
+            desc->arch.creator_domid = currd->domain_id;
     }
 
     return irq;
@@ -307,14 +315,23 @@ void destroy_irq(unsigned int irq)
 
     BUG_ON(!MSI_IRQ(irq));
 
-    if ( hardware_domain )
+    if ( desc->arch.creator_domid != DOMID_INVALID )
     {
-        int err = irq_deny_access(hardware_domain, irq);
+        struct domain *d = get_domain_by_id(desc->arch.creator_domid);
 
-        if ( err )
-            printk(XENLOG_G_ERR
-                   "Could not revoke Dom0 access to IRQ%u (error %d)\n",
-                   irq, err);
+        if ( d )
+        {
+            int err = irq_deny_access(d, irq);
+
+            if ( err )
+                printk(XENLOG_G_ERR
+                       "Could not revoke %pd access to IRQ%u (error %d)\n",
+                       d, irq, err);
+
+            put_domain(d);
+        }
+
+        desc->arch.creator_domid = DOMID_INVALID;
     }
 
     spin_lock_irqsave(&desc->lock, flags);
@@ -381,6 +398,7 @@ int arch_init_one_irq_desc(struct irq_desc *desc)
 
     desc->arch.vector = IRQ_VECTOR_UNASSIGNED;
     desc->arch.old_vector = IRQ_VECTOR_UNASSIGNED;
+    desc->arch.creator_domid = DOMID_INVALID;
 
     return 0;
 }
@@ -2133,7 +2151,7 @@ int map_domain_pirq(
             spin_unlock_irqrestore(&desc->lock, flags);
 
             info = NULL;
-            irq = create_irq(NUMA_NO_NODE);
+            irq = create_irq(NUMA_NO_NODE, true);
             ret = irq >= 0 ? prepare_domain_irq_pirq(d, irq, pirq + nr, &info)
                            : irq;
             if ( ret < 0 )
@@ -2818,7 +2836,7 @@ int allocate_and_map_msi_pirq(struct domain *d, int 
index, int *pirq_p,
         if ( irq == -1 )
         {
     case MAP_PIRQ_TYPE_MULTI_MSI:
-            irq = create_irq(NUMA_NO_NODE);
+            irq = create_irq(NUMA_NO_NODE, true);
         }
 
         if ( irq < nr_irqs_gsi || irq >= nr_irqs )
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 8667de6d67..fcd3979a39 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -722,7 +722,7 @@ static void __init ns16550_init_irq(struct serial_port 
*port)
     struct ns16550 *uart = port->uart;
 
     if ( uart->msi )
-        uart->irq = create_irq(0);
+        uart->irq = create_irq(0, false);
 #endif
 }
 
diff --git a/xen/drivers/passthrough/amd/iommu_init.c 
b/xen/drivers/passthrough/amd/iommu_init.c
index 574f04dd81..6f53c7ec08 100644
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -766,7 +766,7 @@ static bool_t __init set_iommu_interrupt_handler(struct 
amd_iommu *iommu)
 {
     int irq, ret;
 
-    irq = create_irq(NUMA_NO_NODE);
+    irq = create_irq(NUMA_NO_NODE, false);
     if ( irq <= 0 )
     {
         dprintk(XENLOG_ERR, "IOMMU: no irqs\n");
diff --git a/xen/drivers/passthrough/vtd/iommu.c 
b/xen/drivers/passthrough/vtd/iommu.c
index 3c17f11386..f08eec070d 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1097,7 +1097,8 @@ static int __init iommu_set_interrupt(struct 
acpi_drhd_unit *drhd)
     struct irq_desc *desc;
 
     irq = create_irq(rhsa ? pxm_to_node(rhsa->proximity_domain)
-                          : NUMA_NO_NODE);
+                          : NUMA_NO_NODE,
+                     false);
     if ( irq <= 0 )
     {
         dprintk(XENLOG_ERR VTDPREFIX, "IOMMU: no irq available!\n");
diff --git a/xen/include/asm-x86/irq.h b/xen/include/asm-x86/irq.h
index d3124f7b5d..5f720d30d1 100644
--- a/xen/include/asm-x86/irq.h
+++ b/xen/include/asm-x86/irq.h
@@ -45,6 +45,11 @@ struct arch_irq_desc {
         unsigned move_cleanup_count;
         u8 move_in_progress : 1;
         s8 used;
+        /*
+         * Weak reference to domain having permission over this IRQ (which can
+         * be different from the domain actually having the IRQ assigned)
+         */
+        domid_t creator_domid;
 };
 
 /* For use with irq_desc.arch.used */
@@ -161,7 +166,11 @@ int  init_irq_data(void);
 void clear_irq_vector(int irq);
 
 int irq_to_vector(int irq);
-int create_irq(nodeid_t node);
+/*
+ * If grant_access is set the current domain is given permissions over
+ * the created IRQ.
+ */
+int create_irq(nodeid_t node, bool grant_access);
 void destroy_irq(unsigned int irq);
 int assign_irq_vector(int irq, const cpumask_t *);
 
--
generated by git-patchbot for /home/xen/git/xen.git#staging

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.