[PATCH 0/4 v3] Introduce device assignment flag operation helper function

Discussion:

(too old to reply)

Ethan Zhao

2014-07-29 04:10:01 UTC

This patch set introduces three PCI device flag operation helper functions
when set pci device PF/VF to assigned or deassigned status also check it.
and patch 2,3,4 apply these helper functions to KVM,XEN and PCI.

v2: simplify unnecessory ternary operation in function pci_is_dev_assigned().
v3: amend helper function naming.

Appreciate suggestion from
***@redhat.com,
***@citrix.com,
***@intel.com

BR,
Ethan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Ethan Zhao

2014-07-29 04:10:02 UTC

Permalink

Use helper function instead of direct operation to pci device
flag when set device to assigned or deassigned.

Signed-off-by: Ethan Zhao <***@oracle.com>
---
v3: amend helper functions naming.
virt/kvm/assigned-dev.c | 2 +-
virt/kvm/iommu.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c
index bf06577..d122bda 100644
--- a/virt/kvm/assigned-dev.c
+++ b/virt/kvm/assigned-dev.c
@@ -302,7 +302,7 @@ 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_clear_dev_assigned(assigned_dev->dev);

pci_release_regions(assigned_dev->dev);
pci_disable_device(assigned_dev->dev);
diff --git a/virt/kvm/iommu.c b/virt/kvm/iommu.c
index 0df7d4b..8cfe021 100644
--- a/virt/kvm/iommu.c
+++ b/virt/kvm/iommu.c
@@ -194,7 +194,7 @@ int kvm_assign_device(struct kvm *kvm,
goto out_unmap;
}

- pdev->dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+ pci_set_dev_assigned(pdev);

dev_info(&pdev->dev, "kvm assign device\n");

@@ -220,7 +220,7 @@ int kvm_deassign_device(struct kvm *kvm,

iommu_detach_device(domain, &pdev->dev);

- pdev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
+ pci_clear_dev_assigned(pdev);

dev_info(&pdev->dev, "kvm deassign device\n");

--
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Ethan Zhao

2014-07-29 12:30:02 UTC

Permalink

This patch was already
Acked-by: Paolo Bonzini <***@redhat.com>
I forgot to add the Ack tag.

Thanks,
Ethan

Sorry for last post in HTML format with ipad.

Post by Ethan Zhao
Use helper function instead of direct operation to pci device
flag when set device to assigned or deassigned.
---
v3: amend helper functions naming.
virt/kvm/assigned-dev.c | 2 +-
virt/kvm/iommu.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c
index bf06577..d122bda 100644
--- a/virt/kvm/assigned-dev.c
+++ b/virt/kvm/assigned-dev.c
@@ -302,7 +302,7 @@ 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_clear_dev_assigned(assigned_dev->dev);
pci_release_regions(assigned_dev->dev);
pci_disable_device(assigned_dev->dev);
diff --git a/virt/kvm/iommu.c b/virt/kvm/iommu.c
index 0df7d4b..8cfe021 100644
--- a/virt/kvm/iommu.c
+++ b/virt/kvm/iommu.c
@@ -194,7 +194,7 @@ int kvm_assign_device(struct kvm *kvm,
goto out_unmap;
}
- pdev->dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+ pci_set_dev_assigned(pdev);
dev_info(&pdev->dev, "kvm assign device\n");
@@ -220,7 +220,7 @@ int kvm_deassign_device(struct kvm *kvm,
iommu_detach_device(domain, &pdev->dev);
- pdev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
+ pci_clear_dev_assigned(pdev);
dev_info(&pdev->dev, "kvm deassign device\n");
--
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Ethan Zhao

2014-07-29 04:10:02 UTC

Permalink

Use pci device flag operation helper functions when set device
to assigned or deassigned state.

Signed-off-by: Ethan Zhao <***@oracle.com>
---
v3: amend helper functions naming.
drivers/xen/xen-pciback/pci_stub.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c
index 62fcd48..71f69f1 100644
--- a/drivers/xen/xen-pciback/pci_stub.c
+++ b/drivers/xen/xen-pciback/pci_stub.c
@@ -133,7 +133,7 @@ static void pcistub_device_release(struct kref *kref)
xen_pcibk_config_free_dyn_fields(dev);
xen_pcibk_config_free_dev(dev);

- dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
+ pci_clear_dev_assigned(dev);
pci_dev_put(dev);

kfree(psdev);
@@ -404,7 +404,7 @@ static int pcistub_init_device(struct pci_dev *dev)
dev_dbg(&dev->dev, "reset device\n");
xen_pcibk_reset_device(dev);

- dev->dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+ pci_set_dev_assigned(dev);
return 0;

config_release:

David Vrabel

2014-07-29 10:10:03 UTC

Permalink

Post by Ethan Zhao
Use pci device flag operation helper functions when set device
to assigned or deassigned state.

Konrad already reviewed this but you've not included his reviewed-by tag.

I don't understand why we bother with this flag on Xen since we never
use it but:

Acked-by: David Vrabel <***@citrix.com>

I'm expecting this to go via the PCI tree.

David

Post by Ethan Zhao
--- a/drivers/xen/xen-pciback/pci_stub.c
+++ b/drivers/xen/xen-pciback/pci_stub.c
@@ -133,7 +133,7 @@ static void pcistub_device_release(struct kref *kref)
xen_pcibk_config_free_dyn_fields(dev);
xen_pcibk_config_free_dev(dev);
- dev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
+ pci_clear_dev_assigned(dev);
pci_dev_put(dev);
kfree(psdev);
@@ -404,7 +404,7 @@ static int pcistub_init_device(struct pci_dev *dev)
dev_dbg(&dev->dev, "reset device\n");
xen_pcibk_reset_device(dev);
- dev->dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+ pci_set_dev_assigned(dev);
return 0;

Ethan Zhao

2014-07-29 12:20:03 UTC

Permalink

Post by David Vrabel

Post by Ethan Zhao
Use pci device flag operation helper functions when set device
to assigned or deassigned state.

Konrad already reviewed this but you've not included his reviewed-by tag.

yep，I lost Konrad's reviewed-by tag.
Thanks.

Post by David Vrabel
I don't understand why we bother with this flag on Xen since we never
I'm expecting this to go via the PCI tree.
David

Ethan Zhao

2014-07-29 12:40:03 UTC

Permalink

Post by David Vrabel

Post by Ethan Zhao
Use pci device flag operation helper functions when set device
to assigned or deassigned state.

Konrad already reviewed this but you've not included his reviewed-by tag.
I don't understand why we bother with this flag on Xen since we never
I'm expecting this to go via the PCI tree.

Seems Bjorn still holds his bullet for last shot :)

Post by David Vrabel
David

Ethan Zhao

2014-07-29 04:10:02 UTC

Permalink

This patch introduced three helper functions to hide direct
device flag operation.

void pci_set_dev_assigned(struct pci_dev *pdev);
void pci_clear_dev_assigned(struct pci_dev *pdev);
bool pci_is_dev_assigned(struct pci_dev *pdev);

Signed-off-by: Ethan Zhao <***@oracle.com>
---
v2: simplify unnecessory ternary operation in function pci_is_dev_assigned();
v3: amend helper functions naming.
include/linux/pci.h | 13 +++++++++++++
1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index aab57b4..5f6f8fa 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1129,6 +1129,19 @@ resource_size_t pcibios_window_alignment(struct pci_bus *bus,

int pci_set_vga_state(struct pci_dev *pdev, bool decode,
unsigned int command_bits, u32 flags);
+/* helper functions for operation of device flag */
+static inline void pci_set_dev_assigned(struct pci_dev *pdev)
+{
+ pdev->dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+}
+static inline void pci_clear_dev_assigned(struct pci_dev *pdev)
+{
+ pdev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
+}
+static inline bool pci_is_dev_assigned(struct pci_dev *pdev)
+{
+ return pdev->dev_flags & PCI_DEV_FLAGS_ASSIGNED;
+}
/* kmem_cache style wrapper around pci_alloc_consistent() */

#include <linux/pci-dma.h>

Ethan Zhao

2014-07-29 04:10:02 UTC

Permalink

Use device flag operation helper functions when check device
assignment status.

Signed-off-by: Ethan Zhao <***@oracle.com>
---
v3: amend helper functions naming.
drivers/pci/iov.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index de7a747..61fad36 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -633,7 +633,7 @@ int pci_vfs_assigned(struct pci_dev *dev)
* our dev as the physical function and the assigned bit is set
*/
if (vfdev->is_virtfn && (vfdev->physfn == dev) &&
- (vfdev->dev_flags & PCI_DEV_FLAGS_ASSIGNED))
+ pci_is_dev_assigned(vfdev))
vfs_assigned++;

vfdev = pci_get_device(dev->vendor, dev_id, vfdev);