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

[Xen-devel] [PATCH 4/4] xen/x86: Rework inclusion between struct pirq and struct hvm_pirq_dpci



From: Julien Grall <jgrall@xxxxxxxxxx>

At the moment, alloc_pirq_struct() relies on the field 'arch' to be the
last member of the structure.

As this is used for computing the size of the structure, the value will
be miscomputed if a new field is added afterwards.

Such quirkiness makes quite difficult to understand how struct pirq
works. Given that struct hvm_pirq_dpci is only used in combination of a
struct pirq, we can inverse the inclusion. i.e pirq will now be
contained in struct hvm_pirq_dpci.

As the field pirq.arch.hvm.emuirq is as well HVM specific, this is now
moved in struct hvm_pirq_dpci.

There is a few side effects with this changes:
    - We now need to distinguish between PIRQ allocated for HVM and PV
      guests. This is to allow us to know what we are freeing.
    - container_of is not able to cater with const and non-const at the
      same time. So we need to introduce two macros (const and
      non-const).

Lastly all the HVM specific pirq code can now be moved in hvm/irq.h
allowing use to drop the include from irq.h. This is one less header
included treewide.

Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
---
 xen/arch/arm/irq.c            |  5 +++++
 xen/arch/x86/hvm/irq.c        |  7 ++++---
 xen/arch/x86/irq.c            | 39 ++++++++++++++++++++++++-----------
 xen/common/domain.c           |  7 +------
 xen/drivers/passthrough/io.c  |  1 +
 xen/include/asm-x86/hvm/irq.h | 19 +++++++++++++++++
 xen/include/asm-x86/irq.h     | 19 +++--------------
 xen/include/xen/domain.h      |  3 +++
 8 files changed, 63 insertions(+), 37 deletions(-)

diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 3877657a52..fd108ea3a5 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -582,6 +582,11 @@ struct pirq *alloc_pirq_struct(struct domain *d)
     return NULL;
 }
 
+void arch_free_pirq_struct(struct rcu_head *head)
+{
+    ASSERT_UNREACHABLE();
+}
+
 /*
  * These are all unreachable given an alloc_pirq_struct
  * which returns NULL, all callers try to lookup struct pirq first
diff --git a/xen/arch/x86/hvm/irq.c b/xen/arch/x86/hvm/irq.c
index c684422b24..e0bb0a8b90 100644
--- a/xen/arch/x86/hvm/irq.c
+++ b/xen/arch/x86/hvm/irq.c
@@ -29,7 +29,8 @@
 
 bool hvm_domain_use_pirq(const struct domain *d, const struct pirq *pirq)
 {
-    return is_hvm_domain(d) && pirq && pirq->arch.hvm.emuirq != IRQ_UNBOUND;
+    return is_hvm_domain(d) && pirq &&
+        const_pirq_dpci(pirq)->emuirq != IRQ_UNBOUND;
 }
 
 /* Must be called with hvm_domain->irq_lock hold */
@@ -396,7 +397,7 @@ int hvm_inject_msi(struct domain *d, uint64_t addr, 
uint32_t data)
             struct pirq *info = pirq_info(d, pirq);
 
             /* if it is the first time, allocate the pirq */
-            if ( !info || info->arch.hvm.emuirq == IRQ_UNBOUND )
+            if ( !info || pirq_dpci(info)->emuirq == IRQ_UNBOUND )
             {
                 int rc;
 
@@ -409,7 +410,7 @@ int hvm_inject_msi(struct domain *d, uint64_t addr, 
uint32_t data)
                 if ( !info )
                     return -EBUSY;
             }
-            else if ( info->arch.hvm.emuirq != IRQ_MSI_EMU )
+            else if ( pirq_dpci(info)->emuirq != IRQ_MSI_EMU )
                 return -EINVAL;
             send_guest_pirq(d, info);
             return 0;
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 310ac00a60..3e01101f88 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -1286,22 +1286,37 @@ void cleanup_domain_irq_mapping(struct domain *d)
 
 struct pirq *alloc_pirq_struct(struct domain *d)
 {
-    size_t sz = is_hvm_domain(d) ? sizeof(struct pirq) :
-                                   offsetof(struct pirq, arch.hvm);
-    struct pirq *pirq = xzalloc_bytes(sz);
+    struct pirq *pirq;
 
-    if ( pirq )
+    if ( is_hvm_domain(d) )
     {
-        if ( is_hvm_domain(d) )
+        struct hvm_pirq_dpci *dpci = xzalloc(struct hvm_pirq_dpci);
+
+        if ( dpci )
         {
-            pirq->arch.hvm.emuirq = IRQ_UNBOUND;
-            pt_pirq_init(d, &pirq->arch.hvm.dpci);
+            pt_pirq_init(d, dpci);
+            pirq = dpci_pirq(dpci);
+            pirq->arch.hvm = true;
         }
+        else
+            pirq = NULL;
     }
+    else
+        pirq = xzalloc(struct pirq);
 
     return pirq;
 }
 
+void arch_free_pirq_struct(struct rcu_head *head)
+{
+    struct pirq *pirq = container_of(head, struct pirq, rcu_head);
+
+    if ( pirq->arch.hvm )
+        xfree(pirq_dpci(pirq));
+    else
+        xfree(pirq);
+}
+
 void (pirq_cleanup_check)(struct pirq *pirq, struct domain *d)
 {
     /*
@@ -1315,9 +1330,9 @@ void (pirq_cleanup_check)(struct pirq *pirq, struct 
domain *d)
 
     if ( is_hvm_domain(d) )
     {
-        if ( pirq->arch.hvm.emuirq != IRQ_UNBOUND )
+        if ( pirq_dpci(pirq)->emuirq != IRQ_UNBOUND )
             return;
-        if ( !pt_pirq_cleanup_check(&pirq->arch.hvm.dpci) )
+        if ( !pt_pirq_cleanup_check(pirq_dpci(pirq)) )
             return;
     }
 
@@ -2029,7 +2044,7 @@ static inline bool is_free_pirq(const struct domain *d,
                                 const struct pirq *pirq)
 {
     return !pirq || (!pirq->arch.irq && (!is_hvm_domain(d) ||
-        pirq->arch.hvm.emuirq == IRQ_UNBOUND));
+        const_pirq_dpci(pirq)->emuirq == IRQ_UNBOUND));
 }
 
 int get_free_pirq(struct domain *d, int type)
@@ -2724,7 +2739,7 @@ int map_domain_emuirq_pirq(struct domain *d, int pirq, 
int emuirq)
             return err;
         }
     }
-    info->arch.hvm.emuirq = emuirq;
+    pirq_dpci(info)->emuirq = emuirq;
 
     return 0;
 }
@@ -2754,7 +2769,7 @@ int unmap_domain_pirq_emuirq(struct domain *d, int pirq)
     info = pirq_info(d, pirq);
     if ( info )
     {
-        info->arch.hvm.emuirq = IRQ_UNBOUND;
+        pirq_dpci(info)->emuirq = IRQ_UNBOUND;
         pirq_cleanup_check(info, d);
     }
     if ( emuirq != IRQ_PT )
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 0b1103fdb2..7f04da79e6 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1625,16 +1625,11 @@ struct pirq *pirq_get_info(struct domain *d, int pirq)
     return info;
 }
 
-static void _free_pirq_struct(struct rcu_head *head)
-{
-    xfree(container_of(head, struct pirq, rcu_head));
-}
-
 void free_pirq_struct(void *ptr)
 {
     struct pirq *pirq = ptr;
 
-    call_rcu(&pirq->rcu_head, _free_pirq_struct);
+    call_rcu(&pirq->rcu_head, arch_free_pirq_struct);
 }
 
 struct migrate_info {
diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/io.c
index b292e79382..e7b288b4aa 100644
--- a/xen/drivers/passthrough/io.c
+++ b/xen/drivers/passthrough/io.c
@@ -769,6 +769,7 @@ int pt_irq_destroy_bind(
 
 void pt_pirq_init(struct domain *d, struct hvm_pirq_dpci *dpci)
 {
+    dpci->emuirq = IRQ_UNBOUND;
     INIT_LIST_HEAD(&dpci->digl_list);
     dpci->gmsi.dest_vcpu_id = -1;
 }
diff --git a/xen/include/asm-x86/hvm/irq.h b/xen/include/asm-x86/hvm/irq.h
index 5b7e90c179..0ccfaad53b 100644
--- a/xen/include/asm-x86/hvm/irq.h
+++ b/xen/include/asm-x86/hvm/irq.h
@@ -21,6 +21,7 @@
 #ifndef __ASM_X86_HVM_IRQ_H__
 #define __ASM_X86_HVM_IRQ_H__
 
+#include <xen/irq.h>
 #include <xen/timer.h>
 
 #include <asm/hvm/hvm.h>
@@ -171,8 +172,26 @@ struct hvm_pirq_dpci {
     struct hvm_gmsi_info gmsi;
     struct timer timer;
     struct list_head softirq_list;
+    int emuirq;
+    struct pirq pirq;
 };
 
+#define pirq_dpci(p)                                                    \
+    ((p) ? container_of(p, struct hvm_pirq_dpci, pirq) : NULL)
+#define const_pirq_dpci(p)                                              \
+    ((p) ? container_of(p, const struct hvm_pirq_dpci, pirq) : NULL)
+
+#define dpci_pirq(pd) (&(pd)->pirq)
+
+#define domain_pirq_to_emuirq(d, p) ({                                  \
+    struct pirq *__pi = pirq_info(d, p);                                \
+    __pi ? pirq_dpci(__pi)->emuirq : IRQ_UNBOUND;                       \
+})
+#define domain_emuirq_to_pirq(d, emuirq) ({                             \
+    void *__ret = radix_tree_lookup(&(d)->arch.hvm.emuirq_pirq, emuirq);\
+    __ret ? radix_tree_ptr_to_int(__ret) : IRQ_UNBOUND;                 \
+})
+
 void pt_pirq_init(struct domain *, struct hvm_pirq_dpci *);
 bool pt_pirq_cleanup_check(struct hvm_pirq_dpci *);
 int pt_pirq_iterate(struct domain *d,
diff --git a/xen/include/asm-x86/irq.h b/xen/include/asm-x86/irq.h
index 44aefc8f03..07a63bae04 100644
--- a/xen/include/asm-x86/irq.h
+++ b/xen/include/asm-x86/irq.h
@@ -8,7 +8,6 @@
 #include <xen/cpumask.h>
 #include <xen/percpu.h>
 #include <xen/smp.h>
-#include <asm/hvm/irq.h>
 
 extern unsigned int nr_irqs_gsi;
 extern unsigned int nr_irqs;
@@ -133,17 +132,10 @@ DECLARE_PER_CPU(unsigned int, irq_count);
 
 struct arch_pirq {
     int irq;
-    union {
-        struct hvm_pirq {
-            int emuirq;
-            struct hvm_pirq_dpci dpci;
-        } hvm;
-    };
+    /* Is the PIRQ associated to an HVM domain? */
+    bool hvm;
 };
 
-#define pirq_dpci(pirq) ((pirq) ? &(pirq)->arch.hvm.dpci : NULL)
-#define dpci_pirq(pd) container_of(pd, struct pirq, arch.hvm.dpci)
-
 int pirq_shared(struct domain *d , int irq);
 
 int map_domain_pirq(struct domain *d, int pirq, int irq, int type,
@@ -198,12 +190,7 @@ void cleanup_domain_irq_mapping(struct domain *);
     __ret ? radix_tree_ptr_to_int(__ret) : 0;                   \
 })
 #define PIRQ_ALLOCATED -1
-#define domain_pirq_to_emuirq(d, pirq) pirq_field(d, pirq,              \
-    arch.hvm.emuirq, IRQ_UNBOUND)
-#define domain_emuirq_to_pirq(d, emuirq) ({                             \
-    void *__ret = radix_tree_lookup(&(d)->arch.hvm.emuirq_pirq, emuirq);\
-    __ret ? radix_tree_ptr_to_int(__ret) : IRQ_UNBOUND;                 \
-})
+
 #define IRQ_UNBOUND -1
 #define IRQ_PT -2
 #define IRQ_MSI_EMU -3
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index 89bf0a1721..99aea630d4 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -42,6 +42,9 @@ void free_vcpu_struct(struct vcpu *v);
 
 /* Allocate/free a PIRQ structure. */
 struct pirq *alloc_pirq_struct(struct domain *);
+
+/* Per-arch callback used by the RCU */
+void arch_free_pirq_struct(struct rcu_head *head);
 void free_pirq_struct(void *);
 
 /*
-- 
2.24.0


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

 


Rackspace

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