DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] pci/linux: copy new id for inserted device
@ 2020-08-21 22:06 Jim Harris
  2020-08-24 21:10 ` [dpdk-dev] [PATCH v2] " Jim Harris
  0 siblings, 1 reply; 10+ messages in thread
From: Jim Harris @ 2020-08-21 22:06 UTC (permalink / raw)
  To: dev; +Cc: Jim Harris

When a device is inserted into an existing BDF slot
that has not been probed, we must overwrite the old
PCI ID with the ID of the new function. Otherwise
we may not probe the function with the correct driver,
if at all.

Signed-off-by: Jim Harris <james.r.harris@intel.com>
---
 drivers/bus/pci/linux/pci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index a2198abf4..4701661e5 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -352,6 +352,7 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 				if (!rte_dev_is_probed(&dev2->device)) {
 					dev2->kdrv = dev->kdrv;
 					dev2->max_vfs = dev->max_vfs;
+					memcpy(&dev2->id, &dev->id, sizeof(dev2->id));
 					pci_name_set(dev2);
 					memmove(dev2->mem_resource,
 						dev->mem_resource,
@@ -365,7 +366,8 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 					 * need to do anything here unless...
 					 **/
 					if (dev2->kdrv != dev->kdrv ||
-						dev2->max_vfs != dev->max_vfs)
+						dev2->max_vfs != dev->max_vfs ||
+						memcmp(&dev2->id, &dev->id, sizeof(dev2->id))
 						/*
 						 * This should not happens.
 						 * But it is still possible if
-- 
2.20.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [dpdk-dev] [PATCH v2] pci/linux: copy new id for inserted device
  2020-08-21 22:06 [dpdk-dev] [PATCH] pci/linux: copy new id for inserted device Jim Harris
@ 2020-08-24 21:10 ` Jim Harris
  2020-10-07 15:05   ` Thomas Monjalon
  2020-10-12 22:01   ` [dpdk-dev] [PATCH v3] " Jim Harris
  0 siblings, 2 replies; 10+ messages in thread
From: Jim Harris @ 2020-08-24 21:10 UTC (permalink / raw)
  To: dev; +Cc: Jim Harris

When a device is inserted into an existing BDF slot
that has not been probed, we must overwrite the old
PCI ID with the ID of the new function. Otherwise
we may not probe the function with the correct driver,
if at all.

Signed-off-by: Jim Harris <james.r.harris@intel.com>
---
 drivers/bus/pci/linux/pci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index a2198abf4..d8fd973b2 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -352,6 +352,7 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 				if (!rte_dev_is_probed(&dev2->device)) {
 					dev2->kdrv = dev->kdrv;
 					dev2->max_vfs = dev->max_vfs;
+					memcpy(&dev2->id, &dev->id, sizeof(dev2->id));
 					pci_name_set(dev2);
 					memmove(dev2->mem_resource,
 						dev->mem_resource,
@@ -365,7 +366,8 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 					 * need to do anything here unless...
 					 **/
 					if (dev2->kdrv != dev->kdrv ||
-						dev2->max_vfs != dev->max_vfs)
+						dev2->max_vfs != dev->max_vfs ||
+						memcmp(&dev2->id, &dev->id, sizeof(dev2->id)))
 						/*
 						 * This should not happens.
 						 * But it is still possible if
-- 
2.20.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [PATCH v2] pci/linux: copy new id for inserted device
  2020-08-24 21:10 ` [dpdk-dev] [PATCH v2] " Jim Harris
@ 2020-10-07 15:05   ` Thomas Monjalon
  2020-10-12 20:58     ` Harris, James R
  2020-10-12 22:01   ` [dpdk-dev] [PATCH v3] " Jim Harris
  1 sibling, 1 reply; 10+ messages in thread
From: Thomas Monjalon @ 2020-10-07 15:05 UTC (permalink / raw)
  To: Jim Harris; +Cc: dev, david.marchand, bruce.richardson, ferruh.yigit

Hi Jim,

Sorry I see nobody reviewed your patch.

Jim Harris <james.r.harris@intel.com> wrote:
> + memcpy(&dev2->id, &dev->id, sizeof(dev2->id));
[...]
> + memcmp(&dev2->id, &dev->id, sizeof(dev2->id)))

Why using memcpy and memcmp instead of simple assignment and comparison?



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [PATCH v2] pci/linux: copy new id for inserted device
  2020-10-07 15:05   ` Thomas Monjalon
@ 2020-10-12 20:58     ` Harris, James R
  2020-10-12 21:54       ` Thomas Monjalon
  0 siblings, 1 reply; 10+ messages in thread
From: Harris, James R @ 2020-10-12 20:58 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, david.marchand, Richardson, Bruce, Yigit, Ferruh



On 10/7/20, 8:06 AM, "Thomas Monjalon" <thomas@monjalon.net> wrote:

    Hi Jim,

    Sorry I see nobody reviewed your patch.

    Jim Harris <james.r.harris@intel.com> wrote:
    > + memcpy(&dev2->id, &dev->id, sizeof(dev2->id));
    [...]
    > + memcmp(&dev2->id, &dev->id, sizeof(dev2->id)))

    Why using memcpy and memcmp instead of simple assignment and comparison?

Direct assignment and comparison would work too.  I did see some similar cases though using memcpy for rte_pci_addr (which is similar to rte_pci_id) in linux/pci_uio.c and linux/pci_vfio.c.  It wasn't clear to me if direct assignment/comparison for structures was the norm for DPDK.

I'm happy to send a v2 with a direct assignment/comparison though if that is preferred.

-Jim




^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [PATCH v2] pci/linux: copy new id for inserted device
  2020-10-12 20:58     ` Harris, James R
@ 2020-10-12 21:54       ` Thomas Monjalon
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Monjalon @ 2020-10-12 21:54 UTC (permalink / raw)
  To: Harris, James R; +Cc: dev, david.marchand, Richardson, Bruce, Yigit, Ferruh

12/10/2020 22:58, Harris, James R:
> 
> On 10/7/20, 8:06 AM, "Thomas Monjalon" <thomas@monjalon.net> wrote:
> 
>     Hi Jim,
> 
>     Sorry I see nobody reviewed your patch.
> 
>     Jim Harris <james.r.harris@intel.com> wrote:
>     > + memcpy(&dev2->id, &dev->id, sizeof(dev2->id));
>     [...]
>     > + memcmp(&dev2->id, &dev->id, sizeof(dev2->id)))
> 
>     Why using memcpy and memcmp instead of simple assignment and comparison?
> 
> Direct assignment and comparison would work too.  I did see some similar cases though using memcpy for rte_pci_addr (which is similar to rte_pci_id) in linux/pci_uio.c and linux/pci_vfio.c.  It wasn't clear to me if direct assignment/comparison for structures was the norm for DPDK.
> 
> I'm happy to send a v2 with a direct assignment/comparison though if that is preferred.

Yes please, direct assignment is preferred.
Thanks



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [dpdk-dev] [PATCH v3] pci/linux: copy new id for inserted device
  2020-08-24 21:10 ` [dpdk-dev] [PATCH v2] " Jim Harris
  2020-10-07 15:05   ` Thomas Monjalon
@ 2020-10-12 22:01   ` Jim Harris
  2020-10-13  8:40     ` Thomas Monjalon
                       ` (2 more replies)
  1 sibling, 3 replies; 10+ messages in thread
From: Jim Harris @ 2020-10-12 22:01 UTC (permalink / raw)
  To: dev, thomas; +Cc: Jim Harris

When a device is inserted into an existing BDF slot
that has not been probed, we must overwrite the old
PCI ID with the ID of the new function. Otherwise
we may not probe the function with the correct driver,
if at all.

Signed-off-by: Jim Harris <james.r.harris@intel.com>
---
 drivers/bus/pci/linux/pci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index a2198abf4..2e37735c2 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -352,6 +352,7 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 				if (!rte_dev_is_probed(&dev2->device)) {
 					dev2->kdrv = dev->kdrv;
 					dev2->max_vfs = dev->max_vfs;
+					dev2->id = dev->id;
 					pci_name_set(dev2);
 					memmove(dev2->mem_resource,
 						dev->mem_resource,
@@ -365,7 +366,8 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 					 * need to do anything here unless...
 					 **/
 					if (dev2->kdrv != dev->kdrv ||
-						dev2->max_vfs != dev->max_vfs)
+						dev2->max_vfs != dev->max_vfs ||
+						dev2->id != dev->id)
 						/*
 						 * This should not happens.
 						 * But it is still possible if
-- 
2.20.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [PATCH v3] pci/linux: copy new id for inserted device
  2020-10-12 22:01   ` [dpdk-dev] [PATCH v3] " Jim Harris
@ 2020-10-13  8:40     ` Thomas Monjalon
  2020-10-13 14:06     ` [dpdk-dev] [PATCH v4] " Jim Harris
  2020-10-13 17:03     ` [dpdk-dev] [PATCH v5] " Jim Harris
  2 siblings, 0 replies; 10+ messages in thread
From: Thomas Monjalon @ 2020-10-13  8:40 UTC (permalink / raw)
  To: Jim Harris; +Cc: dev

13/10/2020 00:01, Jim Harris:
> When a device is inserted into an existing BDF slot
> that has not been probed, we must overwrite the old
> PCI ID with the ID of the new function. Otherwise
> we may not probe the function with the correct driver,
> if at all.
> 
> Signed-off-by: Jim Harris <james.r.harris@intel.com>
> ---
> --- a/drivers/bus/pci/linux/pci.c
> +++ b/drivers/bus/pci/linux/pci.c
> @@ -352,6 +352,7 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
>  				if (!rte_dev_is_probed(&dev2->device)) {
>  					dev2->kdrv = dev->kdrv;
>  					dev2->max_vfs = dev->max_vfs;
> +					dev2->id = dev->id;

This simple assignment looks nicer than memcpy.
Thanks for this change.

[...]
>  					if (dev2->kdrv != dev->kdrv ||
> -						dev2->max_vfs != dev->max_vfs)
> +						dev2->max_vfs != dev->max_vfs ||
> +						dev2->id != dev->id)

Unfortunately comparison of structs is not supported.
It seems your original memcmp is right.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [dpdk-dev] [PATCH v4] pci/linux: copy new id for inserted device
  2020-10-12 22:01   ` [dpdk-dev] [PATCH v3] " Jim Harris
  2020-10-13  8:40     ` Thomas Monjalon
@ 2020-10-13 14:06     ` Jim Harris
  2020-10-13 17:03     ` [dpdk-dev] [PATCH v5] " Jim Harris
  2 siblings, 0 replies; 10+ messages in thread
From: Jim Harris @ 2020-10-13 14:06 UTC (permalink / raw)
  To: dev, thomas; +Cc: Jim Harris

When a device is inserted into an existing BDF slot
that has not been probed, we must overwrite the old
PCI ID with the ID of the new function. Otherwise
we may not probe the function with the correct driver,
if at all.

Signed-off-by: Jim Harris <james.r.harris@intel.com>
---
 drivers/bus/pci/linux/pci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index a2198abf4..86e920d1c 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -352,6 +352,7 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 				if (!rte_dev_is_probed(&dev2->device)) {
 					dev2->kdrv = dev->kdrv;
 					dev2->max_vfs = dev->max_vfs;
+					dev2->id = dev->id;
 					pci_name_set(dev2);
 					memmove(dev2->mem_resource,
 						dev->mem_resource,
@@ -365,7 +366,8 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 					 * need to do anything here unless...
 					 **/
 					if (dev2->kdrv != dev->kdrv ||
-						dev2->max_vfs != dev->max_vfs)
+						dev2->max_vfs != dev->max_vfs ||
+						memcmp(&dev2->id, &dev->id))
 						/*
 						 * This should not happens.
 						 * But it is still possible if
-- 
2.20.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [dpdk-dev] [PATCH v5] pci/linux: copy new id for inserted device
  2020-10-12 22:01   ` [dpdk-dev] [PATCH v3] " Jim Harris
  2020-10-13  8:40     ` Thomas Monjalon
  2020-10-13 14:06     ` [dpdk-dev] [PATCH v4] " Jim Harris
@ 2020-10-13 17:03     ` Jim Harris
  2020-10-13 21:02       ` Thomas Monjalon
  2 siblings, 1 reply; 10+ messages in thread
From: Jim Harris @ 2020-10-13 17:03 UTC (permalink / raw)
  To: dev, thomas; +Cc: Jim Harris

When a device is inserted into an existing BDF slot
that has not been probed, we must overwrite the old
PCI ID with the ID of the new function. Otherwise
we may not probe the function with the correct driver,
if at all.

Signed-off-by: Jim Harris <james.r.harris@intel.com>
---
 drivers/bus/pci/linux/pci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index a2198abf4..e51f9d756 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -352,6 +352,7 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 				if (!rte_dev_is_probed(&dev2->device)) {
 					dev2->kdrv = dev->kdrv;
 					dev2->max_vfs = dev->max_vfs;
+					dev2->id = dev->id;
 					pci_name_set(dev2);
 					memmove(dev2->mem_resource,
 						dev->mem_resource,
@@ -365,7 +366,8 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 					 * need to do anything here unless...
 					 **/
 					if (dev2->kdrv != dev->kdrv ||
-						dev2->max_vfs != dev->max_vfs)
+						dev2->max_vfs != dev->max_vfs ||
+						memcmp(&dev2->id, &dev->id, sizeof(dev2->id)))
 						/*
 						 * This should not happens.
 						 * But it is still possible if
-- 
2.20.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [PATCH v5] pci/linux: copy new id for inserted device
  2020-10-13 17:03     ` [dpdk-dev] [PATCH v5] " Jim Harris
@ 2020-10-13 21:02       ` Thomas Monjalon
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Monjalon @ 2020-10-13 21:02 UTC (permalink / raw)
  To: Jim Harris; +Cc: dev

13/10/2020 19:03, Jim Harris:
> When a device is inserted into an existing BDF slot
> that has not been probed, we must overwrite the old
> PCI ID with the ID of the new function. Otherwise
> we may not probe the function with the correct driver,
> if at all.
> 
> Signed-off-by: Jim Harris <james.r.harris@intel.com>

Applied, thanks



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-10-13 21:02 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-21 22:06 [dpdk-dev] [PATCH] pci/linux: copy new id for inserted device Jim Harris
2020-08-24 21:10 ` [dpdk-dev] [PATCH v2] " Jim Harris
2020-10-07 15:05   ` Thomas Monjalon
2020-10-12 20:58     ` Harris, James R
2020-10-12 21:54       ` Thomas Monjalon
2020-10-12 22:01   ` [dpdk-dev] [PATCH v3] " Jim Harris
2020-10-13  8:40     ` Thomas Monjalon
2020-10-13 14:06     ` [dpdk-dev] [PATCH v4] " Jim Harris
2020-10-13 17:03     ` [dpdk-dev] [PATCH v5] " Jim Harris
2020-10-13 21:02       ` Thomas Monjalon

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).