diff mbox

[net-next,1/8] pci: Add flag indicating device has been assigned by KVM

Message ID 1316599974-23205-2-git-send-email-jeffrey.t.kirsher@intel.com
State Superseded, archived
Delegated to: David Miller
Headers show

Commit Message

Kirsher, Jeffrey T Sept. 21, 2011, 10:12 a.m. UTC
From: Greg Rose <gregory.v.rose@intel.com>

Device drivers that create and destroy SR-IOV virtual functions via
calls to pci_enable_sriov() and pci_disable_sriov can cause catastrophic
failures if they attempt to destroy VFs while they are assigned to
guest virtual machines.  By adding a flag for use by the KVM module
to indicate that a device is assigned a device driver can check that
flag and avoid destroying VFs while they are assigned and avoid system
failures.

Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 include/linux/pci.h     |    2 ++
 virt/kvm/assigned-dev.c |    2 ++
 virt/kvm/iommu.c        |    4 ++++
 3 files changed, 8 insertions(+), 0 deletions(-)

Comments

Kirsher, Jeffrey T Sept. 23, 2011, 4:16 a.m. UTC | #1
On Wed, Sep 21, 2011 at 03:12, Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
> From: Greg Rose <gregory.v.rose@intel.com>
>
> Device drivers that create and destroy SR-IOV virtual functions via
> calls to pci_enable_sriov() and pci_disable_sriov can cause catastrophic
> failures if they attempt to destroy VFs while they are assigned to
> guest virtual machines.  By adding a flag for use by the KVM module
> to indicate that a device is assigned a device driver can check that
> flag and avoid destroying VFs while they are assigned and avoid system
> failures.
>
> Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>  include/linux/pci.h     |    2 ++
>  virt/kvm/assigned-dev.c |    2 ++
>  virt/kvm/iommu.c        |    4 ++++
>  3 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index f27893b..4f511da 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -174,6 +174,8 @@ enum pci_dev_flags {
>        PCI_DEV_FLAGS_MSI_INTX_DISABLE_BUG = (__force pci_dev_flags_t) 1,
>        /* Device configuration is irrevocably lost if disabled into D3 */
>        PCI_DEV_FLAGS_NO_D3 = (__force pci_dev_flags_t) 2,
> +       /* Provide indication device is assigned by KVM */
> +       PCI_DEV_FLAGS_ASSIGNED = (__force pci_dev_flags_t) 4,
>  };
>
>  enum pci_irq_reroute_variant {
> diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c
> index 4e9eaeb..eaf3a50 100644
> --- a/virt/kvm/assigned-dev.c
> +++ b/virt/kvm/assigned-dev.c
> @@ -205,6 +205,8 @@ static void kvm_free_assigned_device(struct kvm *kvm,
>        else
>                pci_restore_state(assigned_dev->dev);
>
> +       assigned_dev->dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
> +
>        pci_release_regions(assigned_dev->dev);
>        pci_disable_device(assigned_dev->dev);
>        pci_dev_put(assigned_dev->dev);
> diff --git a/virt/kvm/iommu.c b/virt/kvm/iommu.c
> index 78c80f6..967aba1 100644
> --- a/virt/kvm/iommu.c
> +++ b/virt/kvm/iommu.c
> @@ -187,6 +187,8 @@ int kvm_assign_device(struct kvm *kvm,
>                        goto out_unmap;
>        }
>
> +       pdev->dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
> +
>        printk(KERN_DEBUG "assign device %x:%x:%x.%x\n",
>                assigned_dev->host_segnr,
>                assigned_dev->host_busnr,
> @@ -215,6 +217,8 @@ int kvm_deassign_device(struct kvm *kvm,
>
>        iommu_detach_device(domain, &pdev->dev);
>
> +       pdev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
> +
>        printk(KERN_DEBUG "deassign device %x:%x:%x.%x\n",
>                assigned_dev->host_segnr,
>                assigned_dev->host_busnr,
> --
> 1.7.6.2

Jesse/Konrad/Ian-

I sent this patch out as part of a pull request for David Miller's
net-next tree.  I know that Greg sent this originally out to the
linux-pci mailing list as a RFC. Since Greg also has a patch against
ixgbe which implemented this flag, I sent both patches for inclusion
into David Miller's net-next.

Dave is wanting to ensure that the PCI maintainers have reviewed this
and are ok with it before pulls my series of patches.
Jesse Barnes Sept. 23, 2011, 5:20 a.m. UTC | #2
[oops meant to cc everyone, yes this has my ack]

On Thu, 22 Sep 2011 21:16:18 -0700
Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:

> On Wed, Sep 21, 2011 at 03:12, Jeff Kirsher
> <jeffrey.t.kirsher@intel.com> wrote:
> > From: Greg Rose <gregory.v.rose@intel.com>
> >
> > Device drivers that create and destroy SR-IOV virtual functions via
> > calls to pci_enable_sriov() and pci_disable_sriov can cause
> > catastrophic failures if they attempt to destroy VFs while they are
> > assigned to guest virtual machines.  By adding a flag for use by
> > the KVM module to indicate that a device is assigned a device
> > driver can check that flag and avoid destroying VFs while they are
> > assigned and avoid system failures.
> >
> > Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > ---
> >  include/linux/pci.h     |    2 ++
> >  virt/kvm/assigned-dev.c |    2 ++
> >  virt/kvm/iommu.c        |    4 ++++
> >  3 files changed, 8 insertions(+), 0 deletions(-)
> >
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index f27893b..4f511da 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -174,6 +174,8 @@ enum pci_dev_flags {
> >        PCI_DEV_FLAGS_MSI_INTX_DISABLE_BUG = (__force
> > pci_dev_flags_t) 1, /* Device configuration is irrevocably lost if
> > disabled into D3 */ PCI_DEV_FLAGS_NO_D3 = (__force pci_dev_flags_t)
> > 2,
> > +       /* Provide indication device is assigned by KVM */
> > +       PCI_DEV_FLAGS_ASSIGNED = (__force pci_dev_flags_t) 4,
> >  };
> >
> >  enum pci_irq_reroute_variant {
> > diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c
> > index 4e9eaeb..eaf3a50 100644
> > --- a/virt/kvm/assigned-dev.c
> > +++ b/virt/kvm/assigned-dev.c
> > @@ -205,6 +205,8 @@ static void kvm_free_assigned_device(struct kvm
> > *kvm, else
> >                pci_restore_state(assigned_dev->dev);
> >
> > +       assigned_dev->dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
> > +
> >        pci_release_regions(assigned_dev->dev);
> >        pci_disable_device(assigned_dev->dev);
> >        pci_dev_put(assigned_dev->dev);
> > diff --git a/virt/kvm/iommu.c b/virt/kvm/iommu.c
> > index 78c80f6..967aba1 100644
> > --- a/virt/kvm/iommu.c
> > +++ b/virt/kvm/iommu.c
> > @@ -187,6 +187,8 @@ int kvm_assign_device(struct kvm *kvm,
> >                        goto out_unmap;
> >        }
> >
> > +       pdev->dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
> > +
> >        printk(KERN_DEBUG "assign device %x:%x:%x.%x\n",
> >                assigned_dev->host_segnr,
> >                assigned_dev->host_busnr,
> > @@ -215,6 +217,8 @@ int kvm_deassign_device(struct kvm *kvm,
> >
> >        iommu_detach_device(domain, &pdev->dev);
> >
> > +       pdev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
> > +
> >        printk(KERN_DEBUG "deassign device %x:%x:%x.%x\n",
> >                assigned_dev->host_segnr,
> >                assigned_dev->host_busnr,
> > --
> > 1.7.6.2
> 
> Jesse/Konrad/Ian-
> 
> I sent this patch out as part of a pull request for David Miller's
> net-next tree.  I know that Greg sent this originally out to the
> linux-pci mailing list as a RFC. Since Greg also has a patch against
> ixgbe which implemented this flag, I sent both patches for inclusion
> into David Miller's net-next.
> 
> Dave is wanting to ensure that the PCI maintainers have reviewed this
> and are ok with it before pulls my series of patches.
> 

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kirsher, Jeffrey T Sept. 23, 2011, 5:23 a.m. UTC | #3
On Thu, 2011-09-22 at 22:19 -0700, Jesse Barnes wrote:
> On Thu, 22 Sep 2011 21:16:18 -0700
> Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
> 
> > On Wed, Sep 21, 2011 at 03:12, Jeff Kirsher
> > <jeffrey.t.kirsher@intel.com> wrote:
> > > From: Greg Rose <gregory.v.rose@intel.com>
> > >
> > > Device drivers that create and destroy SR-IOV virtual functions via
> > > calls to pci_enable_sriov() and pci_disable_sriov can cause
> > > catastrophic failures if they attempt to destroy VFs while they are
> > > assigned to guest virtual machines.  By adding a flag for use by
> > > the KVM module to indicate that a device is assigned a device
> > > driver can check that flag and avoid destroying VFs while they are
> > > assigned and avoid system failures.
> > >
> > > Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> > > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > > ---
> > >  include/linux/pci.h     |    2 ++
> > >  virt/kvm/assigned-dev.c |    2 ++
> > >  virt/kvm/iommu.c        |    4 ++++
> > >  3 files changed, 8 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > > index f27893b..4f511da 100644
> > > --- a/include/linux/pci.h
> > > +++ b/include/linux/pci.h
> > > @@ -174,6 +174,8 @@ enum pci_dev_flags {
> > >        PCI_DEV_FLAGS_MSI_INTX_DISABLE_BUG = (__force
> > > pci_dev_flags_t) 1, /* Device configuration is irrevocably lost if
> > > disabled into D3 */ PCI_DEV_FLAGS_NO_D3 = (__force pci_dev_flags_t)
> > > 2,
> > > +       /* Provide indication device is assigned by KVM */
> > > +       PCI_DEV_FLAGS_ASSIGNED = (__force pci_dev_flags_t) 4,
> > >  };
> > >
> > >  enum pci_irq_reroute_variant {
> > > diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c
> > > index 4e9eaeb..eaf3a50 100644
> > > --- a/virt/kvm/assigned-dev.c
> > > +++ b/virt/kvm/assigned-dev.c
> > > @@ -205,6 +205,8 @@ static void kvm_free_assigned_device(struct kvm
> > > *kvm, else
> > >                pci_restore_state(assigned_dev->dev);
> > >
> > > +       assigned_dev->dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
> > > +
> > >        pci_release_regions(assigned_dev->dev);
> > >        pci_disable_device(assigned_dev->dev);
> > >        pci_dev_put(assigned_dev->dev);
> > > diff --git a/virt/kvm/iommu.c b/virt/kvm/iommu.c
> > > index 78c80f6..967aba1 100644
> > > --- a/virt/kvm/iommu.c
> > > +++ b/virt/kvm/iommu.c
> > > @@ -187,6 +187,8 @@ int kvm_assign_device(struct kvm *kvm,
> > >                        goto out_unmap;
> > >        }
> > >
> > > +       pdev->dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
> > > +
> > >        printk(KERN_DEBUG "assign device %x:%x:%x.%x\n",
> > >                assigned_dev->host_segnr,
> > >                assigned_dev->host_busnr,
> > > @@ -215,6 +217,8 @@ int kvm_deassign_device(struct kvm *kvm,
> > >
> > >        iommu_detach_device(domain, &pdev->dev);
> > >
> > > +       pdev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
> > > +
> > >        printk(KERN_DEBUG "deassign device %x:%x:%x.%x\n",
> > >                assigned_dev->host_segnr,
> > >                assigned_dev->host_busnr,
> > > --
> > > 1.7.6.2
> > 
> > Jesse/Konrad/Ian-
> > 
> > I sent this patch out as part of a pull request for David Miller's
> > net-next tree.  I know that Greg sent this originally out to the
> > linux-pci mailing list as a RFC. Since Greg also has a patch against
> > ixgbe which implemented this flag, I sent both patches for inclusion
> > into David Miller's net-next.
> > 
> > Dave is wanting to ensure that the PCI maintainers have reviewed this
> > and are ok with it before pulls my series of patches.
> 
> Yeah I think I looked at this one in the past, my ack must have been
> dropped.  You can add it if you want.
> 
> Jesse

Thanks Jesse!
Ian Campbell Sept. 23, 2011, 7:27 a.m. UTC | #4
On Thu, 2011-09-22 at 21:16 -0700, Jeff Kirsher wrote:
> 
> 
> Jesse/Konrad/Ian-
> 
> I sent this patch out as part of a pull request for David Miller's
> net-next tree.  I know that Greg sent this originally out to the
> linux-pci mailing list as a RFC. Since Greg also has a patch against
> ixgbe which implemented this flag, I sent both patches for inclusion
> into David Miller's net-next.
> 
> Dave is wanting to ensure that the PCI maintainers have reviewed this
> and are ok with it before pulls my series of patches.

I'm not a PCI maintainer by any stretch of the imagination but FWIW this
change is fine by me.

My original reason for commenting on this patch was just to wonder
whether this would also be useful for Xen and I think the answer is we
should patch xen-pciback to use this new flag but I've not had time to
look into that.

I suppose by that measure the comment could be less KVM specific:
> +       /* Provide indication device is assigned by KVM */
> +       PCI_DEV_FLAGS_ASSIGNED = (__force pci_dev_flags_t) 4,

but that's not exactly a big deal.

I suppose really the flag indicates "VF in use" rather than necessarily
"assigned"? Would it be just as bad to have a VF driver in the host
active when the PF was unloaded?

Ian.
Rose, Gregory V Sept. 23, 2011, 2:41 p.m. UTC | #5
> -----Original Message-----

> From: Ian Campbell [mailto:ijc@hellion.org.uk]

> Sent: Friday, September 23, 2011 12:28 AM

> To: Kirsher, Jeffrey T

> Cc: davem@davemloft.net; konrad.wilk@oracle.com; Jesse Barnes; Rose,

> Gregory V; netdev@vger.kernel.org; gospo@redhat.com; linux-

> pci@vger.kernel.org

> Subject: Re: [net-next 1/8] pci: Add flag indicating device has been

> assigned by KVM

> 

> On Thu, 2011-09-22 at 21:16 -0700, Jeff Kirsher wrote:

> >

> >

> > Jesse/Konrad/Ian-

> >

> > I sent this patch out as part of a pull request for David Miller's

> > net-next tree.  I know that Greg sent this originally out to the

> > linux-pci mailing list as a RFC. Since Greg also has a patch against

> > ixgbe which implemented this flag, I sent both patches for inclusion

> > into David Miller's net-next.

> >

> > Dave is wanting to ensure that the PCI maintainers have reviewed this

> > and are ok with it before pulls my series of patches.

> 

> I'm not a PCI maintainer by any stretch of the imagination but FWIW this

> change is fine by me.

> 

> My original reason for commenting on this patch was just to wonder

> whether this would also be useful for Xen and I think the answer is we

> should patch xen-pciback to use this new flag but I've not had time to

> look into that.

> 

> I suppose by that measure the comment could be less KVM specific:

> > +       /* Provide indication device is assigned by KVM */

> > +       PCI_DEV_FLAGS_ASSIGNED = (__force pci_dev_flags_t) 4,


We can resubmit with a more generic comment, maybe this:

/* Provide indication device is assigned by a Virtual Machine Manager */

> 

> but that's not exactly a big deal.

> 

> I suppose really the flag indicates "VF in use" rather than necessarily

> "assigned"? Would it be just as bad to have a VF driver in the host

> active when the PF was unloaded?


There is no issue with unloading an active VF in the host because the hot remove event is seen by the host and accesses to the device stop.  However, when you hot remove the device the host and it is assigned to a VM the VM is unaware of the event and has no way of propagating the hot remove event.  So it really is a VF assigned to VM flag.  There's no need to mark VFs as in use in the host.

- Greg
Ian Campbell Sept. 23, 2011, 3:03 p.m. UTC | #6
On Fri, 2011-09-23 at 07:41 -0700, Rose, Gregory V wrote:
> > -----Original Message-----
> > From: Ian Campbell [mailto:ijc@hellion.org.uk]
> > Sent: Friday, September 23, 2011 12:28 AM
> > To: Kirsher, Jeffrey T
> > Cc: davem@davemloft.net; konrad.wilk@oracle.com; Jesse Barnes; Rose,
> > Gregory V; netdev@vger.kernel.org; gospo@redhat.com; linux-
> > pci@vger.kernel.org
> > Subject: Re: [net-next 1/8] pci: Add flag indicating device has been
> > assigned by KVM
> > 
> > On Thu, 2011-09-22 at 21:16 -0700, Jeff Kirsher wrote:
> > >
> > >
> > > Jesse/Konrad/Ian-
> > >
> > > I sent this patch out as part of a pull request for David Miller's
> > > net-next tree.  I know that Greg sent this originally out to the
> > > linux-pci mailing list as a RFC. Since Greg also has a patch against
> > > ixgbe which implemented this flag, I sent both patches for inclusion
> > > into David Miller's net-next.
> > >
> > > Dave is wanting to ensure that the PCI maintainers have reviewed this
> > > and are ok with it before pulls my series of patches.
> > 
> > I'm not a PCI maintainer by any stretch of the imagination but FWIW this
> > change is fine by me.
> > 
> > My original reason for commenting on this patch was just to wonder
> > whether this would also be useful for Xen and I think the answer is we
> > should patch xen-pciback to use this new flag but I've not had time to
> > look into that.
> > 
> > I suppose by that measure the comment could be less KVM specific:
> > > +       /* Provide indication device is assigned by KVM */
> > > +       PCI_DEV_FLAGS_ASSIGNED = (__force pci_dev_flags_t) 4,
> 
> We can resubmit with a more generic comment, maybe this:
> 
> /* Provide indication device is assigned by a Virtual Machine Manager */

Sounds good to me.
> 
> > 
> > but that's not exactly a big deal.
> > 
> > I suppose really the flag indicates "VF in use" rather than necessarily
> > "assigned"? Would it be just as bad to have a VF driver in the host
> > active when the PF was unloaded?
> 
> There is no issue with unloading an active VF in the host because the
> hot remove event is seen by the host and accesses to the device stop.
> However, when you hot remove the device the host and it is assigned to
> a VM the VM is unaware of the event and has no way of propagating the
> hot remove event.  So it really is a VF assigned to VM flag.  There's
> no need to mark VFs as in use in the host.

Ah, ok. Thanks for explaining.

Ian.
Kirsher, Jeffrey T Sept. 23, 2011, 3:45 p.m. UTC | #7
On Fri, 2011-09-23 at 08:11 -0700, Rose, Gregory V wrote:
> > -----Original Message-----
> > From: Ian Campbell [mailto:ijc@hellion.org.uk]
> > Sent: Friday, September 23, 2011 8:04 AM
> > To: Rose, Gregory V
> > Cc: Kirsher, Jeffrey T; davem@davemloft.net; konrad.wilk@oracle.com; Jesse
> > Barnes; netdev@vger.kernel.org; gospo@redhat.com; linux-
> > pci@vger.kernel.org
> > Subject: RE: [net-next 1/8] pci: Add flag indicating device has been
> > assigned by KVM
> > 
> > On Fri, 2011-09-23 at 07:41 -0700, Rose, Gregory V wrote:
> > > > -----Original Message-----
> > > > From: Ian Campbell [mailto:ijc@hellion.org.uk]
> > > > Sent: Friday, September 23, 2011 12:28 AM
> > > > To: Kirsher, Jeffrey T
> > > > Cc: davem@davemloft.net; konrad.wilk@oracle.com; Jesse Barnes; Rose,
> > > > Gregory V; netdev@vger.kernel.org; gospo@redhat.com; linux-
> > > > pci@vger.kernel.org
> > > > Subject: Re: [net-next 1/8] pci: Add flag indicating device has been
> > > > assigned by KVM
> > > >
> > > > I suppose by that measure the comment could be less KVM specific:
> > > > > +       /* Provide indication device is assigned by KVM */
> > > > > +       PCI_DEV_FLAGS_ASSIGNED = (__force pci_dev_flags_t) 4,
> > >
> > > We can resubmit with a more generic comment, maybe this:
> > >
> > > /* Provide indication device is assigned by a Virtual Machine Manager */
> > 
> > Sounds good to me.
> 
> Dave, Jeff,
> 
> Should I resubmit the patch or would it be more convenient to post a follow on patch that fixes up the comment?  Either way is fine by me.
> 
> - Greg
> 

let's fix up the patch in my tree and add Jesse Barnes's ACK at the same
time.
Konrad Rzeszutek Wilk Sept. 23, 2011, 4:43 p.m. UTC | #8
On Fri, Sep 23, 2011 at 08:45:09AM -0700, Jeff Kirsher wrote:
> On Fri, 2011-09-23 at 08:11 -0700, Rose, Gregory V wrote:
> > > -----Original Message-----
> > > From: Ian Campbell [mailto:ijc@hellion.org.uk]
> > > Sent: Friday, September 23, 2011 8:04 AM
> > > To: Rose, Gregory V
> > > Cc: Kirsher, Jeffrey T; davem@davemloft.net; konrad.wilk@oracle.com; Jesse
> > > Barnes; netdev@vger.kernel.org; gospo@redhat.com; linux-
> > > pci@vger.kernel.org
> > > Subject: RE: [net-next 1/8] pci: Add flag indicating device has been
> > > assigned by KVM
> > > 
> > > On Fri, 2011-09-23 at 07:41 -0700, Rose, Gregory V wrote:
> > > > > -----Original Message-----
> > > > > From: Ian Campbell [mailto:ijc@hellion.org.uk]
> > > > > Sent: Friday, September 23, 2011 12:28 AM
> > > > > To: Kirsher, Jeffrey T
> > > > > Cc: davem@davemloft.net; konrad.wilk@oracle.com; Jesse Barnes; Rose,
> > > > > Gregory V; netdev@vger.kernel.org; gospo@redhat.com; linux-
> > > > > pci@vger.kernel.org
> > > > > Subject: Re: [net-next 1/8] pci: Add flag indicating device has been
> > > > > assigned by KVM
> > > > >
> > > > > I suppose by that measure the comment could be less KVM specific:
> > > > > > +       /* Provide indication device is assigned by KVM */
> > > > > > +       PCI_DEV_FLAGS_ASSIGNED = (__force pci_dev_flags_t) 4,
> > > >
> > > > We can resubmit with a more generic comment, maybe this:
> > > >
> > > > /* Provide indication device is assigned by a Virtual Machine Manager */
> > > 
> > > Sounds good to me.
> > 
> > Dave, Jeff,
> > 
> > Should I resubmit the patch or would it be more convenient to post a follow on patch that fixes up the comment?  Either way is fine by me.
> > 
> > - Greg
> > 
> 
> let's fix up the patch in my tree and add Jesse Barnes's ACK at the same
> time.

Were are the patches now? I am kind of curious to see the changes to the xen pciback.



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rose, Gregory V Sept. 23, 2011, 5:04 p.m. UTC | #9
> -----Original Message-----
> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> Sent: Friday, September 23, 2011 9:43 AM
> To: Kirsher, Jeffrey T
> Cc: Rose, Gregory V; Ian Campbell; davem@davemloft.net; Jesse Barnes;
> netdev@vger.kernel.org; gospo@redhat.com; linux-pci@vger.kernel.org
> Subject: Re: [net-next 1/8] pci: Add flag indicating device has been
> assigned by KVM
> 
> On Fri, Sep 23, 2011 at 08:45:09AM -0700, Jeff Kirsher wrote:
> > On Fri, 2011-09-23 at 08:11 -0700, Rose, Gregory V wrote:
> > > > -----Original Message-----
> > > > From: Ian Campbell [mailto:ijc@hellion.org.uk]
> > > > Sent: Friday, September 23, 2011 8:04 AM
> > > > To: Rose, Gregory V
> > > > Cc: Kirsher, Jeffrey T; davem@davemloft.net; konrad.wilk@oracle.com;
> Jesse
> > > > Barnes; netdev@vger.kernel.org; gospo@redhat.com; linux-
> > > > pci@vger.kernel.org
> > > > Subject: RE: [net-next 1/8] pci: Add flag indicating device has been
> > > > assigned by KVM
> > > >
> > > > On Fri, 2011-09-23 at 07:41 -0700, Rose, Gregory V wrote:
> > > > > > -----Original Message-----
> > > > > > From: Ian Campbell [mailto:ijc@hellion.org.uk]
> > > > > > Sent: Friday, September 23, 2011 12:28 AM
> > > > > > To: Kirsher, Jeffrey T
> > > > > > Cc: davem@davemloft.net; konrad.wilk@oracle.com; Jesse Barnes;
> Rose,
> > > > > > Gregory V; netdev@vger.kernel.org; gospo@redhat.com; linux-
> > > > > > pci@vger.kernel.org
> > > > > > Subject: Re: [net-next 1/8] pci: Add flag indicating device has
> been
> > > > > > assigned by KVM
> > > > > >
> > > > > > I suppose by that measure the comment could be less KVM
> specific:
> > > > > > > +       /* Provide indication device is assigned by KVM */
> > > > > > > +       PCI_DEV_FLAGS_ASSIGNED = (__force pci_dev_flags_t) 4,
> > > > >
> > > > > We can resubmit with a more generic comment, maybe this:
> > > > >
> > > > > /* Provide indication device is assigned by a Virtual Machine
> Manager */
> > > >
> > > > Sounds good to me.
> > >
> > > Dave, Jeff,
> > >
> > > Should I resubmit the patch or would it be more convenient to post a
> follow on patch that fixes up the comment?  Either way is fine by me.
> > >
> > > - Greg
> > >
> >
> > let's fix up the patch in my tree and add Jesse Barnes's ACK at the same
> > time.
> 
> Were are the patches now? I am kind of curious to see the changes to the
> xen pciback.
> 
> 

Version 2 of this patch with the modified comment was just posted to netdev.  Ian Campbell had mentioned that the Xen pciback driver should be modified to use this flag but I don't believe any patches for it have been created yet.

- Greg
 

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Konrad Rzeszutek Wilk Sept. 23, 2011, 6 p.m. UTC | #10
On Fri, Sep 23, 2011 at 10:04:59AM -0700, Rose, Gregory V wrote:
> > -----Original Message-----
> > From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> > Sent: Friday, September 23, 2011 9:43 AM
> > To: Kirsher, Jeffrey T
> > Cc: Rose, Gregory V; Ian Campbell; davem@davemloft.net; Jesse Barnes;
> > netdev@vger.kernel.org; gospo@redhat.com; linux-pci@vger.kernel.org
> > Subject: Re: [net-next 1/8] pci: Add flag indicating device has been
> > assigned by KVM
> > 
> > On Fri, Sep 23, 2011 at 08:45:09AM -0700, Jeff Kirsher wrote:
> > > On Fri, 2011-09-23 at 08:11 -0700, Rose, Gregory V wrote:
> > > > > -----Original Message-----
> > > > > From: Ian Campbell [mailto:ijc@hellion.org.uk]
> > > > > Sent: Friday, September 23, 2011 8:04 AM
> > > > > To: Rose, Gregory V
> > > > > Cc: Kirsher, Jeffrey T; davem@davemloft.net; konrad.wilk@oracle.com;
> > Jesse
> > > > > Barnes; netdev@vger.kernel.org; gospo@redhat.com; linux-
> > > > > pci@vger.kernel.org
> > > > > Subject: RE: [net-next 1/8] pci: Add flag indicating device has been
> > > > > assigned by KVM
> > > > >
> > > > > On Fri, 2011-09-23 at 07:41 -0700, Rose, Gregory V wrote:
> > > > > > > -----Original Message-----
> > > > > > > From: Ian Campbell [mailto:ijc@hellion.org.uk]
> > > > > > > Sent: Friday, September 23, 2011 12:28 AM
> > > > > > > To: Kirsher, Jeffrey T
> > > > > > > Cc: davem@davemloft.net; konrad.wilk@oracle.com; Jesse Barnes;
> > Rose,
> > > > > > > Gregory V; netdev@vger.kernel.org; gospo@redhat.com; linux-
> > > > > > > pci@vger.kernel.org
> > > > > > > Subject: Re: [net-next 1/8] pci: Add flag indicating device has
> > been
> > > > > > > assigned by KVM
> > > > > > >
> > > > > > > I suppose by that measure the comment could be less KVM
> > specific:
> > > > > > > > +       /* Provide indication device is assigned by KVM */
> > > > > > > > +       PCI_DEV_FLAGS_ASSIGNED = (__force pci_dev_flags_t) 4,
> > > > > >
> > > > > > We can resubmit with a more generic comment, maybe this:
> > > > > >
> > > > > > /* Provide indication device is assigned by a Virtual Machine
> > Manager */
> > > > >
> > > > > Sounds good to me.
> > > >
> > > > Dave, Jeff,
> > > >
> > > > Should I resubmit the patch or would it be more convenient to post a
> > follow on patch that fixes up the comment?  Either way is fine by me.
> > > >
> > > > - Greg
> > > >
> > >
> > > let's fix up the patch in my tree and add Jesse Barnes's ACK at the same
> > > time.
> > 
> > Were are the patches now? I am kind of curious to see the changes to the
> > xen pciback.
> > 
> > 
> 
> Version 2 of this patch with the modified comment was just posted to netdev.  Ian Campbell had mentioned that the Xen pciback driver should be modified to use this flag but I don't believe any patches for it have been created yet.

http://marc.info/?i=20110729165446.GA6731@dumpdata.com

?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rose, Gregory V Sept. 23, 2011, 6:24 p.m. UTC | #11
> -----Original Message-----
> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> Sent: Friday, September 23, 2011 11:00 AM
> To: Rose, Gregory V
> Cc: Kirsher, Jeffrey T; Ian Campbell; davem@davemloft.net; Jesse Barnes;
> netdev@vger.kernel.org; gospo@redhat.com; linux-pci@vger.kernel.org
> Subject: Re: [net-next 1/8] pci: Add flag indicating device has been
> assigned by KVM
> 
> On Fri, Sep 23, 2011 at 10:04:59AM -0700, Rose, Gregory V wrote:
> > > -----Original Message-----
> > > From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> > > Sent: Friday, September 23, 2011 9:43 AM
> > > To: Kirsher, Jeffrey T
> > > Cc: Rose, Gregory V; Ian Campbell; davem@davemloft.net; Jesse Barnes;
> > > netdev@vger.kernel.org; gospo@redhat.com; linux-pci@vger.kernel.org
> > > Subject: Re: [net-next 1/8] pci: Add flag indicating device has been
> > > assigned by KVM
> > >
> > > On Fri, Sep 23, 2011 at 08:45:09AM -0700, Jeff Kirsher wrote:
> > > > On Fri, 2011-09-23 at 08:11 -0700, Rose, Gregory V wrote:
> > > > > > -----Original Message-----
> > > > > > From: Ian Campbell [mailto:ijc@hellion.org.uk]
> > > > > > Sent: Friday, September 23, 2011 8:04 AM
> > > > > > To: Rose, Gregory V
> > > > > > Cc: Kirsher, Jeffrey T; davem@davemloft.net;
> konrad.wilk@oracle.com;
> > > Jesse
> > > > > > Barnes; netdev@vger.kernel.org; gospo@redhat.com; linux-
> > > > > > pci@vger.kernel.org
> > > > > > Subject: RE: [net-next 1/8] pci: Add flag indicating device has
> been
> > > > > > assigned by KVM
> > > > > >
> > > > > > On Fri, 2011-09-23 at 07:41 -0700, Rose, Gregory V wrote:
> > > > > > > > -----Original Message-----
> > > > > > > > From: Ian Campbell [mailto:ijc@hellion.org.uk]
> > > > > > > > Sent: Friday, September 23, 2011 12:28 AM
> > > > > > > > To: Kirsher, Jeffrey T
> > > > > > > > Cc: davem@davemloft.net; konrad.wilk@oracle.com; Jesse
> Barnes;
> > > Rose,
> > > > > > > > Gregory V; netdev@vger.kernel.org; gospo@redhat.com; linux-
> > > > > > > > pci@vger.kernel.org
> > > > > > > > Subject: Re: [net-next 1/8] pci: Add flag indicating device
> has
> > > been
> > > > > > > > assigned by KVM
> > > > > > > >
> > > > > > > > I suppose by that measure the comment could be less KVM
> > > specific:
> > > > > > > > > +       /* Provide indication device is assigned by KVM */
> > > > > > > > > +       PCI_DEV_FLAGS_ASSIGNED = (__force pci_dev_flags_t)
> 4,
> > > > > > >
> > > > > > > We can resubmit with a more generic comment, maybe this:
> > > > > > >
> > > > > > > /* Provide indication device is assigned by a Virtual Machine
> > > Manager */
> > > > > >
> > > > > > Sounds good to me.
> > > > >
> > > > > Dave, Jeff,
> > > > >
> > > > > Should I resubmit the patch or would it be more convenient to post
> a
> > > follow on patch that fixes up the comment?  Either way is fine by me.
> > > > >
> > > > > - Greg
> > > > >
> > > >
> > > > let's fix up the patch in my tree and add Jesse Barnes's ACK at the
> same
> > > > time.
> > >
> > > Were are the patches now? I am kind of curious to see the changes to
> the
> > > xen pciback.
> > >
> > >
> >
> > Version 2 of this patch with the modified comment was just posted to
> netdev.  Ian Campbell had mentioned that the Xen pciback driver should be
> modified to use this flag but I don't believe any patches for it have been
> created yet.
> 
> http://marc.info/?i=20110729165446.GA6731@dumpdata.com
> 

I'm not sure who generated the patches to xen-pciback.  It wasn't me and I couldn't tell from the links you provided who did generate that patch.  I don't see it now though.

- Greg
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Konrad Rzeszutek Wilk Sept. 23, 2011, 7:05 p.m. UTC | #12
> > > Version 2 of this patch with the modified comment was just posted to
> > netdev.  Ian Campbell had mentioned that the Xen pciback driver should be
> > modified to use this flag but I don't believe any patches for it have been
> > created yet.
> > 
> > http://marc.info/?i=20110729165446.GA6731@dumpdata.com
> > 
> 
> I'm not sure who generated the patches to xen-pciback.  It wasn't me and I couldn't tell from the links you provided who did generate that patch.  I don't see it now though.

I provided it.

And I asked for a git tree so I could double check that it actually
worked as I don't have the full set of your patches.

Can you provide me a link to your public git tree or just repost it once more?
> 
> - Greg
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rose, Gregory V Sept. 23, 2011, 7:20 p.m. UTC | #13
> -----Original Message-----
> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> Sent: Friday, September 23, 2011 12:06 PM
> To: Rose, Gregory V
> Cc: Kirsher, Jeffrey T; Ian Campbell; davem@davemloft.net; Jesse Barnes;
> netdev@vger.kernel.org; gospo@redhat.com; linux-pci@vger.kernel.org
> Subject: Re: [net-next 1/8] pci: Add flag indicating device has been
> assigned by KVM
> 
> > > > Version 2 of this patch with the modified comment was just posted to
> > > netdev.  Ian Campbell had mentioned that the Xen pciback driver should
> be
> > > modified to use this flag but I don't believe any patches for it have
> been
> > > created yet.
> > >
> > > http://marc.info/?i=20110729165446.GA6731@dumpdata.com
> > >
> >
> > I'm not sure who generated the patches to xen-pciback.  It wasn't me and
> I couldn't tell from the links you provided who did generate that patch.
> I don't see it now though.
> 
> I provided it.
> 
> And I asked for a git tree so I could double check that it actually
> worked as I don't have the full set of your patches.
> 
> Can you provide me a link to your public git tree or just repost it once
> more?

Ah, I get it now.

Jeff Kirsher is the maintainer of our git trees.  Hopefully he'll come around and provide that info for you soon.

- Greg

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kirsher, Jeffrey T Sept. 24, 2011, 4:41 a.m. UTC | #14
On Fri, 2011-09-23 at 12:05 -0700, Konrad Rzeszutek Wilk wrote:
> > > > Version 2 of this patch with the modified comment was just posted to
> > > netdev.  Ian Campbell had mentioned that the Xen pciback driver should be
> > > modified to use this flag but I don't believe any patches for it have been
> > > created yet.
> > > 
> > > http://marc.info/?i=20110729165446.GA6731@dumpdata.com
> > > 
> > 
> > I'm not sure who generated the patches to xen-pciback.  It wasn't me and I couldn't tell from the links you provided who did generate that patch.  I don't see it now though.
> 
> I provided it.
> 
> And I asked for a git tree so I could double check that it actually
> worked as I don't have the full set of your patches.
> 
> Can you provide me a link to your public git tree or just repost it once more?
> > 
> > - Greg

You can pull my tree or David Miller's net-next tree.  Both have this
patch now.  Here are the URL's

David Miller's net-next tree:
git://github.com/davem330/net-next.git

My net-next tree:
git://github.com/Jkirsher/net-next.git
diff mbox

Patch

diff --git a/include/linux/pci.h b/include/linux/pci.h
index f27893b..4f511da 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -174,6 +174,8 @@  enum pci_dev_flags {
 	PCI_DEV_FLAGS_MSI_INTX_DISABLE_BUG = (__force pci_dev_flags_t) 1,
 	/* Device configuration is irrevocably lost if disabled into D3 */
 	PCI_DEV_FLAGS_NO_D3 = (__force pci_dev_flags_t) 2,
+	/* Provide indication device is assigned by KVM */
+	PCI_DEV_FLAGS_ASSIGNED = (__force pci_dev_flags_t) 4,
 };
 
 enum pci_irq_reroute_variant {
diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c
index 4e9eaeb..eaf3a50 100644
--- a/virt/kvm/assigned-dev.c
+++ b/virt/kvm/assigned-dev.c
@@ -205,6 +205,8 @@  static void kvm_free_assigned_device(struct kvm *kvm,
 	else
 		pci_restore_state(assigned_dev->dev);
 
+	assigned_dev->dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
+
 	pci_release_regions(assigned_dev->dev);
 	pci_disable_device(assigned_dev->dev);
 	pci_dev_put(assigned_dev->dev);
diff --git a/virt/kvm/iommu.c b/virt/kvm/iommu.c
index 78c80f6..967aba1 100644
--- a/virt/kvm/iommu.c
+++ b/virt/kvm/iommu.c
@@ -187,6 +187,8 @@  int kvm_assign_device(struct kvm *kvm,
 			goto out_unmap;
 	}
 
+	pdev->dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+
 	printk(KERN_DEBUG "assign device %x:%x:%x.%x\n",
 		assigned_dev->host_segnr,
 		assigned_dev->host_busnr,
@@ -215,6 +217,8 @@  int kvm_deassign_device(struct kvm *kvm,
 
 	iommu_detach_device(domain, &pdev->dev);
 
+	pdev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
+
 	printk(KERN_DEBUG "deassign device %x:%x:%x.%x\n",
 		assigned_dev->host_segnr,
 		assigned_dev->host_busnr,