* [dpdk-dev] [PATCH 0/2] baseband: fix segfault in Intel drivers @ 2020-10-06 10:04 Maxime Coquelin 2020-10-06 10:04 ` [dpdk-dev] [PATCH 1/2] baseband/fpga_5gnr_fec: fix segfaults with debug Maxime Coquelin ` (3 more replies) 0 siblings, 4 replies; 8+ messages in thread From: Maxime Coquelin @ 2020-10-06 10:04 UTC (permalink / raw) To: dev, stable, nicolas.chautru, trix; +Cc: Maxime Coquelin This series fixes segfaults in fpga_5gnr_fec and fpga_lte_fec drivers when bbdev debug is enabled. Maxime Coquelin (2): baseband/fpga_5gnr_fec: fix segfaults with debug baseband/fpga_lte_fec: fix segfaults with debug drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 4 ++-- drivers/baseband/fpga_lte_fec/fpga_lte_fec.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) -- 2.26.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [dpdk-dev] [PATCH 1/2] baseband/fpga_5gnr_fec: fix segfaults with debug 2020-10-06 10:04 [dpdk-dev] [PATCH 0/2] baseband: fix segfault in Intel drivers Maxime Coquelin @ 2020-10-06 10:04 ` Maxime Coquelin 2020-10-06 16:14 ` Chautru, Nicolas 2020-10-06 10:04 ` [dpdk-dev] [PATCH 2/2] baseband/fpga_lte_fec: " Maxime Coquelin ` (2 subsequent siblings) 3 siblings, 1 reply; 8+ messages in thread From: Maxime Coquelin @ 2020-10-06 10:04 UTC (permalink / raw) To: dev, stable, nicolas.chautru, trix; +Cc: Maxime Coquelin When RTE_LIBRTE_BBDEV_DEBUG is enabled, rte_device's driver pointer is dereferenced twice in fpga_5gnr_fec's probe callback. It causes a segmentation fault because this pointer is only assigned after probe callback call. This patch makes use of rte_pci_driver pointer instead. Fixes: 0b5927cbcba7 ("baseband/fpga_5gnr_fec: add PMD for FPGA 5GNR FEC") Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> --- drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c index 61f9c04ba2..11ee4d3e10 100644 --- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c +++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c @@ -1839,7 +1839,7 @@ fpga_5gnr_fec_init(struct rte_bbdev *dev, struct rte_pci_driver *drv) rte_bbdev_log_debug( "Init device %s [%s] @ virtaddr %p phyaddr %#"PRIx64, - dev->device->driver->name, dev->data->name, + drv->driver.name, dev->data->name, (void *)pci_dev->mem_resource[0].addr, pci_dev->mem_resource[0].phys_addr); } @@ -1895,7 +1895,7 @@ fpga_5gnr_fec_probe(struct rte_pci_driver *pci_drv, ((uint16_t)(version_id >> 16)), ((uint16_t)version_id)); #ifdef RTE_LIBRTE_BBDEV_DEBUG - if (!strcmp(bbdev->device->driver->name, + if (!strcmp(pci_drv->driver.name, RTE_STR(FPGA_5GNR_FEC_PF_DRIVER_NAME))) print_static_reg_debug_info(d->mmio_base); #endif -- 2.26.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] baseband/fpga_5gnr_fec: fix segfaults with debug 2020-10-06 10:04 ` [dpdk-dev] [PATCH 1/2] baseband/fpga_5gnr_fec: fix segfaults with debug Maxime Coquelin @ 2020-10-06 16:14 ` Chautru, Nicolas 2020-10-06 16:34 ` Maxime Coquelin 0 siblings, 1 reply; 8+ messages in thread From: Chautru, Nicolas @ 2020-10-06 16:14 UTC (permalink / raw) To: Maxime Coquelin, dev, stable, trix Thanks Maxime I am not totally sure that this actually got broken in the very commit you point to (I think that there was another pci generic commit which changed the assumption when this pointer was set), but it doesn't harm to change anyway for stable build. Note this is already like this in the new PMD acc100. Acked-By: Nicolas Chautru <nicolas.chautru@intel.com> > -----Original Message----- > From: Maxime Coquelin <maxime.coquelin@redhat.com> > Sent: Tuesday, October 6, 2020 3:04 AM > To: dev@dpdk.org; stable@dpdk.org; Chautru, Nicolas > <nicolas.chautru@intel.com>; trix@redhat.com > Cc: Maxime Coquelin <maxime.coquelin@redhat.com> > Subject: [PATCH 1/2] baseband/fpga_5gnr_fec: fix segfaults with debug > > When RTE_LIBRTE_BBDEV_DEBUG is enabled, rte_device's driver pointer is > dereferenced twice in fpga_5gnr_fec's probe callback. > It causes a segmentation fault because this pointer is only assigned after probe > callback call. > > This patch makes use of rte_pci_driver pointer instead. > > Fixes: 0b5927cbcba7 ("baseband/fpga_5gnr_fec: add PMD for FPGA 5GNR > FEC") > > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> > --- > drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c > b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c > index 61f9c04ba2..11ee4d3e10 100644 > --- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c > +++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c > @@ -1839,7 +1839,7 @@ fpga_5gnr_fec_init(struct rte_bbdev *dev, struct > rte_pci_driver *drv) > > rte_bbdev_log_debug( > "Init device %s [%s] @ virtaddr %p phyaddr %#"PRIx64, > - dev->device->driver->name, dev->data->name, > + drv->driver.name, dev->data->name, > (void *)pci_dev->mem_resource[0].addr, > pci_dev->mem_resource[0].phys_addr); > } > @@ -1895,7 +1895,7 @@ fpga_5gnr_fec_probe(struct rte_pci_driver *pci_drv, > ((uint16_t)(version_id >> 16)), ((uint16_t)version_id)); > > #ifdef RTE_LIBRTE_BBDEV_DEBUG > - if (!strcmp(bbdev->device->driver->name, > + if (!strcmp(pci_drv->driver.name, Why do you have to change this one? Acked-by: Nicolas Chautru <nicolas.chautru@intel.com> > RTE_STR(FPGA_5GNR_FEC_PF_DRIVER_NAME))) > print_static_reg_debug_info(d->mmio_base); > #endif > -- > 2.26.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] baseband/fpga_5gnr_fec: fix segfaults with debug 2020-10-06 16:14 ` Chautru, Nicolas @ 2020-10-06 16:34 ` Maxime Coquelin 2020-10-06 17:41 ` Chautru, Nicolas 0 siblings, 1 reply; 8+ messages in thread From: Maxime Coquelin @ 2020-10-06 16:34 UTC (permalink / raw) To: Chautru, Nicolas, dev, stable, trix Hi Nicolas, On 10/6/20 6:14 PM, Chautru, Nicolas wrote: > Thanks Maxime > I am not totally sure that this actually got broken in the very commit you point to (I think that there was another pci generic commit which changed the assumption when this pointer was set), but it doesn't harm to change anyway for stable build. > Note this is already like this in the new PMD acc100. For fpga_5gnr_fec, we reproduced the issue with v20.05, which was the version the driver was introduced. For fpga_lte_fec, it seems to be the same. The driver was in introduced in v19.08, and dev->device.driver was also assigned after driver's probe callback is called: http://code.dpdk.org/dpdk/v19.08/source/drivers/bus/pci/pci_common.c#L212 Thanks, Maxime > Acked-By: Nicolas Chautru <nicolas.chautru@intel.com> > >> -----Original Message----- >> From: Maxime Coquelin <maxime.coquelin@redhat.com> >> Sent: Tuesday, October 6, 2020 3:04 AM >> To: dev@dpdk.org; stable@dpdk.org; Chautru, Nicolas >> <nicolas.chautru@intel.com>; trix@redhat.com >> Cc: Maxime Coquelin <maxime.coquelin@redhat.com> >> Subject: [PATCH 1/2] baseband/fpga_5gnr_fec: fix segfaults with debug >> >> When RTE_LIBRTE_BBDEV_DEBUG is enabled, rte_device's driver pointer is >> dereferenced twice in fpga_5gnr_fec's probe callback. >> It causes a segmentation fault because this pointer is only assigned after probe >> callback call. >> >> This patch makes use of rte_pci_driver pointer instead. >> >> Fixes: 0b5927cbcba7 ("baseband/fpga_5gnr_fec: add PMD for FPGA 5GNR >> FEC") >> >> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> >> --- >> drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c >> b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c >> index 61f9c04ba2..11ee4d3e10 100644 >> --- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c >> +++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c >> @@ -1839,7 +1839,7 @@ fpga_5gnr_fec_init(struct rte_bbdev *dev, struct >> rte_pci_driver *drv) >> >> rte_bbdev_log_debug( >> "Init device %s [%s] @ virtaddr %p phyaddr %#"PRIx64, >> - dev->device->driver->name, dev->data->name, >> + drv->driver.name, dev->data->name, >> (void *)pci_dev->mem_resource[0].addr, >> pci_dev->mem_resource[0].phys_addr); >> } >> @@ -1895,7 +1895,7 @@ fpga_5gnr_fec_probe(struct rte_pci_driver *pci_drv, >> ((uint16_t)(version_id >> 16)), ((uint16_t)version_id)); >> >> #ifdef RTE_LIBRTE_BBDEV_DEBUG >> - if (!strcmp(bbdev->device->driver->name, >> + if (!strcmp(pci_drv->driver.name, > > Why do you have to change this one? > > Acked-by: Nicolas Chautru <nicolas.chautru@intel.com> > >> RTE_STR(FPGA_5GNR_FEC_PF_DRIVER_NAME))) >> print_static_reg_debug_info(d->mmio_base); >> #endif >> -- >> 2.26.2 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] baseband/fpga_5gnr_fec: fix segfaults with debug 2020-10-06 16:34 ` Maxime Coquelin @ 2020-10-06 17:41 ` Chautru, Nicolas 0 siblings, 0 replies; 8+ messages in thread From: Chautru, Nicolas @ 2020-10-06 17:41 UTC (permalink / raw) To: Maxime Coquelin, dev, stable, trix Hi Maxime, > -----Original Message----- > From: Maxime Coquelin <maxime.coquelin@redhat.com> > Sent: Tuesday, October 6, 2020 9:34 AM > To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org; > stable@dpdk.org; trix@redhat.com > Subject: Re: [PATCH 1/2] baseband/fpga_5gnr_fec: fix segfaults with debug > > Hi Nicolas, > > On 10/6/20 6:14 PM, Chautru, Nicolas wrote: > > Thanks Maxime > > I am not totally sure that this actually got broken in the very commit you > point to (I think that there was another pci generic commit which changed > the assumption when this pointer was set), but it doesn't harm to change > anyway for stable build. > > Note this is already like this in the new PMD acc100. > > For fpga_5gnr_fec, we reproduced the issue with v20.05, which was the > version the driver was introduced. > > For fpga_lte_fec, it seems to be the same. The driver was in introduced in > v19.08, and dev->device.driver was also assigned after driver's probe > callback is called: > > http://code.dpdk.org/dpdk/v19.08/source/drivers/bus/pci/pci_common.c#L > 212 OK Thanks you are right. The change moving the driver assignment to end of probing was done in parallel prior to the formal upstreaming of these drivers. > > Thanks, > Maxime > > > Acked-By: Nicolas Chautru <nicolas.chautru@intel.com> > > > >> -----Original Message----- > >> From: Maxime Coquelin <maxime.coquelin@redhat.com> > >> Sent: Tuesday, October 6, 2020 3:04 AM > >> To: dev@dpdk.org; stable@dpdk.org; Chautru, Nicolas > >> <nicolas.chautru@intel.com>; trix@redhat.com > >> Cc: Maxime Coquelin <maxime.coquelin@redhat.com> > >> Subject: [PATCH 1/2] baseband/fpga_5gnr_fec: fix segfaults with debug > >> > >> When RTE_LIBRTE_BBDEV_DEBUG is enabled, rte_device's driver pointer > >> is dereferenced twice in fpga_5gnr_fec's probe callback. > >> It causes a segmentation fault because this pointer is only assigned > >> after probe callback call. > >> > >> This patch makes use of rte_pci_driver pointer instead. > >> > >> Fixes: 0b5927cbcba7 ("baseband/fpga_5gnr_fec: add PMD for FPGA 5GNR > >> FEC") > >> > >> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> > >> --- > >> drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c > >> b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c > >> index 61f9c04ba2..11ee4d3e10 100644 > >> --- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c > >> +++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c > >> @@ -1839,7 +1839,7 @@ fpga_5gnr_fec_init(struct rte_bbdev *dev, > >> struct rte_pci_driver *drv) > >> > >> rte_bbdev_log_debug( > >> "Init device %s [%s] @ virtaddr %p phyaddr > %#"PRIx64, > >> - dev->device->driver->name, dev->data->name, > >> + drv->driver.name, dev->data->name, > >> (void *)pci_dev->mem_resource[0].addr, > >> pci_dev->mem_resource[0].phys_addr); > >> } > >> @@ -1895,7 +1895,7 @@ fpga_5gnr_fec_probe(struct rte_pci_driver > *pci_drv, > >> ((uint16_t)(version_id >> 16)), ((uint16_t)version_id)); > >> > >> #ifdef RTE_LIBRTE_BBDEV_DEBUG > >> - if (!strcmp(bbdev->device->driver->name, > >> + if (!strcmp(pci_drv->driver.name, > > > > Why do you have to change this one? > > > > Acked-by: Nicolas Chautru <nicolas.chautru@intel.com> > > > >> RTE_STR(FPGA_5GNR_FEC_PF_DRIVER_NAME))) > >> print_static_reg_debug_info(d->mmio_base); > >> #endif > >> -- > >> 2.26.2 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [dpdk-dev] [PATCH 2/2] baseband/fpga_lte_fec: fix segfaults with debug 2020-10-06 10:04 [dpdk-dev] [PATCH 0/2] baseband: fix segfault in Intel drivers Maxime Coquelin 2020-10-06 10:04 ` [dpdk-dev] [PATCH 1/2] baseband/fpga_5gnr_fec: fix segfaults with debug Maxime Coquelin @ 2020-10-06 10:04 ` Maxime Coquelin 2020-10-06 16:05 ` [dpdk-dev] [PATCH 0/2] baseband: fix segfault in Intel drivers Tom Rix 2020-10-06 21:07 ` Akhil Goyal 3 siblings, 0 replies; 8+ messages in thread From: Maxime Coquelin @ 2020-10-06 10:04 UTC (permalink / raw) To: dev, stable, nicolas.chautru, trix; +Cc: Maxime Coquelin When RTE_LIBRTE_BBDEV_DEBUG is enabled, rte_device's driver pointer is dereferenced twice in fpga_lte_fec's probe callback. It causes a segmentation fault because this pointer is only assigned after probe callback call. This patch makes use of rte_pci_driver pointer instead Fixes: efd453698c49 ("baseband/fpga_lte_fec: add driver for FEC on FPGA") Cc: stable@dpdk.org Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> --- drivers/baseband/fpga_lte_fec/fpga_lte_fec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c index 37018b9c7f..c80721be96 100644 --- a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c +++ b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c @@ -2328,7 +2328,7 @@ fpga_lte_fec_init(struct rte_bbdev *dev, struct rte_pci_driver *drv) rte_bbdev_log_debug( "Init device %s [%s] @ virtaddr %p phyaddr %#"PRIx64, - dev->device->driver->name, dev->data->name, + drv->driver.name, dev->data->name, (void *)pci_dev->mem_resource[0].addr, pci_dev->mem_resource[0].phys_addr); } @@ -2383,7 +2383,7 @@ fpga_lte_fec_probe(struct rte_pci_driver *pci_drv, ((uint16_t)(version_id >> 16)), ((uint16_t)version_id)); #ifdef RTE_LIBRTE_BBDEV_DEBUG - if (!strcmp(bbdev->device->driver->name, + if (!strcmp(pci_drv->driver.name, RTE_STR(FPGA_LTE_FEC_PF_DRIVER_NAME))) print_static_reg_debug_info(d->mmio_base); #endif -- 2.26.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH 0/2] baseband: fix segfault in Intel drivers 2020-10-06 10:04 [dpdk-dev] [PATCH 0/2] baseband: fix segfault in Intel drivers Maxime Coquelin 2020-10-06 10:04 ` [dpdk-dev] [PATCH 1/2] baseband/fpga_5gnr_fec: fix segfaults with debug Maxime Coquelin 2020-10-06 10:04 ` [dpdk-dev] [PATCH 2/2] baseband/fpga_lte_fec: " Maxime Coquelin @ 2020-10-06 16:05 ` Tom Rix 2020-10-06 21:07 ` Akhil Goyal 3 siblings, 0 replies; 8+ messages in thread From: Tom Rix @ 2020-10-06 16:05 UTC (permalink / raw) To: Maxime Coquelin, dev, stable, nicolas.chautru This change looks fine. Reviewed-by: Tom Rix <trix@redhat.com> In looking at the surrounding code i see the is-pf bool being found by checking the driver name. That is a fairly expensive check. Is there a better way to do that ? Tom On 10/6/20 3:04 AM, Maxime Coquelin wrote: > This series fixes segfaults in fpga_5gnr_fec and fpga_lte_fec > drivers when bbdev debug is enabled. > > Maxime Coquelin (2): > baseband/fpga_5gnr_fec: fix segfaults with debug > baseband/fpga_lte_fec: fix segfaults with debug > > drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 4 ++-- > drivers/baseband/fpga_lte_fec/fpga_lte_fec.c | 4 ++-- > 2 files changed, 4 insertions(+), 4 deletions(-) > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH 0/2] baseband: fix segfault in Intel drivers 2020-10-06 10:04 [dpdk-dev] [PATCH 0/2] baseband: fix segfault in Intel drivers Maxime Coquelin ` (2 preceding siblings ...) 2020-10-06 16:05 ` [dpdk-dev] [PATCH 0/2] baseband: fix segfault in Intel drivers Tom Rix @ 2020-10-06 21:07 ` Akhil Goyal 3 siblings, 0 replies; 8+ messages in thread From: Akhil Goyal @ 2020-10-06 21:07 UTC (permalink / raw) To: Maxime Coquelin, dev, stable, nicolas.chautru, trix > Subject: [dpdk-dev] [PATCH 0/2] baseband: fix segfault in Intel drivers > > This series fixes segfaults in fpga_5gnr_fec and fpga_lte_fec > drivers when bbdev debug is enabled. > > Maxime Coquelin (2): > baseband/fpga_5gnr_fec: fix segfaults with debug > baseband/fpga_lte_fec: fix segfaults with debug > Applied to dpdk-next-crypto Thanks. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-10-06 21:07 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-06 10:04 [dpdk-dev] [PATCH 0/2] baseband: fix segfault in Intel drivers Maxime Coquelin 2020-10-06 10:04 ` [dpdk-dev] [PATCH 1/2] baseband/fpga_5gnr_fec: fix segfaults with debug Maxime Coquelin 2020-10-06 16:14 ` Chautru, Nicolas 2020-10-06 16:34 ` Maxime Coquelin 2020-10-06 17:41 ` Chautru, Nicolas 2020-10-06 10:04 ` [dpdk-dev] [PATCH 2/2] baseband/fpga_lte_fec: " Maxime Coquelin 2020-10-06 16:05 ` [dpdk-dev] [PATCH 0/2] baseband: fix segfault in Intel drivers Tom Rix 2020-10-06 21:07 ` Akhil Goyal
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).