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

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

[Ethan Zhao]

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
alex.williamson@redhat.com,
david.vrabel@citrix.com,
alexander.h.duyck@intel.com


BR,
Ethan

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

[Ethan Zhao]

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

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

[Ethan Zhao]

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

Signed-off-by: Ethan Zhao <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

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

[Ethan Zhao]

This patch was already
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
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 <ethan.zhao@oracle.com> wrote:
> Use helper function instead of direct operation to pci device
> flag when set device to assigned or deassigned.
>
> Signed-off-by: Ethan Zhao <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
>

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

[Ethan Zhao]

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

Signed-off-by: Ethan Zhao <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:
-- 
1.7.1

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

[David Vrabel]

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 <ethan.zhao@oracle.com>

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 <david.vrabel@citrix.com>

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

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

[Ethan Zhao]

> 在 2014年7月29日，下午6:07，David Vrabel <david.vrabel@citrix.com> 写道：
> 
>> 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 <ethan.zhao@oracle.com>
> 
> 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 <david.vrabel@citrix.com>
> 
> 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:
>> 
> 

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

[Ethan Zhao]

On Tue, Jul 29, 2014 at 6:07 PM, David Vrabel <david.vrabel@citrix.com> 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 <ethan.zhao@oracle.com>
>
> 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 <david.vrabel@citrix.com>
>
> 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:
>>
>

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

[Ethan Zhao]

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

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