DPDK patches and discussions
 help / color / mirror / Atom feed
From: Maxime Coquelin <maxime.coquelin@redhat.com>
To: "Chautru, Nicolas" <nicolas.chautru@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"stable@dpdk.org" <stable@dpdk.org>,
	"trix@redhat.com" <trix@redhat.com>
Subject: Re: [dpdk-dev] [PATCH 1/2] baseband/fpga_5gnr_fec: fix segfaults with debug
Date: Tue, 6 Oct 2020 18:34:18 +0200
Message-ID: <a96db390-2cf6-3ef5-8756-31260a3f5597@redhat.com> (raw)
In-Reply-To: <BY5PR11MB4451289FA17C7D33B7FBEC38F80D0@BY5PR11MB4451.namprd11.prod.outlook.com>

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
> 


  reply	other threads:[~2020-10-06 16:34 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a96db390-2cf6-3ef5-8756-31260a3f5597@redhat.com \
    --to=maxime.coquelin@redhat.com \
    --cc=dev@dpdk.org \
    --cc=nicolas.chautru@intel.com \
    --cc=stable@dpdk.org \
    --cc=trix@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git