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

Re: [Xen-devel] [PATCH] xen-pciback: notify hypervisor about devices intended to be assigned to guests



On Tue, Mar 12, 2013 at 04:59:18PM +0000, Jan Beulich wrote:
> >>> On 12.03.13 at 16:06, "Jan Beulich" <JBeulich@xxxxxxxx> 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@xxxxxxxx>
> 
> 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;
> 
> 

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


 


Rackspace

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