diff mbox

[v3,7/9] vpci: add a priority field to the vPCI register initializer

Message ID 20170427143546.14662-8-roger.pau@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Roger Pau Monné April 27, 2017, 2:35 p.m. UTC 
And mark the capability and header vPCI register initializers as high priority,
so that they are initialized first.

This is needed for MSI-X, since MSI-X needs to know the position of the BARs in
order to perform it's initialization, and in order to mask or enable the
MSI/MSI-X functionality on demand.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
 tools/tests/vpci/Makefile       |  4 ++--
 xen/drivers/vpci/capabilities.c |  2 +-
 xen/drivers/vpci/header.c       |  2 +-
 xen/drivers/vpci/vpci.c         | 14 ++++++++++++--
 xen/include/xen/vpci.h          | 13 +++++++++++--
 5 files changed, 27 insertions(+), 8 deletions(-)

Comments

Jan Beulich May 23, 2017, 12:52 p.m. UTC | #1 
>>> On 27.04.17 at 16:35, <roger.pau@citrix.com> wrote:
> +#define REGISTER_VPCI_INIT(f, p)                                        \
> +  static const struct vpci_register_init                                \
> +                      x##_entry __used_section(".data.vpci") = {        \
> +    .init = f,                                                          \
> +    .priority = p,                                                      \
> +}

I think I'd rather see this done by ordering the entries in
.data.vpci suitably, e.g. by adding numeric tags and using SORT()
or some such in the linker script. Iirc upstream Linux did change to
such a model for some of their initialization, so you may be able to
glean something there.

Jan
Roger Pau Monné June 26, 2017, 2:41 p.m. UTC | #2 
On Tue, May 23, 2017 at 06:52:42AM -0600, Jan Beulich wrote:
> >>> On 27.04.17 at 16:35, <roger.pau@citrix.com> wrote:
> > +#define REGISTER_VPCI_INIT(f, p)                                        \
> > +  static const struct vpci_register_init                                \
> > +                      x##_entry __used_section(".data.vpci") = {        \
> > +    .init = f,                                                          \
> > +    .priority = p,                                                      \
> > +}
> 
> I think I'd rather see this done by ordering the entries in
> .data.vpci suitably, e.g. by adding numeric tags and using SORT()
> or some such in the linker script. Iirc upstream Linux did change to
> such a model for some of their initialization, so you may be able to
> glean something there.

Thanks, I've now switched to using SORT and a numeric suffix to the
section used by each entry. This looks much better and requires less
code changes.

Roger.
diff mbox

Patch

diff --git a/tools/tests/vpci/Makefile b/tools/tests/vpci/Makefile
index 7969fcbd82..e5edc4f512 100644
--- a/tools/tests/vpci/Makefile
+++ b/tools/tests/vpci/Makefile
@@ -31,8 +31,8 @@  vpci.c: $(XEN_ROOT)/xen/drivers/vpci/vpci.c
 	# Trick the compiler so it doesn't complain about missing symbols
 	sed -e '/#include/d' \
 	    -e '1s;^;#include "emul.h"\
-	             const vpci_register_init_t __start_vpci_array[1]\;\
-	             const vpci_register_init_t __end_vpci_array[1]\;\
+	             const struct vpci_register_init __start_vpci_array[1]\;\
+	             const struct vpci_register_init __end_vpci_array[1]\;\
 	             ;' <$< >$@
 
 rbtree.h: $(XEN_ROOT)/xen/include/xen/rbtree.h
diff --git a/xen/drivers/vpci/capabilities.c b/xen/drivers/vpci/capabilities.c
index b2a3326aa7..204355e673 100644
--- a/xen/drivers/vpci/capabilities.c
+++ b/xen/drivers/vpci/capabilities.c
@@ -145,7 +145,7 @@  static int vpci_capabilities_init(struct pci_dev *pdev)
     return 0;
 }
 
-REGISTER_VPCI_INIT(vpci_capabilities_init);
+REGISTER_VPCI_INIT(vpci_capabilities_init, true);
 
 /*
  * Local variables:
diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index a401cf6915..3deec53efd 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -278,7 +278,7 @@  static int vpci_init_bars(struct pci_dev *pdev)
     return 0;
 }
 
-REGISTER_VPCI_INIT(vpci_init_bars);
+REGISTER_VPCI_INIT(vpci_init_bars, true);
 
 /*
  * Local variables:
diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index b159f0db80..e6154b742e 100644
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -20,7 +20,7 @@ 
 #include <xen/sched.h>
 #include <xen/vpci.h>
 
-extern const vpci_register_init_t __start_vpci_array[], __end_vpci_array[];
+extern const struct vpci_register_init __start_vpci_array[], __end_vpci_array[];
 #define NUM_VPCI_INIT (__end_vpci_array - __start_vpci_array)
 #define vpci_init __start_vpci_array
 
@@ -37,6 +37,7 @@  struct vpci_register {
 int xen_vpci_add_handlers(struct pci_dev *pdev)
 {
     int i, rc = 0;
+    bool priority = true;
 
     if ( !has_vpci(pdev->domain) )
         return 0;
@@ -47,9 +48,13 @@  int xen_vpci_add_handlers(struct pci_dev *pdev)
 
     pdev->vpci->handlers = RB_ROOT;
 
+ again:
     for ( i = 0; i < NUM_VPCI_INIT; i++ )
     {
-        rc = vpci_init[i](pdev);
+        if ( priority != vpci_init[i].priority )
+            continue;
+
+        rc = vpci_init[i].init(pdev);
         if ( rc )
             break;
     }
@@ -69,6 +74,11 @@  int xen_vpci_add_handlers(struct pci_dev *pdev)
         }
         xfree(pdev->vpci);
     }
+    else if ( priority )
+    {
+        priority = false;
+        goto again;
+    }
 
     return rc;
 }
diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h
index d41277f39b..2bf61d6c15 100644
--- a/xen/include/xen/vpci.h
+++ b/xen/include/xen/vpci.h
@@ -29,8 +29,17 @@  typedef int (*vpci_write_t)(struct pci_dev *pdev, unsigned int reg,
 
 typedef int (*vpci_register_init_t)(struct pci_dev *dev);
 
-#define REGISTER_VPCI_INIT(x) \
-  static const vpci_register_init_t x##_entry __used_section(".data.vpci") = x
+struct vpci_register_init {
+    vpci_register_init_t init;
+    bool priority;
+};
+
+#define REGISTER_VPCI_INIT(f, p)                                        \
+  static const struct vpci_register_init                                \
+                      x##_entry __used_section(".data.vpci") = {        \
+    .init = f,                                                          \
+    .priority = p,                                                      \
+}
 
 /* Add vPCI handlers to device. */
 int xen_vpci_add_handlers(struct pci_dev *dev);