All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xen-pciback: notify hypervisor about devices intended to be assigned to guests
@ 2013-02-06 16:58 Jan Beulich
  2013-02-06 17:12 ` Konrad Rzeszutek Wilk
  2013-02-06 17:12 ` Konrad Rzeszutek Wilk
  0 siblings, 2 replies; 10+ messages in thread
From: Jan Beulich @ 2013-02-06 16:58 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: xen-devel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 5410 bytes --]

For MSI-X capable devices the hypervisor wants to write protect the
MSI-X table and PBA, yet it can't assume that resources have been
assigned to their final values at device enumeration time. Thus have
pciback do that notification, as having the device controlled by it is
a prerequisite to assigning the device to guests anyway.

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

---
 arch/x86/include/asm/xen/hypercall.h |    4 +-
 drivers/xen/fallback.c               |    3 +
 drivers/xen/xen-pciback/pci_stub.c   |   59 ++++++++++++++++++++++++++---------
 include/xen/interface/physdev.h      |    6 +++
 4 files changed, 54 insertions(+), 18 deletions(-)

--- 3.8-rc6/arch/x86/include/asm/xen/hypercall.h
+++ 3.8-rc6-xen-pciback-MSI-X-prepare/arch/x86/include/asm/xen/hypercall.h
@@ -382,14 +382,14 @@ HYPERVISOR_console_io(int cmd, int count
 	return _hypercall3(int, console_io, cmd, count, str);
 }
 
-extern int __must_check HYPERVISOR_physdev_op_compat(int, void *);
+extern int __must_check xen_physdev_op_compat(int, void *);
 
 static inline int
 HYPERVISOR_physdev_op(int cmd, void *arg)
 {
 	int rc = _hypercall2(int, physdev_op, cmd, arg);
 	if (unlikely(rc == -ENOSYS))
-		rc = HYPERVISOR_physdev_op_compat(cmd, arg);
+		rc = xen_physdev_op_compat(cmd, arg);
 	return rc;
 }
 
--- 3.8-rc6/drivers/xen/fallback.c
+++ 3.8-rc6-xen-pciback-MSI-X-prepare/drivers/xen/fallback.c
@@ -44,7 +44,7 @@ int xen_event_channel_op_compat(int cmd,
 }
 EXPORT_SYMBOL_GPL(xen_event_channel_op_compat);
 
-int HYPERVISOR_physdev_op_compat(int cmd, void *arg)
+int xen_physdev_op_compat(int cmd, void *arg)
 {
 	struct physdev_op op;
 	int rc;
@@ -78,3 +78,4 @@ int HYPERVISOR_physdev_op_compat(int cmd
 
 	return rc;
 }
+EXPORT_SYMBOL_GPL(xen_physdev_op_compat);
--- 3.8-rc6/drivers/xen/xen-pciback/pci_stub.c
+++ 3.8-rc6-xen-pciback-MSI-X-prepare/drivers/xen/xen-pciback/pci_stub.c
@@ -17,6 +17,7 @@
 #include <xen/events.h>
 #include <asm/xen/pci.h>
 #include <asm/xen/hypervisor.h>
+#include <xen/interface/physdev.h>
 #include "pciback.h"
 #include "conf_space.h"
 #include "conf_space_quirks.h"
@@ -85,37 +86,52 @@ static struct pcistub_device *pcistub_de
 static void pcistub_device_release(struct kref *kref)
 {
 	struct pcistub_device *psdev;
+	struct pci_dev *dev;
 	struct xen_pcibk_dev_data *dev_data;
 
 	psdev = container_of(kref, struct pcistub_device, kref);
-	dev_data = pci_get_drvdata(psdev->dev);
+	dev = psdev->dev;
+	dev_data = pci_get_drvdata(dev);
 
-	dev_dbg(&psdev->dev->dev, "pcistub_device_release\n");
+	dev_dbg(&dev->dev, "pcistub_device_release\n");
 
-	xen_unregister_device_domain_owner(psdev->dev);
+	xen_unregister_device_domain_owner(dev);
 
 	/* Call the reset function which does not take lock as this
 	 * is called from "unbind" which takes a device_lock mutex.
 	 */
-	__pci_reset_function_locked(psdev->dev);
-	if (pci_load_and_free_saved_state(psdev->dev,
-					  &dev_data->pci_saved_state)) {
-		dev_dbg(&psdev->dev->dev, "Could not reload PCI state\n");
-	} else
-		pci_restore_state(psdev->dev);
+	__pci_reset_function_locked(dev);
+	if (pci_load_and_free_saved_state(dev, &dev_data->pci_saved_state))
+		dev_dbg(&dev->dev, "Could not reload PCI state\n");
+	else
+		pci_restore_state(dev);
+
+	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
+		struct physdev_pci_device ppdev = {
+			.seg = pci_domain_nr(dev->bus),
+			.bus = dev->bus->number,
+			.devfn = dev->devfn
+		};
+		int err = HYPERVISOR_physdev_op(PHYSDEVOP_release_msix,
+						&ppdev);
+
+		if (err)
+			dev_warn(&dev->dev, "MSI-X release failed (%d)\n",
+				 err);
+	}
 
 	/* Disable the device */
-	xen_pcibk_reset_device(psdev->dev);
+	xen_pcibk_reset_device(dev);
 
 	kfree(dev_data);
-	pci_set_drvdata(psdev->dev, NULL);
+	pci_set_drvdata(dev, NULL);
 
 	/* Clean-up the device */
-	xen_pcibk_config_free_dyn_fields(psdev->dev);
-	xen_pcibk_config_free_dev(psdev->dev);
+	xen_pcibk_config_free_dyn_fields(dev);
+	xen_pcibk_config_free_dev(dev);
 
-	psdev->dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
-	pci_dev_put(psdev->dev);
+	dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
+	pci_dev_put(dev);
 
 	kfree(psdev);
 }
@@ -355,6 +371,19 @@ static int pcistub_init_device(struct pc
 	if (err)
 		goto config_release;
 
+	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
+		struct physdev_pci_device ppdev = {
+			.seg = pci_domain_nr(dev->bus),
+			.bus = dev->bus->number,
+			.devfn = dev->devfn
+		};
+
+		err = HYPERVISOR_physdev_op(PHYSDEVOP_prepare_msix, &ppdev);
+		if (err)
+			dev_err(&dev->dev, "MSI-X preparation failed (%d)\n",
+				err);
+	}
+
 	/* We need the device active to save the state. */
 	dev_dbg(&dev->dev, "save state of device\n");
 	pci_save_state(dev);
--- 3.8-rc6/include/xen/interface/physdev.h
+++ 3.8-rc6-xen-pciback-MSI-X-prepare/include/xen/interface/physdev.h
@@ -251,6 +251,12 @@ struct physdev_pci_device_add {
 
 #define PHYSDEVOP_pci_device_remove     26
 #define PHYSDEVOP_restore_msi_ext       27
+/*
+ * Dom0 should use these two to announce MMIO resources assigned to
+ * MSI-X capable devices won't (prepare) or may (release) change.
+ */
+#define PHYSDEVOP_prepare_msix          30
+#define PHYSDEVOP_release_msix          31
 struct physdev_pci_device {
     /* IN */
     uint16_t seg;



[-- Attachment #2: linux-3.8-rc6-xen-pciback-MSI-X-prepare.patch --]
[-- Type: text/plain, Size: 5488 bytes --]

xen-pciback: notify hypervisor about devices intended to be assigned to guests

For MSI-X capable devices the hypervisor wants to write protect the
MSI-X table and PBA, yet it can't assume that resources have been
assigned to their final values at device enumeration time. Thus have
pciback do that notification, as having the device controlled by it is
a prerequisite to assigning the device to guests anyway.

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

---
 arch/x86/include/asm/xen/hypercall.h |    4 +-
 drivers/xen/fallback.c               |    3 +
 drivers/xen/xen-pciback/pci_stub.c   |   59 ++++++++++++++++++++++++++---------
 include/xen/interface/physdev.h      |    6 +++
 4 files changed, 54 insertions(+), 18 deletions(-)

--- 3.8-rc6/arch/x86/include/asm/xen/hypercall.h
+++ 3.8-rc6-xen-pciback-MSI-X-prepare/arch/x86/include/asm/xen/hypercall.h
@@ -382,14 +382,14 @@ HYPERVISOR_console_io(int cmd, int count
 	return _hypercall3(int, console_io, cmd, count, str);
 }
 
-extern int __must_check HYPERVISOR_physdev_op_compat(int, void *);
+extern int __must_check xen_physdev_op_compat(int, void *);
 
 static inline int
 HYPERVISOR_physdev_op(int cmd, void *arg)
 {
 	int rc = _hypercall2(int, physdev_op, cmd, arg);
 	if (unlikely(rc == -ENOSYS))
-		rc = HYPERVISOR_physdev_op_compat(cmd, arg);
+		rc = xen_physdev_op_compat(cmd, arg);
 	return rc;
 }
 
--- 3.8-rc6/drivers/xen/fallback.c
+++ 3.8-rc6-xen-pciback-MSI-X-prepare/drivers/xen/fallback.c
@@ -44,7 +44,7 @@ int xen_event_channel_op_compat(int cmd,
 }
 EXPORT_SYMBOL_GPL(xen_event_channel_op_compat);
 
-int HYPERVISOR_physdev_op_compat(int cmd, void *arg)
+int xen_physdev_op_compat(int cmd, void *arg)
 {
 	struct physdev_op op;
 	int rc;
@@ -78,3 +78,4 @@ int HYPERVISOR_physdev_op_compat(int cmd
 
 	return rc;
 }
+EXPORT_SYMBOL_GPL(xen_physdev_op_compat);
--- 3.8-rc6/drivers/xen/xen-pciback/pci_stub.c
+++ 3.8-rc6-xen-pciback-MSI-X-prepare/drivers/xen/xen-pciback/pci_stub.c
@@ -17,6 +17,7 @@
 #include <xen/events.h>
 #include <asm/xen/pci.h>
 #include <asm/xen/hypervisor.h>
+#include <xen/interface/physdev.h>
 #include "pciback.h"
 #include "conf_space.h"
 #include "conf_space_quirks.h"
@@ -85,37 +86,52 @@ static struct pcistub_device *pcistub_de
 static void pcistub_device_release(struct kref *kref)
 {
 	struct pcistub_device *psdev;
+	struct pci_dev *dev;
 	struct xen_pcibk_dev_data *dev_data;
 
 	psdev = container_of(kref, struct pcistub_device, kref);
-	dev_data = pci_get_drvdata(psdev->dev);
+	dev = psdev->dev;
+	dev_data = pci_get_drvdata(dev);
 
-	dev_dbg(&psdev->dev->dev, "pcistub_device_release\n");
+	dev_dbg(&dev->dev, "pcistub_device_release\n");
 
-	xen_unregister_device_domain_owner(psdev->dev);
+	xen_unregister_device_domain_owner(dev);
 
 	/* Call the reset function which does not take lock as this
 	 * is called from "unbind" which takes a device_lock mutex.
 	 */
-	__pci_reset_function_locked(psdev->dev);
-	if (pci_load_and_free_saved_state(psdev->dev,
-					  &dev_data->pci_saved_state)) {
-		dev_dbg(&psdev->dev->dev, "Could not reload PCI state\n");
-	} else
-		pci_restore_state(psdev->dev);
+	__pci_reset_function_locked(dev);
+	if (pci_load_and_free_saved_state(dev, &dev_data->pci_saved_state))
+		dev_dbg(&dev->dev, "Could not reload PCI state\n");
+	else
+		pci_restore_state(dev);
+
+	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
+		struct physdev_pci_device ppdev = {
+			.seg = pci_domain_nr(dev->bus),
+			.bus = dev->bus->number,
+			.devfn = dev->devfn
+		};
+		int err = HYPERVISOR_physdev_op(PHYSDEVOP_release_msix,
+						&ppdev);
+
+		if (err)
+			dev_warn(&dev->dev, "MSI-X release failed (%d)\n",
+				 err);
+	}
 
 	/* Disable the device */
-	xen_pcibk_reset_device(psdev->dev);
+	xen_pcibk_reset_device(dev);
 
 	kfree(dev_data);
-	pci_set_drvdata(psdev->dev, NULL);
+	pci_set_drvdata(dev, NULL);
 
 	/* Clean-up the device */
-	xen_pcibk_config_free_dyn_fields(psdev->dev);
-	xen_pcibk_config_free_dev(psdev->dev);
+	xen_pcibk_config_free_dyn_fields(dev);
+	xen_pcibk_config_free_dev(dev);
 
-	psdev->dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
-	pci_dev_put(psdev->dev);
+	dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
+	pci_dev_put(dev);
 
 	kfree(psdev);
 }
@@ -355,6 +371,19 @@ static int pcistub_init_device(struct pc
 	if (err)
 		goto config_release;
 
+	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
+		struct physdev_pci_device ppdev = {
+			.seg = pci_domain_nr(dev->bus),
+			.bus = dev->bus->number,
+			.devfn = dev->devfn
+		};
+
+		err = HYPERVISOR_physdev_op(PHYSDEVOP_prepare_msix, &ppdev);
+		if (err)
+			dev_err(&dev->dev, "MSI-X preparation failed (%d)\n",
+				err);
+	}
+
 	/* We need the device active to save the state. */
 	dev_dbg(&dev->dev, "save state of device\n");
 	pci_save_state(dev);
--- 3.8-rc6/include/xen/interface/physdev.h
+++ 3.8-rc6-xen-pciback-MSI-X-prepare/include/xen/interface/physdev.h
@@ -251,6 +251,12 @@ struct physdev_pci_device_add {
 
 #define PHYSDEVOP_pci_device_remove     26
 #define PHYSDEVOP_restore_msi_ext       27
+/*
+ * Dom0 should use these two to announce MMIO resources assigned to
+ * MSI-X capable devices won't (prepare) or may (release) change.
+ */
+#define PHYSDEVOP_prepare_msix          30
+#define PHYSDEVOP_release_msix          31
 struct physdev_pci_device {
     /* IN */
     uint16_t seg;

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

* Re: [PATCH] xen-pciback: notify hypervisor about devices intended to be assigned to guests
  2013-02-06 16:58 [PATCH] xen-pciback: notify hypervisor about devices intended to be assigned to guests Jan Beulich
@ 2013-02-06 17:12 ` Konrad Rzeszutek Wilk
  2013-02-07  9:00   ` Jan Beulich
  2013-02-07  9:00   ` Jan Beulich
  2013-02-06 17:12 ` Konrad Rzeszutek Wilk
  1 sibling, 2 replies; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-02-06 17:12 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel, linux-kernel

On Wed, Feb 06, 2013 at 04:58:01PM +0000, Jan Beulich wrote:
> For MSI-X capable devices the hypervisor wants to write protect the
> MSI-X table and PBA, yet it can't assume that resources have been
> assigned to their final values at device enumeration time. Thus have
> pciback do that notification, as having the device controlled by it is
> a prerequisite to assigning the device to guests anyway.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> ---
>  arch/x86/include/asm/xen/hypercall.h |    4 +-
>  drivers/xen/fallback.c               |    3 +
>  drivers/xen/xen-pciback/pci_stub.c   |   59 ++++++++++++++++++++++++++---------
>  include/xen/interface/physdev.h      |    6 +++
>  4 files changed, 54 insertions(+), 18 deletions(-)
> 
> --- 3.8-rc6/arch/x86/include/asm/xen/hypercall.h
> +++ 3.8-rc6-xen-pciback-MSI-X-prepare/arch/x86/include/asm/xen/hypercall.h
> @@ -382,14 +382,14 @@ HYPERVISOR_console_io(int cmd, int count
>  	return _hypercall3(int, console_io, cmd, count, str);
>  }
>  
> -extern int __must_check HYPERVISOR_physdev_op_compat(int, void *);
> +extern int __must_check xen_physdev_op_compat(int, void *);
>  
>  static inline int
>  HYPERVISOR_physdev_op(int cmd, void *arg)
>  {
>  	int rc = _hypercall2(int, physdev_op, cmd, arg);
>  	if (unlikely(rc == -ENOSYS))
> -		rc = HYPERVISOR_physdev_op_compat(cmd, arg);
> +		rc = xen_physdev_op_compat(cmd, arg);
>  	return rc;
>  }
>  
> --- 3.8-rc6/drivers/xen/fallback.c
> +++ 3.8-rc6-xen-pciback-MSI-X-prepare/drivers/xen/fallback.c
> @@ -44,7 +44,7 @@ int xen_event_channel_op_compat(int cmd,
>  }
>  EXPORT_SYMBOL_GPL(xen_event_channel_op_compat);
>  
> -int HYPERVISOR_physdev_op_compat(int cmd, void *arg)
> +int xen_physdev_op_compat(int cmd, void *arg)
>  {
>  	struct physdev_op op;
>  	int rc;
> @@ -78,3 +78,4 @@ int HYPERVISOR_physdev_op_compat(int cmd
>  
>  	return rc;
>  }
> +EXPORT_SYMBOL_GPL(xen_physdev_op_compat);
> --- 3.8-rc6/drivers/xen/xen-pciback/pci_stub.c
> +++ 3.8-rc6-xen-pciback-MSI-X-prepare/drivers/xen/xen-pciback/pci_stub.c
> @@ -17,6 +17,7 @@
>  #include <xen/events.h>
>  #include <asm/xen/pci.h>
>  #include <asm/xen/hypervisor.h>
> +#include <xen/interface/physdev.h>
>  #include "pciback.h"
>  #include "conf_space.h"
>  #include "conf_space_quirks.h"
> @@ -85,37 +86,52 @@ static struct pcistub_device *pcistub_de
>  static void pcistub_device_release(struct kref *kref)
>  {
>  	struct pcistub_device *psdev;
> +	struct pci_dev *dev;
>  	struct xen_pcibk_dev_data *dev_data;
>  
>  	psdev = container_of(kref, struct pcistub_device, kref);
> -	dev_data = pci_get_drvdata(psdev->dev);
> +	dev = psdev->dev;
> +	dev_data = pci_get_drvdata(dev);
>  
> -	dev_dbg(&psdev->dev->dev, "pcistub_device_release\n");
> +	dev_dbg(&dev->dev, "pcistub_device_release\n");
>  
> -	xen_unregister_device_domain_owner(psdev->dev);
> +	xen_unregister_device_domain_owner(dev);
>  
>  	/* Call the reset function which does not take lock as this
>  	 * is called from "unbind" which takes a device_lock mutex.
>  	 */
> -	__pci_reset_function_locked(psdev->dev);
> -	if (pci_load_and_free_saved_state(psdev->dev,
> -					  &dev_data->pci_saved_state)) {
> -		dev_dbg(&psdev->dev->dev, "Could not reload PCI state\n");
> -	} else
> -		pci_restore_state(psdev->dev);
> +	__pci_reset_function_locked(dev);
> +	if (pci_load_and_free_saved_state(dev, &dev_data->pci_saved_state))
> +		dev_dbg(&dev->dev, "Could not reload PCI state\n");
> +	else
> +		pci_restore_state(dev);
> +
> +	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
> +		struct physdev_pci_device ppdev = {
> +			.seg = pci_domain_nr(dev->bus),
> +			.bus = dev->bus->number,
> +			.devfn = dev->devfn
> +		};
> +		int err = HYPERVISOR_physdev_op(PHYSDEVOP_release_msix,
> +						&ppdev);
> +
> +		if (err)
> +			dev_warn(&dev->dev, "MSI-X release failed (%d)\n",
> +				 err);
> +	}
>  
>  	/* Disable the device */
> -	xen_pcibk_reset_device(psdev->dev);
> +	xen_pcibk_reset_device(dev);
>  
>  	kfree(dev_data);
> -	pci_set_drvdata(psdev->dev, NULL);
> +	pci_set_drvdata(dev, NULL);
>  
>  	/* Clean-up the device */
> -	xen_pcibk_config_free_dyn_fields(psdev->dev);
> -	xen_pcibk_config_free_dev(psdev->dev);
> +	xen_pcibk_config_free_dyn_fields(dev);
> +	xen_pcibk_config_free_dev(dev);
>  
> -	psdev->dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
> -	pci_dev_put(psdev->dev);
> +	dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
> +	pci_dev_put(dev);
>  
>  	kfree(psdev);
>  }
> @@ -355,6 +371,19 @@ static int pcistub_init_device(struct pc
>  	if (err)
>  		goto config_release;
>  
> +	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
> +		struct physdev_pci_device ppdev = {
> +			.seg = pci_domain_nr(dev->bus),
> +			.bus = dev->bus->number,
> +			.devfn = dev->devfn
> +		};
> +
> +		err = HYPERVISOR_physdev_op(PHYSDEVOP_prepare_msix, &ppdev);
> +		if (err)
> +			dev_err(&dev->dev, "MSI-X preparation failed (%d)\n",
> +				err);
> +	}
> +
>  	/* We need the device active to save the state. */
>  	dev_dbg(&dev->dev, "save state of device\n");
>  	pci_save_state(dev);
> --- 3.8-rc6/include/xen/interface/physdev.h
> +++ 3.8-rc6-xen-pciback-MSI-X-prepare/include/xen/interface/physdev.h
> @@ -251,6 +251,12 @@ struct physdev_pci_device_add {
>  
>  #define PHYSDEVOP_pci_device_remove     26
>  #define PHYSDEVOP_restore_msi_ext       27
> +/*
> + * Dom0 should use these two to announce MMIO resources assigned to
> + * MSI-X capable devices won't (prepare) or may (release) change.
> + */
> +#define PHYSDEVOP_prepare_msix          30
> +#define PHYSDEVOP_release_msix          31
>  struct physdev_pci_device {
>      /* IN */
>      uint16_t seg;
> 
> 

> xen-pciback: notify hypervisor about devices intended to be assigned to guests
> 
> For MSI-X capable devices the hypervisor wants to write protect the
> MSI-X table and PBA, yet it can't assume that resources have been
> assigned to their final values at device enumeration time. Thus have
> pciback do that notification, as having the device controlled by it is
> a prerequisite to assigning the device to guests anyway.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> ---
>  arch/x86/include/asm/xen/hypercall.h |    4 +-
>  drivers/xen/fallback.c               |    3 +
>  drivers/xen/xen-pciback/pci_stub.c   |   59 ++++++++++++++++++++++++++---------
>  include/xen/interface/physdev.h      |    6 +++
>  4 files changed, 54 insertions(+), 18 deletions(-)
> 
> --- 3.8-rc6/arch/x86/include/asm/xen/hypercall.h
> +++ 3.8-rc6-xen-pciback-MSI-X-prepare/arch/x86/include/asm/xen/hypercall.h
> @@ -382,14 +382,14 @@ HYPERVISOR_console_io(int cmd, int count
>  	return _hypercall3(int, console_io, cmd, count, str);
>  }
>  
> -extern int __must_check HYPERVISOR_physdev_op_compat(int, void *);
> +extern int __must_check xen_physdev_op_compat(int, void *);
>  
>  static inline int
>  HYPERVISOR_physdev_op(int cmd, void *arg)
>  {
>  	int rc = _hypercall2(int, physdev_op, cmd, arg);
>  	if (unlikely(rc == -ENOSYS))
> -		rc = HYPERVISOR_physdev_op_compat(cmd, arg);
> +		rc = xen_physdev_op_compat(cmd, arg);
>  	return rc;
>  }
>  
> --- 3.8-rc6/drivers/xen/fallback.c
> +++ 3.8-rc6-xen-pciback-MSI-X-prepare/drivers/xen/fallback.c
> @@ -44,7 +44,7 @@ int xen_event_channel_op_compat(int cmd,
>  }
>  EXPORT_SYMBOL_GPL(xen_event_channel_op_compat);
>  
> -int HYPERVISOR_physdev_op_compat(int cmd, void *arg)
> +int xen_physdev_op_compat(int cmd, void *arg)
>  {
>  	struct physdev_op op;
>  	int rc;
> @@ -78,3 +78,4 @@ int HYPERVISOR_physdev_op_compat(int cmd
>  
>  	return rc;
>  }
> +EXPORT_SYMBOL_GPL(xen_physdev_op_compat);
> --- 3.8-rc6/drivers/xen/xen-pciback/pci_stub.c
> +++ 3.8-rc6-xen-pciback-MSI-X-prepare/drivers/xen/xen-pciback/pci_stub.c
> @@ -17,6 +17,7 @@
>  #include <xen/events.h>
>  #include <asm/xen/pci.h>
>  #include <asm/xen/hypervisor.h>
> +#include <xen/interface/physdev.h>
>  #include "pciback.h"
>  #include "conf_space.h"
>  #include "conf_space_quirks.h"
> @@ -85,37 +86,52 @@ static struct pcistub_device *pcistub_de
>  static void pcistub_device_release(struct kref *kref)
>  {
>  	struct pcistub_device *psdev;
> +	struct pci_dev *dev;
>  	struct xen_pcibk_dev_data *dev_data;
>  
>  	psdev = container_of(kref, struct pcistub_device, kref);
> -	dev_data = pci_get_drvdata(psdev->dev);
> +	dev = psdev->dev;
> +	dev_data = pci_get_drvdata(dev);
>  
> -	dev_dbg(&psdev->dev->dev, "pcistub_device_release\n");
> +	dev_dbg(&dev->dev, "pcistub_device_release\n");
>  
> -	xen_unregister_device_domain_owner(psdev->dev);
> +	xen_unregister_device_domain_owner(dev);
>  
>  	/* Call the reset function which does not take lock as this
>  	 * is called from "unbind" which takes a device_lock mutex.
>  	 */
> -	__pci_reset_function_locked(psdev->dev);
> -	if (pci_load_and_free_saved_state(psdev->dev,
> -					  &dev_data->pci_saved_state)) {
> -		dev_dbg(&psdev->dev->dev, "Could not reload PCI state\n");
> -	} else
> -		pci_restore_state(psdev->dev);
> +	__pci_reset_function_locked(dev);
> +	if (pci_load_and_free_saved_state(dev, &dev_data->pci_saved_state))
> +		dev_dbg(&dev->dev, "Could not reload PCI state\n");
> +	else
> +		pci_restore_state(dev);
> +
> +	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
> +		struct physdev_pci_device ppdev = {
> +			.seg = pci_domain_nr(dev->bus),
> +			.bus = dev->bus->number,
> +			.devfn = dev->devfn
> +		};
> +		int err = HYPERVISOR_physdev_op(PHYSDEVOP_release_msix,
> +						&ppdev);
> +
> +		if (err)
> +			dev_warn(&dev->dev, "MSI-X release failed (%d)\n",
> +				 err);
> +	}

Perhaps it should be more off:

		if (err) {
			if (err == -ENOSYS)
				dev_info(&dev->dev,"MSI-X release
hypercall not supported.");
			else
				dev_warn(&dev->dev, "MSI-X release failed (%d)\n",
					 err);
				
>  
>  	/* Disable the device */
> -	xen_pcibk_reset_device(psdev->dev);
> +	xen_pcibk_reset_device(dev);
>  
>  	kfree(dev_data);
> -	pci_set_drvdata(psdev->dev, NULL);
> +	pci_set_drvdata(dev, NULL);
>  
>  	/* Clean-up the device */
> -	xen_pcibk_config_free_dyn_fields(psdev->dev);
> -	xen_pcibk_config_free_dev(psdev->dev);
> +	xen_pcibk_config_free_dyn_fields(dev);
> +	xen_pcibk_config_free_dev(dev);
>  
> -	psdev->dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
> -	pci_dev_put(psdev->dev);
> +	dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
> +	pci_dev_put(dev);
>  
>  	kfree(psdev);
>  }
> @@ -355,6 +371,19 @@ static int pcistub_init_device(struct pc
>  	if (err)
>  		goto config_release;
>  
> +	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
> +		struct physdev_pci_device ppdev = {
> +			.seg = pci_domain_nr(dev->bus),
> +			.bus = dev->bus->number,
> +			.devfn = dev->devfn
> +		};
> +
> +		err = HYPERVISOR_physdev_op(PHYSDEVOP_prepare_msix, &ppdev);
> +		if (err)
> +			dev_err(&dev->dev, "MSI-X preparation failed (%d)\n",
> +				err);
> +	}
> +
>  	/* We need the device active to save the state. */
>  	dev_dbg(&dev->dev, "save state of device\n");
>  	pci_save_state(dev);
> --- 3.8-rc6/include/xen/interface/physdev.h
> +++ 3.8-rc6-xen-pciback-MSI-X-prepare/include/xen/interface/physdev.h
> @@ -251,6 +251,12 @@ struct physdev_pci_device_add {
>  
>  #define PHYSDEVOP_pci_device_remove     26
>  #define PHYSDEVOP_restore_msi_ext       27
> +/*
> + * Dom0 should use these two to announce MMIO resources assigned to
> + * MSI-X capable devices won't (prepare) or may (release) change.
> + */
> +#define PHYSDEVOP_prepare_msix          30
> +#define PHYSDEVOP_release_msix          31
>  struct physdev_pci_device {
>      /* IN */
>      uint16_t seg;


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

* Re: [PATCH] xen-pciback: notify hypervisor about devices intended to be assigned to guests
  2013-02-06 16:58 [PATCH] xen-pciback: notify hypervisor about devices intended to be assigned to guests Jan Beulich
  2013-02-06 17:12 ` Konrad Rzeszutek Wilk
@ 2013-02-06 17:12 ` Konrad Rzeszutek Wilk
  1 sibling, 0 replies; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-02-06 17:12 UTC (permalink / raw)
  To: Jan Beulich; +Cc: linux-kernel, xen-devel

On Wed, Feb 06, 2013 at 04:58:01PM +0000, Jan Beulich wrote:
> For MSI-X capable devices the hypervisor wants to write protect the
> MSI-X table and PBA, yet it can't assume that resources have been
> assigned to their final values at device enumeration time. Thus have
> pciback do that notification, as having the device controlled by it is
> a prerequisite to assigning the device to guests anyway.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> ---
>  arch/x86/include/asm/xen/hypercall.h |    4 +-
>  drivers/xen/fallback.c               |    3 +
>  drivers/xen/xen-pciback/pci_stub.c   |   59 ++++++++++++++++++++++++++---------
>  include/xen/interface/physdev.h      |    6 +++
>  4 files changed, 54 insertions(+), 18 deletions(-)
> 
> --- 3.8-rc6/arch/x86/include/asm/xen/hypercall.h
> +++ 3.8-rc6-xen-pciback-MSI-X-prepare/arch/x86/include/asm/xen/hypercall.h
> @@ -382,14 +382,14 @@ HYPERVISOR_console_io(int cmd, int count
>  	return _hypercall3(int, console_io, cmd, count, str);
>  }
>  
> -extern int __must_check HYPERVISOR_physdev_op_compat(int, void *);
> +extern int __must_check xen_physdev_op_compat(int, void *);
>  
>  static inline int
>  HYPERVISOR_physdev_op(int cmd, void *arg)
>  {
>  	int rc = _hypercall2(int, physdev_op, cmd, arg);
>  	if (unlikely(rc == -ENOSYS))
> -		rc = HYPERVISOR_physdev_op_compat(cmd, arg);
> +		rc = xen_physdev_op_compat(cmd, arg);
>  	return rc;
>  }
>  
> --- 3.8-rc6/drivers/xen/fallback.c
> +++ 3.8-rc6-xen-pciback-MSI-X-prepare/drivers/xen/fallback.c
> @@ -44,7 +44,7 @@ int xen_event_channel_op_compat(int cmd,
>  }
>  EXPORT_SYMBOL_GPL(xen_event_channel_op_compat);
>  
> -int HYPERVISOR_physdev_op_compat(int cmd, void *arg)
> +int xen_physdev_op_compat(int cmd, void *arg)
>  {
>  	struct physdev_op op;
>  	int rc;
> @@ -78,3 +78,4 @@ int HYPERVISOR_physdev_op_compat(int cmd
>  
>  	return rc;
>  }
> +EXPORT_SYMBOL_GPL(xen_physdev_op_compat);
> --- 3.8-rc6/drivers/xen/xen-pciback/pci_stub.c
> +++ 3.8-rc6-xen-pciback-MSI-X-prepare/drivers/xen/xen-pciback/pci_stub.c
> @@ -17,6 +17,7 @@
>  #include <xen/events.h>
>  #include <asm/xen/pci.h>
>  #include <asm/xen/hypervisor.h>
> +#include <xen/interface/physdev.h>
>  #include "pciback.h"
>  #include "conf_space.h"
>  #include "conf_space_quirks.h"
> @@ -85,37 +86,52 @@ static struct pcistub_device *pcistub_de
>  static void pcistub_device_release(struct kref *kref)
>  {
>  	struct pcistub_device *psdev;
> +	struct pci_dev *dev;
>  	struct xen_pcibk_dev_data *dev_data;
>  
>  	psdev = container_of(kref, struct pcistub_device, kref);
> -	dev_data = pci_get_drvdata(psdev->dev);
> +	dev = psdev->dev;
> +	dev_data = pci_get_drvdata(dev);
>  
> -	dev_dbg(&psdev->dev->dev, "pcistub_device_release\n");
> +	dev_dbg(&dev->dev, "pcistub_device_release\n");
>  
> -	xen_unregister_device_domain_owner(psdev->dev);
> +	xen_unregister_device_domain_owner(dev);
>  
>  	/* Call the reset function which does not take lock as this
>  	 * is called from "unbind" which takes a device_lock mutex.
>  	 */
> -	__pci_reset_function_locked(psdev->dev);
> -	if (pci_load_and_free_saved_state(psdev->dev,
> -					  &dev_data->pci_saved_state)) {
> -		dev_dbg(&psdev->dev->dev, "Could not reload PCI state\n");
> -	} else
> -		pci_restore_state(psdev->dev);
> +	__pci_reset_function_locked(dev);
> +	if (pci_load_and_free_saved_state(dev, &dev_data->pci_saved_state))
> +		dev_dbg(&dev->dev, "Could not reload PCI state\n");
> +	else
> +		pci_restore_state(dev);
> +
> +	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
> +		struct physdev_pci_device ppdev = {
> +			.seg = pci_domain_nr(dev->bus),
> +			.bus = dev->bus->number,
> +			.devfn = dev->devfn
> +		};
> +		int err = HYPERVISOR_physdev_op(PHYSDEVOP_release_msix,
> +						&ppdev);
> +
> +		if (err)
> +			dev_warn(&dev->dev, "MSI-X release failed (%d)\n",
> +				 err);
> +	}
>  
>  	/* Disable the device */
> -	xen_pcibk_reset_device(psdev->dev);
> +	xen_pcibk_reset_device(dev);
>  
>  	kfree(dev_data);
> -	pci_set_drvdata(psdev->dev, NULL);
> +	pci_set_drvdata(dev, NULL);
>  
>  	/* Clean-up the device */
> -	xen_pcibk_config_free_dyn_fields(psdev->dev);
> -	xen_pcibk_config_free_dev(psdev->dev);
> +	xen_pcibk_config_free_dyn_fields(dev);
> +	xen_pcibk_config_free_dev(dev);
>  
> -	psdev->dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
> -	pci_dev_put(psdev->dev);
> +	dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
> +	pci_dev_put(dev);
>  
>  	kfree(psdev);
>  }
> @@ -355,6 +371,19 @@ static int pcistub_init_device(struct pc
>  	if (err)
>  		goto config_release;
>  
> +	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
> +		struct physdev_pci_device ppdev = {
> +			.seg = pci_domain_nr(dev->bus),
> +			.bus = dev->bus->number,
> +			.devfn = dev->devfn
> +		};
> +
> +		err = HYPERVISOR_physdev_op(PHYSDEVOP_prepare_msix, &ppdev);
> +		if (err)
> +			dev_err(&dev->dev, "MSI-X preparation failed (%d)\n",
> +				err);
> +	}
> +
>  	/* We need the device active to save the state. */
>  	dev_dbg(&dev->dev, "save state of device\n");
>  	pci_save_state(dev);
> --- 3.8-rc6/include/xen/interface/physdev.h
> +++ 3.8-rc6-xen-pciback-MSI-X-prepare/include/xen/interface/physdev.h
> @@ -251,6 +251,12 @@ struct physdev_pci_device_add {
>  
>  #define PHYSDEVOP_pci_device_remove     26
>  #define PHYSDEVOP_restore_msi_ext       27
> +/*
> + * Dom0 should use these two to announce MMIO resources assigned to
> + * MSI-X capable devices won't (prepare) or may (release) change.
> + */
> +#define PHYSDEVOP_prepare_msix          30
> +#define PHYSDEVOP_release_msix          31
>  struct physdev_pci_device {
>      /* IN */
>      uint16_t seg;
> 
> 

> xen-pciback: notify hypervisor about devices intended to be assigned to guests
> 
> For MSI-X capable devices the hypervisor wants to write protect the
> MSI-X table and PBA, yet it can't assume that resources have been
> assigned to their final values at device enumeration time. Thus have
> pciback do that notification, as having the device controlled by it is
> a prerequisite to assigning the device to guests anyway.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> ---
>  arch/x86/include/asm/xen/hypercall.h |    4 +-
>  drivers/xen/fallback.c               |    3 +
>  drivers/xen/xen-pciback/pci_stub.c   |   59 ++++++++++++++++++++++++++---------
>  include/xen/interface/physdev.h      |    6 +++
>  4 files changed, 54 insertions(+), 18 deletions(-)
> 
> --- 3.8-rc6/arch/x86/include/asm/xen/hypercall.h
> +++ 3.8-rc6-xen-pciback-MSI-X-prepare/arch/x86/include/asm/xen/hypercall.h
> @@ -382,14 +382,14 @@ HYPERVISOR_console_io(int cmd, int count
>  	return _hypercall3(int, console_io, cmd, count, str);
>  }
>  
> -extern int __must_check HYPERVISOR_physdev_op_compat(int, void *);
> +extern int __must_check xen_physdev_op_compat(int, void *);
>  
>  static inline int
>  HYPERVISOR_physdev_op(int cmd, void *arg)
>  {
>  	int rc = _hypercall2(int, physdev_op, cmd, arg);
>  	if (unlikely(rc == -ENOSYS))
> -		rc = HYPERVISOR_physdev_op_compat(cmd, arg);
> +		rc = xen_physdev_op_compat(cmd, arg);
>  	return rc;
>  }
>  
> --- 3.8-rc6/drivers/xen/fallback.c
> +++ 3.8-rc6-xen-pciback-MSI-X-prepare/drivers/xen/fallback.c
> @@ -44,7 +44,7 @@ int xen_event_channel_op_compat(int cmd,
>  }
>  EXPORT_SYMBOL_GPL(xen_event_channel_op_compat);
>  
> -int HYPERVISOR_physdev_op_compat(int cmd, void *arg)
> +int xen_physdev_op_compat(int cmd, void *arg)
>  {
>  	struct physdev_op op;
>  	int rc;
> @@ -78,3 +78,4 @@ int HYPERVISOR_physdev_op_compat(int cmd
>  
>  	return rc;
>  }
> +EXPORT_SYMBOL_GPL(xen_physdev_op_compat);
> --- 3.8-rc6/drivers/xen/xen-pciback/pci_stub.c
> +++ 3.8-rc6-xen-pciback-MSI-X-prepare/drivers/xen/xen-pciback/pci_stub.c
> @@ -17,6 +17,7 @@
>  #include <xen/events.h>
>  #include <asm/xen/pci.h>
>  #include <asm/xen/hypervisor.h>
> +#include <xen/interface/physdev.h>
>  #include "pciback.h"
>  #include "conf_space.h"
>  #include "conf_space_quirks.h"
> @@ -85,37 +86,52 @@ static struct pcistub_device *pcistub_de
>  static void pcistub_device_release(struct kref *kref)
>  {
>  	struct pcistub_device *psdev;
> +	struct pci_dev *dev;
>  	struct xen_pcibk_dev_data *dev_data;
>  
>  	psdev = container_of(kref, struct pcistub_device, kref);
> -	dev_data = pci_get_drvdata(psdev->dev);
> +	dev = psdev->dev;
> +	dev_data = pci_get_drvdata(dev);
>  
> -	dev_dbg(&psdev->dev->dev, "pcistub_device_release\n");
> +	dev_dbg(&dev->dev, "pcistub_device_release\n");
>  
> -	xen_unregister_device_domain_owner(psdev->dev);
> +	xen_unregister_device_domain_owner(dev);
>  
>  	/* Call the reset function which does not take lock as this
>  	 * is called from "unbind" which takes a device_lock mutex.
>  	 */
> -	__pci_reset_function_locked(psdev->dev);
> -	if (pci_load_and_free_saved_state(psdev->dev,
> -					  &dev_data->pci_saved_state)) {
> -		dev_dbg(&psdev->dev->dev, "Could not reload PCI state\n");
> -	} else
> -		pci_restore_state(psdev->dev);
> +	__pci_reset_function_locked(dev);
> +	if (pci_load_and_free_saved_state(dev, &dev_data->pci_saved_state))
> +		dev_dbg(&dev->dev, "Could not reload PCI state\n");
> +	else
> +		pci_restore_state(dev);
> +
> +	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
> +		struct physdev_pci_device ppdev = {
> +			.seg = pci_domain_nr(dev->bus),
> +			.bus = dev->bus->number,
> +			.devfn = dev->devfn
> +		};
> +		int err = HYPERVISOR_physdev_op(PHYSDEVOP_release_msix,
> +						&ppdev);
> +
> +		if (err)
> +			dev_warn(&dev->dev, "MSI-X release failed (%d)\n",
> +				 err);
> +	}

Perhaps it should be more off:

		if (err) {
			if (err == -ENOSYS)
				dev_info(&dev->dev,"MSI-X release
hypercall not supported.");
			else
				dev_warn(&dev->dev, "MSI-X release failed (%d)\n",
					 err);
				
>  
>  	/* Disable the device */
> -	xen_pcibk_reset_device(psdev->dev);
> +	xen_pcibk_reset_device(dev);
>  
>  	kfree(dev_data);
> -	pci_set_drvdata(psdev->dev, NULL);
> +	pci_set_drvdata(dev, NULL);
>  
>  	/* Clean-up the device */
> -	xen_pcibk_config_free_dyn_fields(psdev->dev);
> -	xen_pcibk_config_free_dev(psdev->dev);
> +	xen_pcibk_config_free_dyn_fields(dev);
> +	xen_pcibk_config_free_dev(dev);
>  
> -	psdev->dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
> -	pci_dev_put(psdev->dev);
> +	dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
> +	pci_dev_put(dev);
>  
>  	kfree(psdev);
>  }
> @@ -355,6 +371,19 @@ static int pcistub_init_device(struct pc
>  	if (err)
>  		goto config_release;
>  
> +	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
> +		struct physdev_pci_device ppdev = {
> +			.seg = pci_domain_nr(dev->bus),
> +			.bus = dev->bus->number,
> +			.devfn = dev->devfn
> +		};
> +
> +		err = HYPERVISOR_physdev_op(PHYSDEVOP_prepare_msix, &ppdev);
> +		if (err)
> +			dev_err(&dev->dev, "MSI-X preparation failed (%d)\n",
> +				err);
> +	}
> +
>  	/* We need the device active to save the state. */
>  	dev_dbg(&dev->dev, "save state of device\n");
>  	pci_save_state(dev);
> --- 3.8-rc6/include/xen/interface/physdev.h
> +++ 3.8-rc6-xen-pciback-MSI-X-prepare/include/xen/interface/physdev.h
> @@ -251,6 +251,12 @@ struct physdev_pci_device_add {
>  
>  #define PHYSDEVOP_pci_device_remove     26
>  #define PHYSDEVOP_restore_msi_ext       27
> +/*
> + * Dom0 should use these two to announce MMIO resources assigned to
> + * MSI-X capable devices won't (prepare) or may (release) change.
> + */
> +#define PHYSDEVOP_prepare_msix          30
> +#define PHYSDEVOP_release_msix          31
>  struct physdev_pci_device {
>      /* IN */
>      uint16_t seg;

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

* Re: [PATCH] xen-pciback: notify hypervisor about devices intended to be assigned to guests
  2013-02-06 17:12 ` Konrad Rzeszutek Wilk
@ 2013-02-07  9:00   ` Jan Beulich
  2013-02-07  9:00   ` Jan Beulich
  1 sibling, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2013-02-07  9:00 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: xen-devel, linux-kernel

>>> On 06.02.13 at 18:12, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
>> +	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
>> +		struct physdev_pci_device ppdev = {
>> +			.seg = pci_domain_nr(dev->bus),
>> +			.bus = dev->bus->number,
>> +			.devfn = dev->devfn
>> +		};
>> +		int err = HYPERVISOR_physdev_op(PHYSDEVOP_release_msix,
>> +						&ppdev);
>> +
>> +		if (err)
>> +			dev_warn(&dev->dev, "MSI-X release failed (%d)\n",
>> +				 err);
>> +	}
> 
> Perhaps it should be more off:
> 
> 		if (err) {
> 			if (err == -ENOSYS)
> 				dev_info(&dev->dev,"MSI-X release
> hypercall not supported.");
> 			else
> 				dev_warn(&dev->dev, "MSI-X release failed (%d)\n",
> 					 err);
> 				

Why would you want to special case this? The more that _really_
old hypervisors returned -EINVAL instead of -ENOSYS here?

Jan


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

* Re: [PATCH] xen-pciback: notify hypervisor about devices intended to be assigned to guests
  2013-02-06 17:12 ` Konrad Rzeszutek Wilk
  2013-02-07  9:00   ` Jan Beulich
@ 2013-02-07  9:00   ` Jan Beulich
  1 sibling, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2013-02-07  9:00 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: linux-kernel, xen-devel

>>> On 06.02.13 at 18:12, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
>> +	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
>> +		struct physdev_pci_device ppdev = {
>> +			.seg = pci_domain_nr(dev->bus),
>> +			.bus = dev->bus->number,
>> +			.devfn = dev->devfn
>> +		};
>> +		int err = HYPERVISOR_physdev_op(PHYSDEVOP_release_msix,
>> +						&ppdev);
>> +
>> +		if (err)
>> +			dev_warn(&dev->dev, "MSI-X release failed (%d)\n",
>> +				 err);
>> +	}
> 
> Perhaps it should be more off:
> 
> 		if (err) {
> 			if (err == -ENOSYS)
> 				dev_info(&dev->dev,"MSI-X release
> hypercall not supported.");
> 			else
> 				dev_warn(&dev->dev, "MSI-X release failed (%d)\n",
> 					 err);
> 				

Why would you want to special case this? The more that _really_
old hypervisors returned -EINVAL instead of -ENOSYS here?

Jan

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

* Re: [PATCH] xen-pciback: notify hypervisor about devices intended to be assigned to guests
  2013-03-12 16:59 ` [Xen-devel] " Jan Beulich
@ 2013-03-12 20:06   ` Konrad Rzeszutek Wilk
  0 siblings, 0 replies; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-03-12 20:06 UTC (permalink / raw)
  To: Jan Beulich; +Cc: linux-kernel, xen-devel

On Tue, Mar 12, 2013 at 04:59:18PM +0000, Jan Beulich wrote:
> >>> On 12.03.13 at 16:06, "Jan Beulich" <JBeulich@suse.com> wrote:
> > For MSI-X capable devices the hypervisor wants to write protect the
> > MSI-X table and PBA, yet it can't assume that resources have been
> > assigned to their final values at device enumeration time. Thus have
> > pciback do that notification, as having the device controlled by it is
> > a prerequisite to assigning the device to guests anyway.
> > 
> > This is the kernel part of hypervisor side commit 4245d33 ("x86/MSI:
> > add mechanism to fully protect MSI-X table from PV guest accesses") on
> > the master branch of git://xenbits.xen.org/xen.git.
> > 
> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Just noticed that once again I forgot to Cc stable@ - could you
> please add this as you commit it to your tree?
> 

Naturally.
> I'm sorry about this, Jan

That is OK. I keep on doing it myself as well.

> 
> > ---
> >  arch/x86/include/asm/xen/hypercall.h |    4 +-
> >  drivers/xen/fallback.c               |    3 +
> >  drivers/xen/xen-pciback/pci_stub.c   |   59 ++++++++++++++++++++++++++---------
> >  include/xen/interface/physdev.h      |    6 +++
> >  4 files changed, 54 insertions(+), 18 deletions(-)
> > 
> > --- 3.9-rc2/arch/x86/include/asm/xen/hypercall.h
> > +++ 3.9-rc2-xen-pciback-MSI-X-prepare/arch/x86/include/asm/xen/hypercall.h
> > @@ -382,14 +382,14 @@ HYPERVISOR_console_io(int cmd, int count
> >  	return _hypercall3(int, console_io, cmd, count, str);
> >  }
> >  
> > -extern int __must_check HYPERVISOR_physdev_op_compat(int, void *);
> > +extern int __must_check xen_physdev_op_compat(int, void *);
> >  
> >  static inline int
> >  HYPERVISOR_physdev_op(int cmd, void *arg)
> >  {
> >  	int rc = _hypercall2(int, physdev_op, cmd, arg);
> >  	if (unlikely(rc == -ENOSYS))
> > -		rc = HYPERVISOR_physdev_op_compat(cmd, arg);
> > +		rc = xen_physdev_op_compat(cmd, arg);
> >  	return rc;
> >  }
> >  
> > --- 3.9-rc2/drivers/xen/fallback.c
> > +++ 3.9-rc2-xen-pciback-MSI-X-prepare/drivers/xen/fallback.c
> > @@ -44,7 +44,7 @@ int xen_event_channel_op_compat(int cmd,
> >  }
> >  EXPORT_SYMBOL_GPL(xen_event_channel_op_compat);
> >  
> > -int HYPERVISOR_physdev_op_compat(int cmd, void *arg)
> > +int xen_physdev_op_compat(int cmd, void *arg)
> >  {
> >  	struct physdev_op op;
> >  	int rc;
> > @@ -78,3 +78,4 @@ int HYPERVISOR_physdev_op_compat(int cmd
> >  
> >  	return rc;
> >  }
> > +EXPORT_SYMBOL_GPL(xen_physdev_op_compat);
> > --- 3.9-rc2/drivers/xen/xen-pciback/pci_stub.c
> > +++ 3.9-rc2-xen-pciback-MSI-X-prepare/drivers/xen/xen-pciback/pci_stub.c
> > @@ -17,6 +17,7 @@
> >  #include <xen/events.h>
> >  #include <asm/xen/pci.h>
> >  #include <asm/xen/hypervisor.h>
> > +#include <xen/interface/physdev.h>
> >  #include "pciback.h"
> >  #include "conf_space.h"
> >  #include "conf_space_quirks.h"
> > @@ -85,37 +86,52 @@ static struct pcistub_device *pcistub_de
> >  static void pcistub_device_release(struct kref *kref)
> >  {
> >  	struct pcistub_device *psdev;
> > +	struct pci_dev *dev;
> >  	struct xen_pcibk_dev_data *dev_data;
> >  
> >  	psdev = container_of(kref, struct pcistub_device, kref);
> > -	dev_data = pci_get_drvdata(psdev->dev);
> > +	dev = psdev->dev;
> > +	dev_data = pci_get_drvdata(dev);
> >  
> > -	dev_dbg(&psdev->dev->dev, "pcistub_device_release\n");
> > +	dev_dbg(&dev->dev, "pcistub_device_release\n");
> >  
> > -	xen_unregister_device_domain_owner(psdev->dev);
> > +	xen_unregister_device_domain_owner(dev);
> >  
> >  	/* Call the reset function which does not take lock as this
> >  	 * is called from "unbind" which takes a device_lock mutex.
> >  	 */
> > -	__pci_reset_function_locked(psdev->dev);
> > -	if (pci_load_and_free_saved_state(psdev->dev,
> > -					  &dev_data->pci_saved_state)) {
> > -		dev_dbg(&psdev->dev->dev, "Could not reload PCI state\n");
> > -	} else
> > -		pci_restore_state(psdev->dev);
> > +	__pci_reset_function_locked(dev);
> > +	if (pci_load_and_free_saved_state(dev, &dev_data->pci_saved_state))
> > +		dev_dbg(&dev->dev, "Could not reload PCI state\n");
> > +	else
> > +		pci_restore_state(dev);
> > +
> > +	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
> > +		struct physdev_pci_device ppdev = {
> > +			.seg = pci_domain_nr(dev->bus),
> > +			.bus = dev->bus->number,
> > +			.devfn = dev->devfn
> > +		};
> > +		int err = HYPERVISOR_physdev_op(PHYSDEVOP_release_msix,
> > +						&ppdev);
> > +
> > +		if (err)
> > +			dev_warn(&dev->dev, "MSI-X release failed (%d)\n",
> > +				 err);
> > +	}
> >  
> >  	/* Disable the device */
> > -	xen_pcibk_reset_device(psdev->dev);
> > +	xen_pcibk_reset_device(dev);
> >  
> >  	kfree(dev_data);
> > -	pci_set_drvdata(psdev->dev, NULL);
> > +	pci_set_drvdata(dev, NULL);
> >  
> >  	/* Clean-up the device */
> > -	xen_pcibk_config_free_dyn_fields(psdev->dev);
> > -	xen_pcibk_config_free_dev(psdev->dev);
> > +	xen_pcibk_config_free_dyn_fields(dev);
> > +	xen_pcibk_config_free_dev(dev);
> >  
> > -	psdev->dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
> > -	pci_dev_put(psdev->dev);
> > +	dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
> > +	pci_dev_put(dev);
> >  
> >  	kfree(psdev);
> >  }
> > @@ -355,6 +371,19 @@ static int pcistub_init_device(struct pc
> >  	if (err)
> >  		goto config_release;
> >  
> > +	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
> > +		struct physdev_pci_device ppdev = {
> > +			.seg = pci_domain_nr(dev->bus),
> > +			.bus = dev->bus->number,
> > +			.devfn = dev->devfn
> > +		};
> > +
> > +		err = HYPERVISOR_physdev_op(PHYSDEVOP_prepare_msix, &ppdev);
> > +		if (err)
> > +			dev_err(&dev->dev, "MSI-X preparation failed (%d)\n",
> > +				err);
> > +	}
> > +
> >  	/* We need the device active to save the state. */
> >  	dev_dbg(&dev->dev, "save state of device\n");
> >  	pci_save_state(dev);
> > --- 3.9-rc2/include/xen/interface/physdev.h
> > +++ 3.9-rc2-xen-pciback-MSI-X-prepare/include/xen/interface/physdev.h
> > @@ -251,6 +251,12 @@ struct physdev_pci_device_add {
> >  
> >  #define PHYSDEVOP_pci_device_remove     26
> >  #define PHYSDEVOP_restore_msi_ext       27
> > +/*
> > + * Dom0 should use these two to announce MMIO resources assigned to
> > + * MSI-X capable devices won't (prepare) or may (release) change.
> > + */
> > +#define PHYSDEVOP_prepare_msix          30
> > +#define PHYSDEVOP_release_msix          31
> >  struct physdev_pci_device {
> >      /* IN */
> >      uint16_t seg;
> 
> 

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

* Re: [PATCH] xen-pciback: notify hypervisor about devices intended to be assigned to guests
  2013-03-12 15:06 Jan Beulich
@ 2013-03-12 16:59 ` Jan Beulich
  2013-03-12 16:59 ` [Xen-devel] " Jan Beulich
  1 sibling, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2013-03-12 16:59 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: linux-kernel, xen-devel

>>> On 12.03.13 at 16:06, "Jan Beulich" <JBeulich@suse.com> wrote:
> For MSI-X capable devices the hypervisor wants to write protect the
> MSI-X table and PBA, yet it can't assume that resources have been
> assigned to their final values at device enumeration time. Thus have
> pciback do that notification, as having the device controlled by it is
> a prerequisite to assigning the device to guests anyway.
> 
> This is the kernel part of hypervisor side commit 4245d33 ("x86/MSI:
> add mechanism to fully protect MSI-X table from PV guest accesses") on
> the master branch of git://xenbits.xen.org/xen.git.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Just noticed that once again I forgot to Cc stable@ - could you
please add this as you commit it to your tree?

I'm sorry about this, Jan

> ---
>  arch/x86/include/asm/xen/hypercall.h |    4 +-
>  drivers/xen/fallback.c               |    3 +
>  drivers/xen/xen-pciback/pci_stub.c   |   59 ++++++++++++++++++++++++++---------
>  include/xen/interface/physdev.h      |    6 +++
>  4 files changed, 54 insertions(+), 18 deletions(-)
> 
> --- 3.9-rc2/arch/x86/include/asm/xen/hypercall.h
> +++ 3.9-rc2-xen-pciback-MSI-X-prepare/arch/x86/include/asm/xen/hypercall.h
> @@ -382,14 +382,14 @@ HYPERVISOR_console_io(int cmd, int count
>  	return _hypercall3(int, console_io, cmd, count, str);
>  }
>  
> -extern int __must_check HYPERVISOR_physdev_op_compat(int, void *);
> +extern int __must_check xen_physdev_op_compat(int, void *);
>  
>  static inline int
>  HYPERVISOR_physdev_op(int cmd, void *arg)
>  {
>  	int rc = _hypercall2(int, physdev_op, cmd, arg);
>  	if (unlikely(rc == -ENOSYS))
> -		rc = HYPERVISOR_physdev_op_compat(cmd, arg);
> +		rc = xen_physdev_op_compat(cmd, arg);
>  	return rc;
>  }
>  
> --- 3.9-rc2/drivers/xen/fallback.c
> +++ 3.9-rc2-xen-pciback-MSI-X-prepare/drivers/xen/fallback.c
> @@ -44,7 +44,7 @@ int xen_event_channel_op_compat(int cmd,
>  }
>  EXPORT_SYMBOL_GPL(xen_event_channel_op_compat);
>  
> -int HYPERVISOR_physdev_op_compat(int cmd, void *arg)
> +int xen_physdev_op_compat(int cmd, void *arg)
>  {
>  	struct physdev_op op;
>  	int rc;
> @@ -78,3 +78,4 @@ int HYPERVISOR_physdev_op_compat(int cmd
>  
>  	return rc;
>  }
> +EXPORT_SYMBOL_GPL(xen_physdev_op_compat);
> --- 3.9-rc2/drivers/xen/xen-pciback/pci_stub.c
> +++ 3.9-rc2-xen-pciback-MSI-X-prepare/drivers/xen/xen-pciback/pci_stub.c
> @@ -17,6 +17,7 @@
>  #include <xen/events.h>
>  #include <asm/xen/pci.h>
>  #include <asm/xen/hypervisor.h>
> +#include <xen/interface/physdev.h>
>  #include "pciback.h"
>  #include "conf_space.h"
>  #include "conf_space_quirks.h"
> @@ -85,37 +86,52 @@ static struct pcistub_device *pcistub_de
>  static void pcistub_device_release(struct kref *kref)
>  {
>  	struct pcistub_device *psdev;
> +	struct pci_dev *dev;
>  	struct xen_pcibk_dev_data *dev_data;
>  
>  	psdev = container_of(kref, struct pcistub_device, kref);
> -	dev_data = pci_get_drvdata(psdev->dev);
> +	dev = psdev->dev;
> +	dev_data = pci_get_drvdata(dev);
>  
> -	dev_dbg(&psdev->dev->dev, "pcistub_device_release\n");
> +	dev_dbg(&dev->dev, "pcistub_device_release\n");
>  
> -	xen_unregister_device_domain_owner(psdev->dev);
> +	xen_unregister_device_domain_owner(dev);
>  
>  	/* Call the reset function which does not take lock as this
>  	 * is called from "unbind" which takes a device_lock mutex.
>  	 */
> -	__pci_reset_function_locked(psdev->dev);
> -	if (pci_load_and_free_saved_state(psdev->dev,
> -					  &dev_data->pci_saved_state)) {
> -		dev_dbg(&psdev->dev->dev, "Could not reload PCI state\n");
> -	} else
> -		pci_restore_state(psdev->dev);
> +	__pci_reset_function_locked(dev);
> +	if (pci_load_and_free_saved_state(dev, &dev_data->pci_saved_state))
> +		dev_dbg(&dev->dev, "Could not reload PCI state\n");
> +	else
> +		pci_restore_state(dev);
> +
> +	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
> +		struct physdev_pci_device ppdev = {
> +			.seg = pci_domain_nr(dev->bus),
> +			.bus = dev->bus->number,
> +			.devfn = dev->devfn
> +		};
> +		int err = HYPERVISOR_physdev_op(PHYSDEVOP_release_msix,
> +						&ppdev);
> +
> +		if (err)
> +			dev_warn(&dev->dev, "MSI-X release failed (%d)\n",
> +				 err);
> +	}
>  
>  	/* Disable the device */
> -	xen_pcibk_reset_device(psdev->dev);
> +	xen_pcibk_reset_device(dev);
>  
>  	kfree(dev_data);
> -	pci_set_drvdata(psdev->dev, NULL);
> +	pci_set_drvdata(dev, NULL);
>  
>  	/* Clean-up the device */
> -	xen_pcibk_config_free_dyn_fields(psdev->dev);
> -	xen_pcibk_config_free_dev(psdev->dev);
> +	xen_pcibk_config_free_dyn_fields(dev);
> +	xen_pcibk_config_free_dev(dev);
>  
> -	psdev->dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
> -	pci_dev_put(psdev->dev);
> +	dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
> +	pci_dev_put(dev);
>  
>  	kfree(psdev);
>  }
> @@ -355,6 +371,19 @@ static int pcistub_init_device(struct pc
>  	if (err)
>  		goto config_release;
>  
> +	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
> +		struct physdev_pci_device ppdev = {
> +			.seg = pci_domain_nr(dev->bus),
> +			.bus = dev->bus->number,
> +			.devfn = dev->devfn
> +		};
> +
> +		err = HYPERVISOR_physdev_op(PHYSDEVOP_prepare_msix, &ppdev);
> +		if (err)
> +			dev_err(&dev->dev, "MSI-X preparation failed (%d)\n",
> +				err);
> +	}
> +
>  	/* We need the device active to save the state. */
>  	dev_dbg(&dev->dev, "save state of device\n");
>  	pci_save_state(dev);
> --- 3.9-rc2/include/xen/interface/physdev.h
> +++ 3.9-rc2-xen-pciback-MSI-X-prepare/include/xen/interface/physdev.h
> @@ -251,6 +251,12 @@ struct physdev_pci_device_add {
>  
>  #define PHYSDEVOP_pci_device_remove     26
>  #define PHYSDEVOP_restore_msi_ext       27
> +/*
> + * Dom0 should use these two to announce MMIO resources assigned to
> + * MSI-X capable devices won't (prepare) or may (release) change.
> + */
> +#define PHYSDEVOP_prepare_msix          30
> +#define PHYSDEVOP_release_msix          31
>  struct physdev_pci_device {
>      /* IN */
>      uint16_t seg;

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

* [PATCH] xen-pciback: notify hypervisor about devices intended to be assigned to guests
@ 2013-03-12 15:06 Jan Beulich
  2013-03-12 16:59 ` Jan Beulich
  2013-03-12 16:59 ` [Xen-devel] " Jan Beulich
  0 siblings, 2 replies; 10+ messages in thread
From: Jan Beulich @ 2013-03-12 15:06 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: xen-devel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 5607 bytes --]

For MSI-X capable devices the hypervisor wants to write protect the
MSI-X table and PBA, yet it can't assume that resources have been
assigned to their final values at device enumeration time. Thus have
pciback do that notification, as having the device controlled by it is
a prerequisite to assigning the device to guests anyway.

This is the kernel part of hypervisor side commit 4245d33 ("x86/MSI:
add mechanism to fully protect MSI-X table from PV guest accesses") on
the master branch of git://xenbits.xen.org/xen.git.

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

---
 arch/x86/include/asm/xen/hypercall.h |    4 +-
 drivers/xen/fallback.c               |    3 +
 drivers/xen/xen-pciback/pci_stub.c   |   59 ++++++++++++++++++++++++++---------
 include/xen/interface/physdev.h      |    6 +++
 4 files changed, 54 insertions(+), 18 deletions(-)

--- 3.9-rc2/arch/x86/include/asm/xen/hypercall.h
+++ 3.9-rc2-xen-pciback-MSI-X-prepare/arch/x86/include/asm/xen/hypercall.h
@@ -382,14 +382,14 @@ HYPERVISOR_console_io(int cmd, int count
 	return _hypercall3(int, console_io, cmd, count, str);
 }
 
-extern int __must_check HYPERVISOR_physdev_op_compat(int, void *);
+extern int __must_check xen_physdev_op_compat(int, void *);
 
 static inline int
 HYPERVISOR_physdev_op(int cmd, void *arg)
 {
 	int rc = _hypercall2(int, physdev_op, cmd, arg);
 	if (unlikely(rc == -ENOSYS))
-		rc = HYPERVISOR_physdev_op_compat(cmd, arg);
+		rc = xen_physdev_op_compat(cmd, arg);
 	return rc;
 }
 
--- 3.9-rc2/drivers/xen/fallback.c
+++ 3.9-rc2-xen-pciback-MSI-X-prepare/drivers/xen/fallback.c
@@ -44,7 +44,7 @@ int xen_event_channel_op_compat(int cmd,
 }
 EXPORT_SYMBOL_GPL(xen_event_channel_op_compat);
 
-int HYPERVISOR_physdev_op_compat(int cmd, void *arg)
+int xen_physdev_op_compat(int cmd, void *arg)
 {
 	struct physdev_op op;
 	int rc;
@@ -78,3 +78,4 @@ int HYPERVISOR_physdev_op_compat(int cmd
 
 	return rc;
 }
+EXPORT_SYMBOL_GPL(xen_physdev_op_compat);
--- 3.9-rc2/drivers/xen/xen-pciback/pci_stub.c
+++ 3.9-rc2-xen-pciback-MSI-X-prepare/drivers/xen/xen-pciback/pci_stub.c
@@ -17,6 +17,7 @@
 #include <xen/events.h>
 #include <asm/xen/pci.h>
 #include <asm/xen/hypervisor.h>
+#include <xen/interface/physdev.h>
 #include "pciback.h"
 #include "conf_space.h"
 #include "conf_space_quirks.h"
@@ -85,37 +86,52 @@ static struct pcistub_device *pcistub_de
 static void pcistub_device_release(struct kref *kref)
 {
 	struct pcistub_device *psdev;
+	struct pci_dev *dev;
 	struct xen_pcibk_dev_data *dev_data;
 
 	psdev = container_of(kref, struct pcistub_device, kref);
-	dev_data = pci_get_drvdata(psdev->dev);
+	dev = psdev->dev;
+	dev_data = pci_get_drvdata(dev);
 
-	dev_dbg(&psdev->dev->dev, "pcistub_device_release\n");
+	dev_dbg(&dev->dev, "pcistub_device_release\n");
 
-	xen_unregister_device_domain_owner(psdev->dev);
+	xen_unregister_device_domain_owner(dev);
 
 	/* Call the reset function which does not take lock as this
 	 * is called from "unbind" which takes a device_lock mutex.
 	 */
-	__pci_reset_function_locked(psdev->dev);
-	if (pci_load_and_free_saved_state(psdev->dev,
-					  &dev_data->pci_saved_state)) {
-		dev_dbg(&psdev->dev->dev, "Could not reload PCI state\n");
-	} else
-		pci_restore_state(psdev->dev);
+	__pci_reset_function_locked(dev);
+	if (pci_load_and_free_saved_state(dev, &dev_data->pci_saved_state))
+		dev_dbg(&dev->dev, "Could not reload PCI state\n");
+	else
+		pci_restore_state(dev);
+
+	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
+		struct physdev_pci_device ppdev = {
+			.seg = pci_domain_nr(dev->bus),
+			.bus = dev->bus->number,
+			.devfn = dev->devfn
+		};
+		int err = HYPERVISOR_physdev_op(PHYSDEVOP_release_msix,
+						&ppdev);
+
+		if (err)
+			dev_warn(&dev->dev, "MSI-X release failed (%d)\n",
+				 err);
+	}
 
 	/* Disable the device */
-	xen_pcibk_reset_device(psdev->dev);
+	xen_pcibk_reset_device(dev);
 
 	kfree(dev_data);
-	pci_set_drvdata(psdev->dev, NULL);
+	pci_set_drvdata(dev, NULL);
 
 	/* Clean-up the device */
-	xen_pcibk_config_free_dyn_fields(psdev->dev);
-	xen_pcibk_config_free_dev(psdev->dev);
+	xen_pcibk_config_free_dyn_fields(dev);
+	xen_pcibk_config_free_dev(dev);
 
-	psdev->dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
-	pci_dev_put(psdev->dev);
+	dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
+	pci_dev_put(dev);
 
 	kfree(psdev);
 }
@@ -355,6 +371,19 @@ static int pcistub_init_device(struct pc
 	if (err)
 		goto config_release;
 
+	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
+		struct physdev_pci_device ppdev = {
+			.seg = pci_domain_nr(dev->bus),
+			.bus = dev->bus->number,
+			.devfn = dev->devfn
+		};
+
+		err = HYPERVISOR_physdev_op(PHYSDEVOP_prepare_msix, &ppdev);
+		if (err)
+			dev_err(&dev->dev, "MSI-X preparation failed (%d)\n",
+				err);
+	}
+
 	/* We need the device active to save the state. */
 	dev_dbg(&dev->dev, "save state of device\n");
 	pci_save_state(dev);
--- 3.9-rc2/include/xen/interface/physdev.h
+++ 3.9-rc2-xen-pciback-MSI-X-prepare/include/xen/interface/physdev.h
@@ -251,6 +251,12 @@ struct physdev_pci_device_add {
 
 #define PHYSDEVOP_pci_device_remove     26
 #define PHYSDEVOP_restore_msi_ext       27
+/*
+ * Dom0 should use these two to announce MMIO resources assigned to
+ * MSI-X capable devices won't (prepare) or may (release) change.
+ */
+#define PHYSDEVOP_prepare_msix          30
+#define PHYSDEVOP_release_msix          31
 struct physdev_pci_device {
     /* IN */
     uint16_t seg;



[-- Attachment #2: linux-3.9-rc2-xen-pciback-MSI-X-prepare.patch --]
[-- Type: text/plain, Size: 5685 bytes --]

xen-pciback: notify hypervisor about devices intended to be assigned to guests

For MSI-X capable devices the hypervisor wants to write protect the
MSI-X table and PBA, yet it can't assume that resources have been
assigned to their final values at device enumeration time. Thus have
pciback do that notification, as having the device controlled by it is
a prerequisite to assigning the device to guests anyway.

This is the kernel part of hypervisor side commit 4245d33 ("x86/MSI:
add mechanism to fully protect MSI-X table from PV guest accesses") on
the master branch of git://xenbits.xen.org/xen.git.

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

---
 arch/x86/include/asm/xen/hypercall.h |    4 +-
 drivers/xen/fallback.c               |    3 +
 drivers/xen/xen-pciback/pci_stub.c   |   59 ++++++++++++++++++++++++++---------
 include/xen/interface/physdev.h      |    6 +++
 4 files changed, 54 insertions(+), 18 deletions(-)

--- 3.9-rc2/arch/x86/include/asm/xen/hypercall.h
+++ 3.9-rc2-xen-pciback-MSI-X-prepare/arch/x86/include/asm/xen/hypercall.h
@@ -382,14 +382,14 @@ HYPERVISOR_console_io(int cmd, int count
 	return _hypercall3(int, console_io, cmd, count, str);
 }
 
-extern int __must_check HYPERVISOR_physdev_op_compat(int, void *);
+extern int __must_check xen_physdev_op_compat(int, void *);
 
 static inline int
 HYPERVISOR_physdev_op(int cmd, void *arg)
 {
 	int rc = _hypercall2(int, physdev_op, cmd, arg);
 	if (unlikely(rc == -ENOSYS))
-		rc = HYPERVISOR_physdev_op_compat(cmd, arg);
+		rc = xen_physdev_op_compat(cmd, arg);
 	return rc;
 }
 
--- 3.9-rc2/drivers/xen/fallback.c
+++ 3.9-rc2-xen-pciback-MSI-X-prepare/drivers/xen/fallback.c
@@ -44,7 +44,7 @@ int xen_event_channel_op_compat(int cmd,
 }
 EXPORT_SYMBOL_GPL(xen_event_channel_op_compat);
 
-int HYPERVISOR_physdev_op_compat(int cmd, void *arg)
+int xen_physdev_op_compat(int cmd, void *arg)
 {
 	struct physdev_op op;
 	int rc;
@@ -78,3 +78,4 @@ int HYPERVISOR_physdev_op_compat(int cmd
 
 	return rc;
 }
+EXPORT_SYMBOL_GPL(xen_physdev_op_compat);
--- 3.9-rc2/drivers/xen/xen-pciback/pci_stub.c
+++ 3.9-rc2-xen-pciback-MSI-X-prepare/drivers/xen/xen-pciback/pci_stub.c
@@ -17,6 +17,7 @@
 #include <xen/events.h>
 #include <asm/xen/pci.h>
 #include <asm/xen/hypervisor.h>
+#include <xen/interface/physdev.h>
 #include "pciback.h"
 #include "conf_space.h"
 #include "conf_space_quirks.h"
@@ -85,37 +86,52 @@ static struct pcistub_device *pcistub_de
 static void pcistub_device_release(struct kref *kref)
 {
 	struct pcistub_device *psdev;
+	struct pci_dev *dev;
 	struct xen_pcibk_dev_data *dev_data;
 
 	psdev = container_of(kref, struct pcistub_device, kref);
-	dev_data = pci_get_drvdata(psdev->dev);
+	dev = psdev->dev;
+	dev_data = pci_get_drvdata(dev);
 
-	dev_dbg(&psdev->dev->dev, "pcistub_device_release\n");
+	dev_dbg(&dev->dev, "pcistub_device_release\n");
 
-	xen_unregister_device_domain_owner(psdev->dev);
+	xen_unregister_device_domain_owner(dev);
 
 	/* Call the reset function which does not take lock as this
 	 * is called from "unbind" which takes a device_lock mutex.
 	 */
-	__pci_reset_function_locked(psdev->dev);
-	if (pci_load_and_free_saved_state(psdev->dev,
-					  &dev_data->pci_saved_state)) {
-		dev_dbg(&psdev->dev->dev, "Could not reload PCI state\n");
-	} else
-		pci_restore_state(psdev->dev);
+	__pci_reset_function_locked(dev);
+	if (pci_load_and_free_saved_state(dev, &dev_data->pci_saved_state))
+		dev_dbg(&dev->dev, "Could not reload PCI state\n");
+	else
+		pci_restore_state(dev);
+
+	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
+		struct physdev_pci_device ppdev = {
+			.seg = pci_domain_nr(dev->bus),
+			.bus = dev->bus->number,
+			.devfn = dev->devfn
+		};
+		int err = HYPERVISOR_physdev_op(PHYSDEVOP_release_msix,
+						&ppdev);
+
+		if (err)
+			dev_warn(&dev->dev, "MSI-X release failed (%d)\n",
+				 err);
+	}
 
 	/* Disable the device */
-	xen_pcibk_reset_device(psdev->dev);
+	xen_pcibk_reset_device(dev);
 
 	kfree(dev_data);
-	pci_set_drvdata(psdev->dev, NULL);
+	pci_set_drvdata(dev, NULL);
 
 	/* Clean-up the device */
-	xen_pcibk_config_free_dyn_fields(psdev->dev);
-	xen_pcibk_config_free_dev(psdev->dev);
+	xen_pcibk_config_free_dyn_fields(dev);
+	xen_pcibk_config_free_dev(dev);
 
-	psdev->dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
-	pci_dev_put(psdev->dev);
+	dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
+	pci_dev_put(dev);
 
 	kfree(psdev);
 }
@@ -355,6 +371,19 @@ static int pcistub_init_device(struct pc
 	if (err)
 		goto config_release;
 
+	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
+		struct physdev_pci_device ppdev = {
+			.seg = pci_domain_nr(dev->bus),
+			.bus = dev->bus->number,
+			.devfn = dev->devfn
+		};
+
+		err = HYPERVISOR_physdev_op(PHYSDEVOP_prepare_msix, &ppdev);
+		if (err)
+			dev_err(&dev->dev, "MSI-X preparation failed (%d)\n",
+				err);
+	}
+
 	/* We need the device active to save the state. */
 	dev_dbg(&dev->dev, "save state of device\n");
 	pci_save_state(dev);
--- 3.9-rc2/include/xen/interface/physdev.h
+++ 3.9-rc2-xen-pciback-MSI-X-prepare/include/xen/interface/physdev.h
@@ -251,6 +251,12 @@ struct physdev_pci_device_add {
 
 #define PHYSDEVOP_pci_device_remove     26
 #define PHYSDEVOP_restore_msi_ext       27
+/*
+ * Dom0 should use these two to announce MMIO resources assigned to
+ * MSI-X capable devices won't (prepare) or may (release) change.
+ */
+#define PHYSDEVOP_prepare_msix          30
+#define PHYSDEVOP_release_msix          31
 struct physdev_pci_device {
     /* IN */
     uint16_t seg;

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

* [PATCH] xen-pciback: notify hypervisor about devices intended to be assigned to guests
@ 2013-03-12 15:06 Jan Beulich
  0 siblings, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2013-03-12 15:06 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: linux-kernel, xen-devel

[-- Attachment #1: Type: text/plain, Size: 5607 bytes --]

For MSI-X capable devices the hypervisor wants to write protect the
MSI-X table and PBA, yet it can't assume that resources have been
assigned to their final values at device enumeration time. Thus have
pciback do that notification, as having the device controlled by it is
a prerequisite to assigning the device to guests anyway.

This is the kernel part of hypervisor side commit 4245d33 ("x86/MSI:
add mechanism to fully protect MSI-X table from PV guest accesses") on
the master branch of git://xenbits.xen.org/xen.git.

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

---
 arch/x86/include/asm/xen/hypercall.h |    4 +-
 drivers/xen/fallback.c               |    3 +
 drivers/xen/xen-pciback/pci_stub.c   |   59 ++++++++++++++++++++++++++---------
 include/xen/interface/physdev.h      |    6 +++
 4 files changed, 54 insertions(+), 18 deletions(-)

--- 3.9-rc2/arch/x86/include/asm/xen/hypercall.h
+++ 3.9-rc2-xen-pciback-MSI-X-prepare/arch/x86/include/asm/xen/hypercall.h
@@ -382,14 +382,14 @@ HYPERVISOR_console_io(int cmd, int count
 	return _hypercall3(int, console_io, cmd, count, str);
 }
 
-extern int __must_check HYPERVISOR_physdev_op_compat(int, void *);
+extern int __must_check xen_physdev_op_compat(int, void *);
 
 static inline int
 HYPERVISOR_physdev_op(int cmd, void *arg)
 {
 	int rc = _hypercall2(int, physdev_op, cmd, arg);
 	if (unlikely(rc == -ENOSYS))
-		rc = HYPERVISOR_physdev_op_compat(cmd, arg);
+		rc = xen_physdev_op_compat(cmd, arg);
 	return rc;
 }
 
--- 3.9-rc2/drivers/xen/fallback.c
+++ 3.9-rc2-xen-pciback-MSI-X-prepare/drivers/xen/fallback.c
@@ -44,7 +44,7 @@ int xen_event_channel_op_compat(int cmd,
 }
 EXPORT_SYMBOL_GPL(xen_event_channel_op_compat);
 
-int HYPERVISOR_physdev_op_compat(int cmd, void *arg)
+int xen_physdev_op_compat(int cmd, void *arg)
 {
 	struct physdev_op op;
 	int rc;
@@ -78,3 +78,4 @@ int HYPERVISOR_physdev_op_compat(int cmd
 
 	return rc;
 }
+EXPORT_SYMBOL_GPL(xen_physdev_op_compat);
--- 3.9-rc2/drivers/xen/xen-pciback/pci_stub.c
+++ 3.9-rc2-xen-pciback-MSI-X-prepare/drivers/xen/xen-pciback/pci_stub.c
@@ -17,6 +17,7 @@
 #include <xen/events.h>
 #include <asm/xen/pci.h>
 #include <asm/xen/hypervisor.h>
+#include <xen/interface/physdev.h>
 #include "pciback.h"
 #include "conf_space.h"
 #include "conf_space_quirks.h"
@@ -85,37 +86,52 @@ static struct pcistub_device *pcistub_de
 static void pcistub_device_release(struct kref *kref)
 {
 	struct pcistub_device *psdev;
+	struct pci_dev *dev;
 	struct xen_pcibk_dev_data *dev_data;
 
 	psdev = container_of(kref, struct pcistub_device, kref);
-	dev_data = pci_get_drvdata(psdev->dev);
+	dev = psdev->dev;
+	dev_data = pci_get_drvdata(dev);
 
-	dev_dbg(&psdev->dev->dev, "pcistub_device_release\n");
+	dev_dbg(&dev->dev, "pcistub_device_release\n");
 
-	xen_unregister_device_domain_owner(psdev->dev);
+	xen_unregister_device_domain_owner(dev);
 
 	/* Call the reset function which does not take lock as this
 	 * is called from "unbind" which takes a device_lock mutex.
 	 */
-	__pci_reset_function_locked(psdev->dev);
-	if (pci_load_and_free_saved_state(psdev->dev,
-					  &dev_data->pci_saved_state)) {
-		dev_dbg(&psdev->dev->dev, "Could not reload PCI state\n");
-	} else
-		pci_restore_state(psdev->dev);
+	__pci_reset_function_locked(dev);
+	if (pci_load_and_free_saved_state(dev, &dev_data->pci_saved_state))
+		dev_dbg(&dev->dev, "Could not reload PCI state\n");
+	else
+		pci_restore_state(dev);
+
+	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
+		struct physdev_pci_device ppdev = {
+			.seg = pci_domain_nr(dev->bus),
+			.bus = dev->bus->number,
+			.devfn = dev->devfn
+		};
+		int err = HYPERVISOR_physdev_op(PHYSDEVOP_release_msix,
+						&ppdev);
+
+		if (err)
+			dev_warn(&dev->dev, "MSI-X release failed (%d)\n",
+				 err);
+	}
 
 	/* Disable the device */
-	xen_pcibk_reset_device(psdev->dev);
+	xen_pcibk_reset_device(dev);
 
 	kfree(dev_data);
-	pci_set_drvdata(psdev->dev, NULL);
+	pci_set_drvdata(dev, NULL);
 
 	/* Clean-up the device */
-	xen_pcibk_config_free_dyn_fields(psdev->dev);
-	xen_pcibk_config_free_dev(psdev->dev);
+	xen_pcibk_config_free_dyn_fields(dev);
+	xen_pcibk_config_free_dev(dev);
 
-	psdev->dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
-	pci_dev_put(psdev->dev);
+	dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
+	pci_dev_put(dev);
 
 	kfree(psdev);
 }
@@ -355,6 +371,19 @@ static int pcistub_init_device(struct pc
 	if (err)
 		goto config_release;
 
+	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
+		struct physdev_pci_device ppdev = {
+			.seg = pci_domain_nr(dev->bus),
+			.bus = dev->bus->number,
+			.devfn = dev->devfn
+		};
+
+		err = HYPERVISOR_physdev_op(PHYSDEVOP_prepare_msix, &ppdev);
+		if (err)
+			dev_err(&dev->dev, "MSI-X preparation failed (%d)\n",
+				err);
+	}
+
 	/* We need the device active to save the state. */
 	dev_dbg(&dev->dev, "save state of device\n");
 	pci_save_state(dev);
--- 3.9-rc2/include/xen/interface/physdev.h
+++ 3.9-rc2-xen-pciback-MSI-X-prepare/include/xen/interface/physdev.h
@@ -251,6 +251,12 @@ struct physdev_pci_device_add {
 
 #define PHYSDEVOP_pci_device_remove     26
 #define PHYSDEVOP_restore_msi_ext       27
+/*
+ * Dom0 should use these two to announce MMIO resources assigned to
+ * MSI-X capable devices won't (prepare) or may (release) change.
+ */
+#define PHYSDEVOP_prepare_msix          30
+#define PHYSDEVOP_release_msix          31
 struct physdev_pci_device {
     /* IN */
     uint16_t seg;



[-- Attachment #2: linux-3.9-rc2-xen-pciback-MSI-X-prepare.patch --]
[-- Type: text/plain, Size: 5685 bytes --]

xen-pciback: notify hypervisor about devices intended to be assigned to guests

For MSI-X capable devices the hypervisor wants to write protect the
MSI-X table and PBA, yet it can't assume that resources have been
assigned to their final values at device enumeration time. Thus have
pciback do that notification, as having the device controlled by it is
a prerequisite to assigning the device to guests anyway.

This is the kernel part of hypervisor side commit 4245d33 ("x86/MSI:
add mechanism to fully protect MSI-X table from PV guest accesses") on
the master branch of git://xenbits.xen.org/xen.git.

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

---
 arch/x86/include/asm/xen/hypercall.h |    4 +-
 drivers/xen/fallback.c               |    3 +
 drivers/xen/xen-pciback/pci_stub.c   |   59 ++++++++++++++++++++++++++---------
 include/xen/interface/physdev.h      |    6 +++
 4 files changed, 54 insertions(+), 18 deletions(-)

--- 3.9-rc2/arch/x86/include/asm/xen/hypercall.h
+++ 3.9-rc2-xen-pciback-MSI-X-prepare/arch/x86/include/asm/xen/hypercall.h
@@ -382,14 +382,14 @@ HYPERVISOR_console_io(int cmd, int count
 	return _hypercall3(int, console_io, cmd, count, str);
 }
 
-extern int __must_check HYPERVISOR_physdev_op_compat(int, void *);
+extern int __must_check xen_physdev_op_compat(int, void *);
 
 static inline int
 HYPERVISOR_physdev_op(int cmd, void *arg)
 {
 	int rc = _hypercall2(int, physdev_op, cmd, arg);
 	if (unlikely(rc == -ENOSYS))
-		rc = HYPERVISOR_physdev_op_compat(cmd, arg);
+		rc = xen_physdev_op_compat(cmd, arg);
 	return rc;
 }
 
--- 3.9-rc2/drivers/xen/fallback.c
+++ 3.9-rc2-xen-pciback-MSI-X-prepare/drivers/xen/fallback.c
@@ -44,7 +44,7 @@ int xen_event_channel_op_compat(int cmd,
 }
 EXPORT_SYMBOL_GPL(xen_event_channel_op_compat);
 
-int HYPERVISOR_physdev_op_compat(int cmd, void *arg)
+int xen_physdev_op_compat(int cmd, void *arg)
 {
 	struct physdev_op op;
 	int rc;
@@ -78,3 +78,4 @@ int HYPERVISOR_physdev_op_compat(int cmd
 
 	return rc;
 }
+EXPORT_SYMBOL_GPL(xen_physdev_op_compat);
--- 3.9-rc2/drivers/xen/xen-pciback/pci_stub.c
+++ 3.9-rc2-xen-pciback-MSI-X-prepare/drivers/xen/xen-pciback/pci_stub.c
@@ -17,6 +17,7 @@
 #include <xen/events.h>
 #include <asm/xen/pci.h>
 #include <asm/xen/hypervisor.h>
+#include <xen/interface/physdev.h>
 #include "pciback.h"
 #include "conf_space.h"
 #include "conf_space_quirks.h"
@@ -85,37 +86,52 @@ static struct pcistub_device *pcistub_de
 static void pcistub_device_release(struct kref *kref)
 {
 	struct pcistub_device *psdev;
+	struct pci_dev *dev;
 	struct xen_pcibk_dev_data *dev_data;
 
 	psdev = container_of(kref, struct pcistub_device, kref);
-	dev_data = pci_get_drvdata(psdev->dev);
+	dev = psdev->dev;
+	dev_data = pci_get_drvdata(dev);
 
-	dev_dbg(&psdev->dev->dev, "pcistub_device_release\n");
+	dev_dbg(&dev->dev, "pcistub_device_release\n");
 
-	xen_unregister_device_domain_owner(psdev->dev);
+	xen_unregister_device_domain_owner(dev);
 
 	/* Call the reset function which does not take lock as this
 	 * is called from "unbind" which takes a device_lock mutex.
 	 */
-	__pci_reset_function_locked(psdev->dev);
-	if (pci_load_and_free_saved_state(psdev->dev,
-					  &dev_data->pci_saved_state)) {
-		dev_dbg(&psdev->dev->dev, "Could not reload PCI state\n");
-	} else
-		pci_restore_state(psdev->dev);
+	__pci_reset_function_locked(dev);
+	if (pci_load_and_free_saved_state(dev, &dev_data->pci_saved_state))
+		dev_dbg(&dev->dev, "Could not reload PCI state\n");
+	else
+		pci_restore_state(dev);
+
+	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
+		struct physdev_pci_device ppdev = {
+			.seg = pci_domain_nr(dev->bus),
+			.bus = dev->bus->number,
+			.devfn = dev->devfn
+		};
+		int err = HYPERVISOR_physdev_op(PHYSDEVOP_release_msix,
+						&ppdev);
+
+		if (err)
+			dev_warn(&dev->dev, "MSI-X release failed (%d)\n",
+				 err);
+	}
 
 	/* Disable the device */
-	xen_pcibk_reset_device(psdev->dev);
+	xen_pcibk_reset_device(dev);
 
 	kfree(dev_data);
-	pci_set_drvdata(psdev->dev, NULL);
+	pci_set_drvdata(dev, NULL);
 
 	/* Clean-up the device */
-	xen_pcibk_config_free_dyn_fields(psdev->dev);
-	xen_pcibk_config_free_dev(psdev->dev);
+	xen_pcibk_config_free_dyn_fields(dev);
+	xen_pcibk_config_free_dev(dev);
 
-	psdev->dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
-	pci_dev_put(psdev->dev);
+	dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
+	pci_dev_put(dev);
 
 	kfree(psdev);
 }
@@ -355,6 +371,19 @@ static int pcistub_init_device(struct pc
 	if (err)
 		goto config_release;
 
+	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
+		struct physdev_pci_device ppdev = {
+			.seg = pci_domain_nr(dev->bus),
+			.bus = dev->bus->number,
+			.devfn = dev->devfn
+		};
+
+		err = HYPERVISOR_physdev_op(PHYSDEVOP_prepare_msix, &ppdev);
+		if (err)
+			dev_err(&dev->dev, "MSI-X preparation failed (%d)\n",
+				err);
+	}
+
 	/* We need the device active to save the state. */
 	dev_dbg(&dev->dev, "save state of device\n");
 	pci_save_state(dev);
--- 3.9-rc2/include/xen/interface/physdev.h
+++ 3.9-rc2-xen-pciback-MSI-X-prepare/include/xen/interface/physdev.h
@@ -251,6 +251,12 @@ struct physdev_pci_device_add {
 
 #define PHYSDEVOP_pci_device_remove     26
 #define PHYSDEVOP_restore_msi_ext       27
+/*
+ * Dom0 should use these two to announce MMIO resources assigned to
+ * MSI-X capable devices won't (prepare) or may (release) change.
+ */
+#define PHYSDEVOP_prepare_msix          30
+#define PHYSDEVOP_release_msix          31
 struct physdev_pci_device {
     /* IN */
     uint16_t seg;

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

* [PATCH] xen-pciback: notify hypervisor about devices intended to be assigned to guests
@ 2013-02-06 16:58 Jan Beulich
  0 siblings, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2013-02-06 16:58 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: linux-kernel, xen-devel

[-- Attachment #1: Type: text/plain, Size: 5410 bytes --]

For MSI-X capable devices the hypervisor wants to write protect the
MSI-X table and PBA, yet it can't assume that resources have been
assigned to their final values at device enumeration time. Thus have
pciback do that notification, as having the device controlled by it is
a prerequisite to assigning the device to guests anyway.

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

---
 arch/x86/include/asm/xen/hypercall.h |    4 +-
 drivers/xen/fallback.c               |    3 +
 drivers/xen/xen-pciback/pci_stub.c   |   59 ++++++++++++++++++++++++++---------
 include/xen/interface/physdev.h      |    6 +++
 4 files changed, 54 insertions(+), 18 deletions(-)

--- 3.8-rc6/arch/x86/include/asm/xen/hypercall.h
+++ 3.8-rc6-xen-pciback-MSI-X-prepare/arch/x86/include/asm/xen/hypercall.h
@@ -382,14 +382,14 @@ HYPERVISOR_console_io(int cmd, int count
 	return _hypercall3(int, console_io, cmd, count, str);
 }
 
-extern int __must_check HYPERVISOR_physdev_op_compat(int, void *);
+extern int __must_check xen_physdev_op_compat(int, void *);
 
 static inline int
 HYPERVISOR_physdev_op(int cmd, void *arg)
 {
 	int rc = _hypercall2(int, physdev_op, cmd, arg);
 	if (unlikely(rc == -ENOSYS))
-		rc = HYPERVISOR_physdev_op_compat(cmd, arg);
+		rc = xen_physdev_op_compat(cmd, arg);
 	return rc;
 }
 
--- 3.8-rc6/drivers/xen/fallback.c
+++ 3.8-rc6-xen-pciback-MSI-X-prepare/drivers/xen/fallback.c
@@ -44,7 +44,7 @@ int xen_event_channel_op_compat(int cmd,
 }
 EXPORT_SYMBOL_GPL(xen_event_channel_op_compat);
 
-int HYPERVISOR_physdev_op_compat(int cmd, void *arg)
+int xen_physdev_op_compat(int cmd, void *arg)
 {
 	struct physdev_op op;
 	int rc;
@@ -78,3 +78,4 @@ int HYPERVISOR_physdev_op_compat(int cmd
 
 	return rc;
 }
+EXPORT_SYMBOL_GPL(xen_physdev_op_compat);
--- 3.8-rc6/drivers/xen/xen-pciback/pci_stub.c
+++ 3.8-rc6-xen-pciback-MSI-X-prepare/drivers/xen/xen-pciback/pci_stub.c
@@ -17,6 +17,7 @@
 #include <xen/events.h>
 #include <asm/xen/pci.h>
 #include <asm/xen/hypervisor.h>
+#include <xen/interface/physdev.h>
 #include "pciback.h"
 #include "conf_space.h"
 #include "conf_space_quirks.h"
@@ -85,37 +86,52 @@ static struct pcistub_device *pcistub_de
 static void pcistub_device_release(struct kref *kref)
 {
 	struct pcistub_device *psdev;
+	struct pci_dev *dev;
 	struct xen_pcibk_dev_data *dev_data;
 
 	psdev = container_of(kref, struct pcistub_device, kref);
-	dev_data = pci_get_drvdata(psdev->dev);
+	dev = psdev->dev;
+	dev_data = pci_get_drvdata(dev);
 
-	dev_dbg(&psdev->dev->dev, "pcistub_device_release\n");
+	dev_dbg(&dev->dev, "pcistub_device_release\n");
 
-	xen_unregister_device_domain_owner(psdev->dev);
+	xen_unregister_device_domain_owner(dev);
 
 	/* Call the reset function which does not take lock as this
 	 * is called from "unbind" which takes a device_lock mutex.
 	 */
-	__pci_reset_function_locked(psdev->dev);
-	if (pci_load_and_free_saved_state(psdev->dev,
-					  &dev_data->pci_saved_state)) {
-		dev_dbg(&psdev->dev->dev, "Could not reload PCI state\n");
-	} else
-		pci_restore_state(psdev->dev);
+	__pci_reset_function_locked(dev);
+	if (pci_load_and_free_saved_state(dev, &dev_data->pci_saved_state))
+		dev_dbg(&dev->dev, "Could not reload PCI state\n");
+	else
+		pci_restore_state(dev);
+
+	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
+		struct physdev_pci_device ppdev = {
+			.seg = pci_domain_nr(dev->bus),
+			.bus = dev->bus->number,
+			.devfn = dev->devfn
+		};
+		int err = HYPERVISOR_physdev_op(PHYSDEVOP_release_msix,
+						&ppdev);
+
+		if (err)
+			dev_warn(&dev->dev, "MSI-X release failed (%d)\n",
+				 err);
+	}
 
 	/* Disable the device */
-	xen_pcibk_reset_device(psdev->dev);
+	xen_pcibk_reset_device(dev);
 
 	kfree(dev_data);
-	pci_set_drvdata(psdev->dev, NULL);
+	pci_set_drvdata(dev, NULL);
 
 	/* Clean-up the device */
-	xen_pcibk_config_free_dyn_fields(psdev->dev);
-	xen_pcibk_config_free_dev(psdev->dev);
+	xen_pcibk_config_free_dyn_fields(dev);
+	xen_pcibk_config_free_dev(dev);
 
-	psdev->dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
-	pci_dev_put(psdev->dev);
+	dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
+	pci_dev_put(dev);
 
 	kfree(psdev);
 }
@@ -355,6 +371,19 @@ static int pcistub_init_device(struct pc
 	if (err)
 		goto config_release;
 
+	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
+		struct physdev_pci_device ppdev = {
+			.seg = pci_domain_nr(dev->bus),
+			.bus = dev->bus->number,
+			.devfn = dev->devfn
+		};
+
+		err = HYPERVISOR_physdev_op(PHYSDEVOP_prepare_msix, &ppdev);
+		if (err)
+			dev_err(&dev->dev, "MSI-X preparation failed (%d)\n",
+				err);
+	}
+
 	/* We need the device active to save the state. */
 	dev_dbg(&dev->dev, "save state of device\n");
 	pci_save_state(dev);
--- 3.8-rc6/include/xen/interface/physdev.h
+++ 3.8-rc6-xen-pciback-MSI-X-prepare/include/xen/interface/physdev.h
@@ -251,6 +251,12 @@ struct physdev_pci_device_add {
 
 #define PHYSDEVOP_pci_device_remove     26
 #define PHYSDEVOP_restore_msi_ext       27
+/*
+ * Dom0 should use these two to announce MMIO resources assigned to
+ * MSI-X capable devices won't (prepare) or may (release) change.
+ */
+#define PHYSDEVOP_prepare_msix          30
+#define PHYSDEVOP_release_msix          31
 struct physdev_pci_device {
     /* IN */
     uint16_t seg;



[-- Attachment #2: linux-3.8-rc6-xen-pciback-MSI-X-prepare.patch --]
[-- Type: text/plain, Size: 5488 bytes --]

xen-pciback: notify hypervisor about devices intended to be assigned to guests

For MSI-X capable devices the hypervisor wants to write protect the
MSI-X table and PBA, yet it can't assume that resources have been
assigned to their final values at device enumeration time. Thus have
pciback do that notification, as having the device controlled by it is
a prerequisite to assigning the device to guests anyway.

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

---
 arch/x86/include/asm/xen/hypercall.h |    4 +-
 drivers/xen/fallback.c               |    3 +
 drivers/xen/xen-pciback/pci_stub.c   |   59 ++++++++++++++++++++++++++---------
 include/xen/interface/physdev.h      |    6 +++
 4 files changed, 54 insertions(+), 18 deletions(-)

--- 3.8-rc6/arch/x86/include/asm/xen/hypercall.h
+++ 3.8-rc6-xen-pciback-MSI-X-prepare/arch/x86/include/asm/xen/hypercall.h
@@ -382,14 +382,14 @@ HYPERVISOR_console_io(int cmd, int count
 	return _hypercall3(int, console_io, cmd, count, str);
 }
 
-extern int __must_check HYPERVISOR_physdev_op_compat(int, void *);
+extern int __must_check xen_physdev_op_compat(int, void *);
 
 static inline int
 HYPERVISOR_physdev_op(int cmd, void *arg)
 {
 	int rc = _hypercall2(int, physdev_op, cmd, arg);
 	if (unlikely(rc == -ENOSYS))
-		rc = HYPERVISOR_physdev_op_compat(cmd, arg);
+		rc = xen_physdev_op_compat(cmd, arg);
 	return rc;
 }
 
--- 3.8-rc6/drivers/xen/fallback.c
+++ 3.8-rc6-xen-pciback-MSI-X-prepare/drivers/xen/fallback.c
@@ -44,7 +44,7 @@ int xen_event_channel_op_compat(int cmd,
 }
 EXPORT_SYMBOL_GPL(xen_event_channel_op_compat);
 
-int HYPERVISOR_physdev_op_compat(int cmd, void *arg)
+int xen_physdev_op_compat(int cmd, void *arg)
 {
 	struct physdev_op op;
 	int rc;
@@ -78,3 +78,4 @@ int HYPERVISOR_physdev_op_compat(int cmd
 
 	return rc;
 }
+EXPORT_SYMBOL_GPL(xen_physdev_op_compat);
--- 3.8-rc6/drivers/xen/xen-pciback/pci_stub.c
+++ 3.8-rc6-xen-pciback-MSI-X-prepare/drivers/xen/xen-pciback/pci_stub.c
@@ -17,6 +17,7 @@
 #include <xen/events.h>
 #include <asm/xen/pci.h>
 #include <asm/xen/hypervisor.h>
+#include <xen/interface/physdev.h>
 #include "pciback.h"
 #include "conf_space.h"
 #include "conf_space_quirks.h"
@@ -85,37 +86,52 @@ static struct pcistub_device *pcistub_de
 static void pcistub_device_release(struct kref *kref)
 {
 	struct pcistub_device *psdev;
+	struct pci_dev *dev;
 	struct xen_pcibk_dev_data *dev_data;
 
 	psdev = container_of(kref, struct pcistub_device, kref);
-	dev_data = pci_get_drvdata(psdev->dev);
+	dev = psdev->dev;
+	dev_data = pci_get_drvdata(dev);
 
-	dev_dbg(&psdev->dev->dev, "pcistub_device_release\n");
+	dev_dbg(&dev->dev, "pcistub_device_release\n");
 
-	xen_unregister_device_domain_owner(psdev->dev);
+	xen_unregister_device_domain_owner(dev);
 
 	/* Call the reset function which does not take lock as this
 	 * is called from "unbind" which takes a device_lock mutex.
 	 */
-	__pci_reset_function_locked(psdev->dev);
-	if (pci_load_and_free_saved_state(psdev->dev,
-					  &dev_data->pci_saved_state)) {
-		dev_dbg(&psdev->dev->dev, "Could not reload PCI state\n");
-	} else
-		pci_restore_state(psdev->dev);
+	__pci_reset_function_locked(dev);
+	if (pci_load_and_free_saved_state(dev, &dev_data->pci_saved_state))
+		dev_dbg(&dev->dev, "Could not reload PCI state\n");
+	else
+		pci_restore_state(dev);
+
+	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
+		struct physdev_pci_device ppdev = {
+			.seg = pci_domain_nr(dev->bus),
+			.bus = dev->bus->number,
+			.devfn = dev->devfn
+		};
+		int err = HYPERVISOR_physdev_op(PHYSDEVOP_release_msix,
+						&ppdev);
+
+		if (err)
+			dev_warn(&dev->dev, "MSI-X release failed (%d)\n",
+				 err);
+	}
 
 	/* Disable the device */
-	xen_pcibk_reset_device(psdev->dev);
+	xen_pcibk_reset_device(dev);
 
 	kfree(dev_data);
-	pci_set_drvdata(psdev->dev, NULL);
+	pci_set_drvdata(dev, NULL);
 
 	/* Clean-up the device */
-	xen_pcibk_config_free_dyn_fields(psdev->dev);
-	xen_pcibk_config_free_dev(psdev->dev);
+	xen_pcibk_config_free_dyn_fields(dev);
+	xen_pcibk_config_free_dev(dev);
 
-	psdev->dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
-	pci_dev_put(psdev->dev);
+	dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
+	pci_dev_put(dev);
 
 	kfree(psdev);
 }
@@ -355,6 +371,19 @@ static int pcistub_init_device(struct pc
 	if (err)
 		goto config_release;
 
+	if (pci_find_capability(dev, PCI_CAP_ID_MSIX)) {
+		struct physdev_pci_device ppdev = {
+			.seg = pci_domain_nr(dev->bus),
+			.bus = dev->bus->number,
+			.devfn = dev->devfn
+		};
+
+		err = HYPERVISOR_physdev_op(PHYSDEVOP_prepare_msix, &ppdev);
+		if (err)
+			dev_err(&dev->dev, "MSI-X preparation failed (%d)\n",
+				err);
+	}
+
 	/* We need the device active to save the state. */
 	dev_dbg(&dev->dev, "save state of device\n");
 	pci_save_state(dev);
--- 3.8-rc6/include/xen/interface/physdev.h
+++ 3.8-rc6-xen-pciback-MSI-X-prepare/include/xen/interface/physdev.h
@@ -251,6 +251,12 @@ struct physdev_pci_device_add {
 
 #define PHYSDEVOP_pci_device_remove     26
 #define PHYSDEVOP_restore_msi_ext       27
+/*
+ * Dom0 should use these two to announce MMIO resources assigned to
+ * MSI-X capable devices won't (prepare) or may (release) change.
+ */
+#define PHYSDEVOP_prepare_msix          30
+#define PHYSDEVOP_release_msix          31
 struct physdev_pci_device {
     /* IN */
     uint16_t seg;

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

end of thread, other threads:[~2013-03-12 20:06 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-06 16:58 [PATCH] xen-pciback: notify hypervisor about devices intended to be assigned to guests Jan Beulich
2013-02-06 17:12 ` Konrad Rzeszutek Wilk
2013-02-07  9:00   ` Jan Beulich
2013-02-07  9:00   ` Jan Beulich
2013-02-06 17:12 ` Konrad Rzeszutek Wilk
2013-02-06 16:58 Jan Beulich
2013-03-12 15:06 Jan Beulich
2013-03-12 15:06 Jan Beulich
2013-03-12 16:59 ` Jan Beulich
2013-03-12 16:59 ` [Xen-devel] " Jan Beulich
2013-03-12 20:06   ` Konrad Rzeszutek Wilk

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.