2014-07-29 04:07:26

by Ethan Zhao

[permalink] [raw]
Subject: [PATCH 0/4 v3] Introduce device assignment flag operation helper function

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
[email protected],
[email protected],
[email protected]


BR,
Ethan


2014-07-29 04:07:36

by Ethan Zhao

[permalink] [raw]
Subject: [PATCH 3/4 v3] xen-pciback: use pci device flag operation helper function

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

Signed-off-by: Ethan Zhao <[email protected]>
---
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:
--
1.7.1

2014-07-29 04:07:52

by Ethan Zhao

[permalink] [raw]
Subject: [PATCH 4/4 v3] PCI: use device flag operation helper function in iov.c

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

Signed-off-by: Ethan Zhao <[email protected]>
---
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);
--
1.7.1

2014-07-29 04:08:12

by Ethan Zhao

[permalink] [raw]
Subject: [PATCH 2/4 v3] KVM: use pci device flag operation helper functions

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

Signed-off-by: Ethan Zhao <[email protected]>
---
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

2014-07-29 04:07:33

by Ethan Zhao

[permalink] [raw]
Subject: [PATCH 1/4 v3] PCI: introduce helper functions for device flag operation

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 <[email protected]>
---
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>
--
1.7.1

2014-07-29 10:07:31

by David Vrabel

[permalink] [raw]
Subject: Re: [PATCH 3/4 v3] xen-pciback: use pci device flag operation helper function

On 29/07/14 05:06, Ethan Zhao wrote:
> Use pci device flag operation helper functions when set device
> to assigned or deassigned state.
>
> Signed-off-by: Ethan Zhao <[email protected]>

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 <[email protected]>

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

David

> --- 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:
>

2014-07-29 12:16:28

by ethan zhao

[permalink] [raw]
Subject: Re: [PATCH 3/4 v3] xen-pciback: use pci device flag operation helper function



> ?? 2014??7??29?գ?????6:07??David Vrabel <[email protected]> д????
>
>> On 29/07/14 05:06, Ethan Zhao wrote:
>> Use pci device flag operation helper functions when set device
>> to assigned or deassigned state.
>>
>> Signed-off-by: Ethan Zhao <[email protected]>
>
> Konrad already reviewed this but you've not included his reviewed-by tag.
>
yep??I lost Konrad's reviewed-by tag.
Thanks.
> I don't understand why we bother with this flag on Xen since we never
> use it but:
>
> Acked-by: David Vrabel <[email protected]>
>
> I'm expecting this to go via the PCI tree.
>
> David
>
>> --- 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:
>>
>

2014-07-29 12:26:50

by ethan zhao

[permalink] [raw]
Subject: Re: [PATCH 2/4 v3] KVM: use pci device flag operation helper functions

This patch was already
Acked-by: Paolo Bonzini <[email protected]>
I forgot to add the Ack tag.

Thanks,
Ethan

Sorry for last post in HTML format with ipad.

On Tue, Jul 29, 2014 at 12:06 PM, Ethan Zhao <[email protected]> wrote:
> Use helper function instead of direct operation to pci device
> flag when set device to assigned or deassigned.
>
> Signed-off-by: Ethan Zhao <[email protected]>
> ---
> 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
>

2014-07-29 12:34:06

by ethan zhao

[permalink] [raw]
Subject: Re: [PATCH 3/4 v3] xen-pciback: use pci device flag operation helper function

On Tue, Jul 29, 2014 at 6:07 PM, David Vrabel <[email protected]> wrote:
> On 29/07/14 05:06, Ethan Zhao wrote:
>> Use pci device flag operation helper functions when set device
>> to assigned or deassigned state.
>>
>> Signed-off-by: Ethan Zhao <[email protected]>
>
> 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 <[email protected]>
>
> I'm expecting this to go via the PCI tree.

Seems Bjorn still holds his bullet for last shot :)
>
> David
>
>> --- 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:
>>
>