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

From: Konrad Rzeszutek Wilk
Date: Tue Mar 12 2013 - 16:06:47 EST


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;
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/