* [PATCH v3 0/4 resend] Introduce device assignment flag operation helper function
@ 2014-08-08 5:36 Ethan Zhao
2014-08-08 5:36 ` [PATCH 1/4] PCI: introduce helper functions for device flag operation Ethan Zhao
` (4 more replies)
0 siblings, 5 replies; 15+ messages in thread
From: Ethan Zhao @ 2014-08-08 5:36 UTC (permalink / raw)
To: bhelgaas, linux-pci, linux-kernel, gleb, pbonzini, kvm,
konrad.wilk, boris.ostrovsky, david.vrabel, xen-devel
Cc: alex.williamson, alexander.h.duyck, ethan.kernel
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
Resend for v3.16 building.
Thanks,
Ethan
---
Ethan Zhao (4):
PCI: introduce helper functions for device flag operation
KVM: use pci device flag operation helper functions
xen-pciback: use pci device flag operation helper function
PCI: use device flag operation helper function in iov.c
drivers/pci/iov.c | 2 +-
drivers/xen/xen-pciback/pci_stub.c | 4 ++--
include/linux/pci.h | 13 +++++++++++++
virt/kvm/assigned-dev.c | 2 +-
virt/kvm/iommu.c | 4 ++--
5 files changed, 19 insertions(+), 6 deletions(-)
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/4] PCI: introduce helper functions for device flag operation
2014-08-08 5:36 [PATCH v3 0/4 resend] Introduce device assignment flag operation helper function Ethan Zhao
@ 2014-08-08 5:36 ` Ethan Zhao
2014-08-08 5:36 ` [PATCH 2/4] KVM: use pci device flag operation helper functions Ethan Zhao
` (3 subsequent siblings)
4 siblings, 0 replies; 15+ messages in thread
From: Ethan Zhao @ 2014-08-08 5:36 UTC (permalink / raw)
To: bhelgaas, linux-pci, linux-kernel, gleb, pbonzini, kvm,
konrad.wilk, boris.ostrovsky, david.vrabel, xen-devel
Cc: alex.williamson, alexander.h.duyck, ethan.kernel
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 466bcd1..b610ab3 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1829,4 +1829,17 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,
*/
struct pci_dev *pci_find_upstream_pcie_bridge(struct pci_dev *pdev);
+/* 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;
+}
#endif /* LINUX_PCI_H */
--
1.7.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/4] KVM: use pci device flag operation helper functions
2014-08-08 5:36 [PATCH v3 0/4 resend] Introduce device assignment flag operation helper function Ethan Zhao
2014-08-08 5:36 ` [PATCH 1/4] PCI: introduce helper functions for device flag operation Ethan Zhao
@ 2014-08-08 5:36 ` Ethan Zhao
2014-08-08 5:36 ` [PATCH 3/4] xen-pciback: use pci device flag operation helper function Ethan Zhao
` (2 subsequent siblings)
4 siblings, 0 replies; 15+ messages in thread
From: Ethan Zhao @ 2014-08-08 5:36 UTC (permalink / raw)
To: bhelgaas, linux-pci, linux-kernel, gleb, pbonzini, kvm,
konrad.wilk, boris.ostrovsky, david.vrabel, xen-devel
Cc: alex.williamson, alexander.h.duyck, ethan.kernel
Use helper function instead of direct operation to pci device
flag when set device to assigned or deassigned.
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
---
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..38581ee 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..34a8b02 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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/4] xen-pciback: use pci device flag operation helper function
2014-08-08 5:36 [PATCH v3 0/4 resend] Introduce device assignment flag operation helper function Ethan Zhao
2014-08-08 5:36 ` [PATCH 1/4] PCI: introduce helper functions for device flag operation Ethan Zhao
2014-08-08 5:36 ` [PATCH 2/4] KVM: use pci device flag operation helper functions Ethan Zhao
@ 2014-08-08 5:36 ` Ethan Zhao
2014-08-08 5:36 ` [PATCH 4/4] PCI: use device flag operation helper function in iov.c Ethan Zhao
2014-09-05 17:15 ` [PATCH v3 0/4 resend] Introduce device assignment flag operation helper function Bjorn Helgaas
4 siblings, 0 replies; 15+ messages in thread
From: Ethan Zhao @ 2014-08-08 5:36 UTC (permalink / raw)
To: bhelgaas, linux-pci, linux-kernel, gleb, pbonzini, kvm,
konrad.wilk, boris.ostrovsky, david.vrabel, xen-devel
Cc: alex.williamson, alexander.h.duyck, ethan.kernel
Use pci device flag operation helper functions when set device
to assigned or deassigned state.
Acked-by: David Vrabel <david.vrabel@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
---
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 d57a173..e593921 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);
@@ -413,7 +413,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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/4] PCI: use device flag operation helper function in iov.c
2014-08-08 5:36 [PATCH v3 0/4 resend] Introduce device assignment flag operation helper function Ethan Zhao
` (2 preceding siblings ...)
2014-08-08 5:36 ` [PATCH 3/4] xen-pciback: use pci device flag operation helper function Ethan Zhao
@ 2014-08-08 5:36 ` Ethan Zhao
2014-09-05 17:15 ` [PATCH v3 0/4 resend] Introduce device assignment flag operation helper function Bjorn Helgaas
4 siblings, 0 replies; 15+ messages in thread
From: Ethan Zhao @ 2014-08-08 5:36 UTC (permalink / raw)
To: bhelgaas, linux-pci, linux-kernel, gleb, pbonzini, kvm,
konrad.wilk, boris.ostrovsky, david.vrabel, xen-devel
Cc: alex.williamson, alexander.h.duyck, ethan.kernel
Use device flag operation helper functions when check device
assignment status.
Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
---
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 cb6f247..4d109c0 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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3 0/4 resend] Introduce device assignment flag operation helper function
2014-08-08 5:36 [PATCH v3 0/4 resend] Introduce device assignment flag operation helper function Ethan Zhao
` (3 preceding siblings ...)
2014-08-08 5:36 ` [PATCH 4/4] PCI: use device flag operation helper function in iov.c Ethan Zhao
@ 2014-09-05 17:15 ` Bjorn Helgaas
2014-09-08 17:08 ` Bjorn Helgaas
4 siblings, 1 reply; 15+ messages in thread
From: Bjorn Helgaas @ 2014-09-05 17:15 UTC (permalink / raw)
To: Ethan Zhao
Cc: linux-pci, linux-kernel, gleb, pbonzini, kvm, konrad.wilk,
boris.ostrovsky, david.vrabel, xen-devel, alex.williamson,
alexander.h.duyck, ethan.kernel
On Fri, Aug 08, 2014 at 01:36:03PM +0800, Ethan Zhao wrote:
> 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
>
> Resend for v3.16 building.
>
> Thanks,
> Ethan
> ---
> Ethan Zhao (4):
> PCI: introduce helper functions for device flag operation
> KVM: use pci device flag operation helper functions
> xen-pciback: use pci device flag operation helper function
> PCI: use device flag operation helper function in iov.c
>
> drivers/pci/iov.c | 2 +-
> drivers/xen/xen-pciback/pci_stub.c | 4 ++--
> include/linux/pci.h | 13 +++++++++++++
> virt/kvm/assigned-dev.c | 2 +-
> virt/kvm/iommu.c | 4 ++--
> 5 files changed, 19 insertions(+), 6 deletions(-)
>
Applied to pci/virtualization for v3.18, thanks!
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 0/4 resend] Introduce device assignment flag operation helper function
2014-09-05 17:15 ` [PATCH v3 0/4 resend] Introduce device assignment flag operation helper function Bjorn Helgaas
@ 2014-09-08 17:08 ` Bjorn Helgaas
2014-09-09 1:48 ` ethan zhao
0 siblings, 1 reply; 15+ messages in thread
From: Bjorn Helgaas @ 2014-09-08 17:08 UTC (permalink / raw)
To: Ethan Zhao
Cc: linux-pci, linux-kernel, gleb, pbonzini, kvm, konrad.wilk,
boris.ostrovsky, david.vrabel, xen-devel, alex.williamson,
alexander.h.duyck, ethan.kernel
On Fri, Sep 05, 2014 at 11:15:22AM -0600, Bjorn Helgaas wrote:
> On Fri, Aug 08, 2014 at 01:36:03PM +0800, Ethan Zhao wrote:
> > 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
> >
> > Resend for v3.16 building.
> >
> > Thanks,
> > Ethan
> > ---
> > Ethan Zhao (4):
> > PCI: introduce helper functions for device flag operation
> > KVM: use pci device flag operation helper functions
> > xen-pciback: use pci device flag operation helper function
> > PCI: use device flag operation helper function in iov.c
> >
> > drivers/pci/iov.c | 2 +-
> > drivers/xen/xen-pciback/pci_stub.c | 4 ++--
> > include/linux/pci.h | 13 +++++++++++++
> > virt/kvm/assigned-dev.c | 2 +-
> > virt/kvm/iommu.c | 4 ++--
> > 5 files changed, 19 insertions(+), 6 deletions(-)
> >
>
> Applied to pci/virtualization for v3.18, thanks!
I dropped these because of build errors from the kbuild-test robot. Fix
and resend if you still want them.
Bjorn
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 0/4 resend] Introduce device assignment flag operation helper function
2014-09-08 17:08 ` Bjorn Helgaas
@ 2014-09-09 1:48 ` ethan zhao
0 siblings, 0 replies; 15+ messages in thread
From: ethan zhao @ 2014-09-09 1:48 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, linux-kernel, gleb, pbonzini, kvm, konrad.wilk,
boris.ostrovsky, david.vrabel, xen-devel, alex.williamson,
alexander.h.duyck, ethan.kernel
Bjorn,
Thanks, will fix the building issue for new version and resend.
Ethan
On 2014/9/9 1:08, Bjorn Helgaas wrote:
> On Fri, Sep 05, 2014 at 11:15:22AM -0600, Bjorn Helgaas wrote:
>> On Fri, Aug 08, 2014 at 01:36:03PM +0800, Ethan Zhao wrote:
>>> 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
>>>
>>> Resend for v3.16 building.
>>>
>>> Thanks,
>>> Ethan
>>> ---
>>> Ethan Zhao (4):
>>> PCI: introduce helper functions for device flag operation
>>> KVM: use pci device flag operation helper functions
>>> xen-pciback: use pci device flag operation helper function
>>> PCI: use device flag operation helper function in iov.c
>>>
>>> drivers/pci/iov.c | 2 +-
>>> drivers/xen/xen-pciback/pci_stub.c | 4 ++--
>>> include/linux/pci.h | 13 +++++++++++++
>>> virt/kvm/assigned-dev.c | 2 +-
>>> virt/kvm/iommu.c | 4 ++--
>>> 5 files changed, 19 insertions(+), 6 deletions(-)
>>>
>> Applied to pci/virtualization for v3.18, thanks!
> I dropped these because of build errors from the kbuild-test robot. Fix
> and resend if you still want them.
>
> Bjorn
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 0/4] Introduce device assignment flag operation helper function
@ 2014-07-22 16:19 Ethan Zhao
2014-07-22 16:19 ` [PATCH 1/4] PCI: introduce helper functions for device flag operation Ethan Zhao
0 siblings, 1 reply; 15+ messages in thread
From: Ethan Zhao @ 2014-07-22 16:19 UTC (permalink / raw)
To: bhelgaas, linux-pci, linux-kernel, gleb, pbonzini, kvm,
konrad.wilk, boris.ostrovsky, david.vrabel, xen-devel
Cc: alex.williamson, alexander.h.duyck, ethan.kernel
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.
Much thanks to suggestion from
alex.williamson@redhat.com,
david.vrabel@citrix.com,
alexander.h.duyck@intel.com
BR,
Ethan
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/4] PCI: introduce helper functions for device flag operation
2014-07-22 16:19 [PATCH 0/4] " Ethan Zhao
@ 2014-07-22 16:19 ` Ethan Zhao
2014-07-28 21:00 ` Alex Williamson
0 siblings, 1 reply; 15+ messages in thread
From: Ethan Zhao @ 2014-07-22 16:19 UTC (permalink / raw)
To: bhelgaas, linux-pci, linux-kernel, gleb, pbonzini, kvm,
konrad.wilk, boris.ostrovsky, david.vrabel, xen-devel
Cc: alex.williamson, alexander.h.duyck, ethan.kernel
This patch introduced three helper functions to hide direct
device flag operation.
void pci_set_dev_assigned(struct pci_dev *pdev);
void pci_set_dev_deassigned(struct pci_dev *pdev);
bool pci_is_dev_assigned(struct pci_dev *pdev);
Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
---
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_set_dev_deassigned(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 ? true : false;
+}
/* kmem_cache style wrapper around pci_alloc_consistent() */
#include <linux/pci-dma.h>
--
1.7.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] PCI: introduce helper functions for device flag operation
2014-07-22 16:19 ` [PATCH 1/4] PCI: introduce helper functions for device flag operation Ethan Zhao
@ 2014-07-28 21:00 ` Alex Williamson
2014-07-29 1:53 ` ethan zhao
0 siblings, 1 reply; 15+ messages in thread
From: Alex Williamson @ 2014-07-28 21:00 UTC (permalink / raw)
To: Ethan Zhao
Cc: bhelgaas, linux-pci, linux-kernel, gleb, pbonzini, kvm,
konrad.wilk, boris.ostrovsky, david.vrabel, xen-devel,
alexander.h.duyck, ethan.kernel
On Wed, 2014-07-23 at 00:19 +0800, Ethan Zhao wrote:
> This patch introduced three helper functions to hide direct
> device flag operation.
>
> void pci_set_dev_assigned(struct pci_dev *pdev);
> void pci_set_dev_deassigned(struct pci_dev *pdev);
> bool pci_is_dev_assigned(struct pci_dev *pdev);
>
> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
> ---
> 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_set_dev_deassigned(struct pci_dev *pdev)
> +{
> + pdev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
> +}
I think pci_clear_dev_assigned() would make more sense, we're not
setting a flag named DEASSIGNED.
> +static inline bool pci_is_dev_assigned(struct pci_dev *pdev)
> +{
> + return pdev->dev_flags & PCI_DEV_FLAGS_ASSIGNED ? true : false;
> +}
The ternary operation isn't necessary. Thanks,
Alex
> /* kmem_cache style wrapper around pci_alloc_consistent() */
>
> #include <linux/pci-dma.h>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] PCI: introduce helper functions for device flag operation
2014-07-28 21:00 ` Alex Williamson
@ 2014-07-29 1:53 ` ethan zhao
2014-07-29 2:31 ` Alex Williamson
0 siblings, 1 reply; 15+ messages in thread
From: ethan zhao @ 2014-07-29 1:53 UTC (permalink / raw)
To: Alex Williamson
Cc: bhelgaas, linux-pci, linux-kernel, gleb, pbonzini, kvm,
konrad.wilk, boris.ostrovsky, david.vrabel, xen-devel,
alexander.h.duyck, ethan.kernel
On 2014/7/29 5:00, Alex Williamson wrote:
> On Wed, 2014-07-23 at 00:19 +0800, Ethan Zhao wrote:
>> This patch introduced three helper functions to hide direct
>> device flag operation.
>>
>> void pci_set_dev_assigned(struct pci_dev *pdev);
>> void pci_set_dev_deassigned(struct pci_dev *pdev);
>> bool pci_is_dev_assigned(struct pci_dev *pdev);
>>
>> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
>> ---
>> 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_set_dev_deassigned(struct pci_dev *pdev)
>> +{
>> + pdev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
>> +}
> I think pci_clear_dev_assigned() would make more sense, we're not
> setting a flag named DEASSIGNED.
Though it is a flag operation now, may not later, we define it
because we want to hide the internal operation.
'set' to 'deassigned' status is enough. So I would like keep it.
>
>> +static inline bool pci_is_dev_assigned(struct pci_dev *pdev)
>> +{
>> + return pdev->dev_flags & PCI_DEV_FLAGS_ASSIGNED ? true : false;
>> +}
> The ternary operation isn't necessary. Thanks,
Yep,
return pdev->dev_flags & PCI_DEV_FLAGS_ASSIGNED
is enough.
Thanks,
Ethan
>
> Alex
>
>> /* kmem_cache style wrapper around pci_alloc_consistent() */
>>
>> #include <linux/pci-dma.h>
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] PCI: introduce helper functions for device flag operation
2014-07-29 1:53 ` ethan zhao
@ 2014-07-29 2:31 ` Alex Williamson
2014-07-29 2:43 ` ethan zhao
0 siblings, 1 reply; 15+ messages in thread
From: Alex Williamson @ 2014-07-29 2:31 UTC (permalink / raw)
To: ethan zhao
Cc: bhelgaas, linux-pci, linux-kernel, gleb, pbonzini, kvm,
konrad.wilk, boris.ostrovsky, david.vrabel, xen-devel,
alexander.h.duyck, ethan.kernel
On Tue, 2014-07-29 at 09:53 +0800, ethan zhao wrote:
> On 2014/7/29 5:00, Alex Williamson wrote:
> > On Wed, 2014-07-23 at 00:19 +0800, Ethan Zhao wrote:
> >> This patch introduced three helper functions to hide direct
> >> device flag operation.
> >>
> >> void pci_set_dev_assigned(struct pci_dev *pdev);
> >> void pci_set_dev_deassigned(struct pci_dev *pdev);
> >> bool pci_is_dev_assigned(struct pci_dev *pdev);
> >>
> >> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
> >> ---
> >> 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_set_dev_deassigned(struct pci_dev *pdev)
> >> +{
> >> + pdev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
> >> +}
> > I think pci_clear_dev_assigned() would make more sense, we're not
> > setting a flag named DEASSIGNED.
> Though it is a flag operation now, may not later, we define it
> because we want to hide the internal operation.
> 'set' to 'deassigned' status is enough. So I would like keep it.
I disagree, the opposite of a 'set' is a 'clear', or at least an
'unset'. Using bit-ops-like terminology doesn't lock us into an
implementation. As written, this could just as easily be setting two
different variables.
> >
> >> +static inline bool pci_is_dev_assigned(struct pci_dev *pdev)
> >> +{
> >> + return pdev->dev_flags & PCI_DEV_FLAGS_ASSIGNED ? true : false;
> >> +}
> > The ternary operation isn't necessary. Thanks,
> Yep,
>
> return pdev->dev_flags & PCI_DEV_FLAGS_ASSIGNED
>
> is enough.
>
> Thanks,
> Ethan
> >
> > Alex
> >
> >> /* kmem_cache style wrapper around pci_alloc_consistent() */
> >>
> >> #include <linux/pci-dma.h>
> >
> >
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] PCI: introduce helper functions for device flag operation
2014-07-29 2:31 ` Alex Williamson
@ 2014-07-29 2:43 ` ethan zhao
2014-07-29 3:37 ` Alexander Duyck
0 siblings, 1 reply; 15+ messages in thread
From: ethan zhao @ 2014-07-29 2:43 UTC (permalink / raw)
To: Alex Williamson
Cc: bhelgaas, linux-pci, linux-kernel, gleb, pbonzini, kvm,
konrad.wilk, boris.ostrovsky, david.vrabel, xen-devel,
alexander.h.duyck, ethan.kernel
On 2014/7/29 10:31, Alex Williamson wrote:
> On Tue, 2014-07-29 at 09:53 +0800, ethan zhao wrote:
>> On 2014/7/29 5:00, Alex Williamson wrote:
>>> On Wed, 2014-07-23 at 00:19 +0800, Ethan Zhao wrote:
>>>> This patch introduced three helper functions to hide direct
>>>> device flag operation.
>>>>
>>>> void pci_set_dev_assigned(struct pci_dev *pdev);
>>>> void pci_set_dev_deassigned(struct pci_dev *pdev);
>>>> bool pci_is_dev_assigned(struct pci_dev *pdev);
>>>>
>>>> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
>>>> ---
>>>> 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_set_dev_deassigned(struct pci_dev *pdev)
>>>> +{
>>>> + pdev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
>>>> +}
>>> I think pci_clear_dev_assigned() would make more sense, we're not
>>> setting a flag named DEASSIGNED.
>> Though it is a flag operation now, may not later, we define it
>> because we want to hide the internal operation.
>> 'set' to 'deassigned' status is enough. So I would like keep it.
> I disagree, the opposite of a 'set' is a 'clear', or at least an
> 'unset'. Using bit-ops-like terminology doesn't lock us into an
> implementation. As written, this could just as easily be setting two
> different variables.
So there are two pairs of opposite:
set assigned ---> unset assigned
set assigned ---> set deassigned
Here you prefer the 'verb' set /unset, and I prefer the 'adj.'
assigned / deassigned.
Do they really have different meaning or make confusion ? I don't
think so.
Thanks,
Ethan
>
>>>> +static inline bool pci_is_dev_assigned(struct pci_dev *pdev)
>>>> +{
>>>> + return pdev->dev_flags & PCI_DEV_FLAGS_ASSIGNED ? true : false;
>>>> +}
>>> The ternary operation isn't necessary. Thanks,
>> Yep,
>>
>> return pdev->dev_flags & PCI_DEV_FLAGS_ASSIGNED
>>
>> is enough.
>>
>> Thanks,
>> Ethan
>>> Alex
>>>
>>>> /* kmem_cache style wrapper around pci_alloc_consistent() */
>>>>
>>>> #include <linux/pci-dma.h>
>>>
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] PCI: introduce helper functions for device flag operation
2014-07-29 2:43 ` ethan zhao
@ 2014-07-29 3:37 ` Alexander Duyck
2014-07-29 3:47 ` Ethan Zhao
0 siblings, 1 reply; 15+ messages in thread
From: Alexander Duyck @ 2014-07-29 3:37 UTC (permalink / raw)
To: ethan zhao, Alex Williamson
Cc: bhelgaas, linux-pci, linux-kernel, gleb, pbonzini, kvm,
konrad.wilk, boris.ostrovsky, david.vrabel, xen-devel,
alexander.h.duyck, ethan.kernel
On 07/28/2014 07:43 PM, ethan zhao wrote:
>
> On 2014/7/29 10:31, Alex Williamson wrote:
>> On Tue, 2014-07-29 at 09:53 +0800, ethan zhao wrote:
>>> On 2014/7/29 5:00, Alex Williamson wrote:
>>>> On Wed, 2014-07-23 at 00:19 +0800, Ethan Zhao wrote:
>>>>> This patch introduced three helper functions to hide direct
>>>>> device flag operation.
>>>>>
>>>>> void pci_set_dev_assigned(struct pci_dev *pdev);
>>>>> void pci_set_dev_deassigned(struct pci_dev *pdev);
>>>>> bool pci_is_dev_assigned(struct pci_dev *pdev);
>>>>>
>>>>> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
>>>>> ---
>>>>> 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_set_dev_deassigned(struct pci_dev *pdev)
>>>>> +{
>>>>> + pdev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
>>>>> +}
>>>> I think pci_clear_dev_assigned() would make more sense, we're not
>>>> setting a flag named DEASSIGNED.
>>> Though it is a flag operation now, may not later, we define it
>>> because we want to hide the internal operation.
>>> 'set' to 'deassigned' status is enough. So I would like keep it.
>> I disagree, the opposite of a 'set' is a 'clear', or at least an
>> 'unset'. Using bit-ops-like terminology doesn't lock us into an
>> implementation. As written, this could just as easily be setting two
>> different variables.
> So there are two pairs of opposite:
>
> set assigned ---> unset assigned
> set assigned ---> set deassigned
>
> Here you prefer the 'verb' set /unset, and I prefer the 'adj.' assigned
> / deassigned.
>
> Do they really have different meaning or make confusion ? I don't think
> so.
>
> Thanks,
> Ethan
>
I agree with Alex W. If you are going to use the "set" name you should
probably use "clear" for the operation that undoes it. Using the term
set implies that it is setting a bit and we currently don't have a
deassigned/unassigned bit.
If that doesn't work perhaps you could use something like
get/put_assigned_device or acquire/release_assigned_device. You just
need to describe what it is you are doing. You could even consider
adding some tests to return an error if you attempt to assign a device
that is already assigned.
Thanks,
Alex D
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] PCI: introduce helper functions for device flag operation
2014-07-29 3:37 ` Alexander Duyck
@ 2014-07-29 3:47 ` Ethan Zhao
0 siblings, 0 replies; 15+ messages in thread
From: Ethan Zhao @ 2014-07-29 3:47 UTC (permalink / raw)
To: Alexander Duyck
Cc: ethan zhao, Alex Williamson, Bjorn Helgaas, linux-pci, LKML,
<gleb@kernel.org>, <pbonzini@redhat.com>,
<kvm@vger.kernel.org>,
Konrad Rzeszutek Wilk, <boris.ostrovsky@oracle.com>,
David Vrabel, xen-devel, <alexander.h.duyck@intel.com>
Both of you and Alex W prefer the 'Verb' , Ok, I accept the suggestion.
Thanks,
Ethan
On Tue, Jul 29, 2014 at 11:37 AM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On 07/28/2014 07:43 PM, ethan zhao wrote:
>>
>> On 2014/7/29 10:31, Alex Williamson wrote:
>>> On Tue, 2014-07-29 at 09:53 +0800, ethan zhao wrote:
>>>> On 2014/7/29 5:00, Alex Williamson wrote:
>>>>> On Wed, 2014-07-23 at 00:19 +0800, Ethan Zhao wrote:
>>>>>> This patch introduced three helper functions to hide direct
>>>>>> device flag operation.
>>>>>>
>>>>>> void pci_set_dev_assigned(struct pci_dev *pdev);
>>>>>> void pci_set_dev_deassigned(struct pci_dev *pdev);
>>>>>> bool pci_is_dev_assigned(struct pci_dev *pdev);
>>>>>>
>>>>>> Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
>>>>>> ---
>>>>>> 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_set_dev_deassigned(struct pci_dev *pdev)
>>>>>> +{
>>>>>> + pdev->dev_flags &= ~PCI_DEV_FLAGS_ASSIGNED;
>>>>>> +}
>>>>> I think pci_clear_dev_assigned() would make more sense, we're not
>>>>> setting a flag named DEASSIGNED.
>>>> Though it is a flag operation now, may not later, we define it
>>>> because we want to hide the internal operation.
>>>> 'set' to 'deassigned' status is enough. So I would like keep it.
>>> I disagree, the opposite of a 'set' is a 'clear', or at least an
>>> 'unset'. Using bit-ops-like terminology doesn't lock us into an
>>> implementation. As written, this could just as easily be setting two
>>> different variables.
>> So there are two pairs of opposite:
>>
>> set assigned ---> unset assigned
>> set assigned ---> set deassigned
>>
>> Here you prefer the 'verb' set /unset, and I prefer the 'adj.' assigned
>> / deassigned.
>>
>> Do they really have different meaning or make confusion ? I don't think
>> so.
>>
>> Thanks,
>> Ethan
>>
>
> I agree with Alex W. If you are going to use the "set" name you should
> probably use "clear" for the operation that undoes it. Using the term
> set implies that it is setting a bit and we currently don't have a
> deassigned/unassigned bit.
>
> If that doesn't work perhaps you could use something like
> get/put_assigned_device or acquire/release_assigned_device. You just
> need to describe what it is you are doing. You could even consider
> adding some tests to return an error if you attempt to assign a device
> that is already assigned.
>
> Thanks,
>
> Alex D
>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2014-09-09 1:48 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-08 5:36 [PATCH v3 0/4 resend] Introduce device assignment flag operation helper function Ethan Zhao
2014-08-08 5:36 ` [PATCH 1/4] PCI: introduce helper functions for device flag operation Ethan Zhao
2014-08-08 5:36 ` [PATCH 2/4] KVM: use pci device flag operation helper functions Ethan Zhao
2014-08-08 5:36 ` [PATCH 3/4] xen-pciback: use pci device flag operation helper function Ethan Zhao
2014-08-08 5:36 ` [PATCH 4/4] PCI: use device flag operation helper function in iov.c Ethan Zhao
2014-09-05 17:15 ` [PATCH v3 0/4 resend] Introduce device assignment flag operation helper function Bjorn Helgaas
2014-09-08 17:08 ` Bjorn Helgaas
2014-09-09 1:48 ` ethan zhao
-- strict thread matches above, loose matches on Subject: below --
2014-07-22 16:19 [PATCH 0/4] " Ethan Zhao
2014-07-22 16:19 ` [PATCH 1/4] PCI: introduce helper functions for device flag operation Ethan Zhao
2014-07-28 21:00 ` Alex Williamson
2014-07-29 1:53 ` ethan zhao
2014-07-29 2:31 ` Alex Williamson
2014-07-29 2:43 ` ethan zhao
2014-07-29 3:37 ` Alexander Duyck
2014-07-29 3:47 ` Ethan Zhao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).