| Message ID | 1589859732-5008-1-git-send-email-wangyunjian@huawei.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Marchand |
| Headers | show |
| Series | fixes for vfio | expand |
| Context | Check | Description |
|---|---|---|
| ci/travis-robot | success | Travis build: passed |
| ci/Intel-compilation | success | Compilation OK |
| ci/checkpatch | success | coding style OK |
Friendly ping > -----Original Message----- > From: wangyunjian > Sent: Tuesday, May 19, 2020 11:42 AM > To: dev@dpdk.org > Cc: anatoly.burakov@intel.com; hemant.agrawal@nxp.com; > sachin.saxena@nxp.com; Lilijun (Jerry) <jerry.lilijun@huawei.com>; xudingke > <xudingke@huawei.com>; wangyunjian <wangyunjian@huawei.com>; > stable@dpdk.org > Subject: [dpdk-dev] [PATCH 2/2] bus/fslmc: fix check for vfio_group_fd > > From: Yunjian Wang <wangyunjian@huawei.com> > > The issue is that a file descriptor at 0 is a valid one. Currently the file not found, > the return value will be set to 0. As a result, it is impossible to distinguish > between a correct descriptor and a failed return value. Fix it to return -ENOENT > instead of 0. > > Fixes: a69f79300262 ("bus/fslmc: support multi VFIO group") > Cc: stable@dpdk.org > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> > --- > drivers/bus/fslmc/fslmc_vfio.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/bus/fslmc/fslmc_vfio.c b/drivers/bus/fslmc/fslmc_vfio.c index > 970969d2b..04ccf4af6 100644 > --- a/drivers/bus/fslmc/fslmc_vfio.c > +++ b/drivers/bus/fslmc/fslmc_vfio.c > @@ -448,11 +448,14 @@ fslmc_vfio_setup_device(const char *sysfs_base, > const char *dev_addr, > > /* get the actual group fd */ > vfio_group_fd = rte_vfio_get_group_fd(iommu_group_no); > - if (vfio_group_fd < 0) > + if (vfio_group_fd < 0 && vfio_group_fd != -ENOENT) > return -1; > > - /* if group_fd == 0, that means the device isn't managed by VFIO */ > - if (vfio_group_fd == 0) { > + /* > + * if vfio_group_fd == -ENOENT, that means the device > + * isn't managed by VFIO > + */ > + if (vfio_group_fd == -ENOENT) { > RTE_LOG(WARNING, EAL, " %s not managed by VFIO driver, > skipping\n", > dev_addr); > return 1; > -- > 2.23.0 >
On 19-May-20 4:42 AM, wangyunjian wrote: > From: Yunjian Wang <wangyunjian@huawei.com> > > The issue is that a file descriptor at 0 is a valid one. Currently > the file not found, the return value will be set to 0. As a result, > it is impossible to distinguish between a correct descriptor and a > failed return value. Fix it to return -ENOENT instead of 0. > > Fixes: a69f79300262 ("bus/fslmc: support multi VFIO group") > Cc: stable@dpdk.org > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> > --- I am unfamiliar with bus/fslmc code but i've taken a quick look, and i've noticed that there's another instance of get_group_fd() usage that you're not modifying - is that intentional?
> -----Original Message----- > From: Burakov, Anatoly [mailto:anatoly.burakov@intel.com] > Sent: Thursday, September 17, 2020 8:56 PM > To: wangyunjian <wangyunjian@huawei.com>; dev@dpdk.org > Cc: hemant.agrawal@nxp.com; sachin.saxena@nxp.com; Lilijun (Jerry) > <jerry.lilijun@huawei.com>; xudingke <xudingke@huawei.com>; > stable@dpdk.org > Subject: Re: [dpdk-dev] [PATCH 2/2] bus/fslmc: fix check for vfio_group_fd > > On 19-May-20 4:42 AM, wangyunjian wrote: > > From: Yunjian Wang <wangyunjian@huawei.com> > > > > The issue is that a file descriptor at 0 is a valid one. Currently the > > file not found, the return value will be set to 0. As a result, it is > > impossible to distinguish between a correct descriptor and a failed > > return value. Fix it to return -ENOENT instead of 0. > > > > Fixes: a69f79300262 ("bus/fslmc: support multi VFIO group") > > Cc: stable@dpdk.org > > > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> > > --- > > I am unfamiliar with bus/fslmc code but i've taken a quick look, and i've noticed > that there's another instance of get_group_fd() usage that you're not > modifying - is that intentional? Thank you for your review. The another instance of get_group_fd(), is this it? int fslmc_vfio_setup_group(void) { ... /* Get the actual group fd */ ret = rte_vfio_get_group_fd(groupid); if (ret < 0) return ret; vfio_group.fd = ret; ... } I don't think this's necessary. Because it must be a valid descriptor before it can be used. Yunjian > > -- > Thanks, > Anatoly
On 17-Sep-20 2:34 PM, wangyunjian wrote: >> -----Original Message----- >> From: Burakov, Anatoly [mailto:anatoly.burakov@intel.com] >> Sent: Thursday, September 17, 2020 8:56 PM >> To: wangyunjian <wangyunjian@huawei.com>; dev@dpdk.org >> Cc: hemant.agrawal@nxp.com; sachin.saxena@nxp.com; Lilijun (Jerry) >> <jerry.lilijun@huawei.com>; xudingke <xudingke@huawei.com>; >> stable@dpdk.org >> Subject: Re: [dpdk-dev] [PATCH 2/2] bus/fslmc: fix check for vfio_group_fd >> >> On 19-May-20 4:42 AM, wangyunjian wrote: >>> From: Yunjian Wang <wangyunjian@huawei.com> >>> >>> The issue is that a file descriptor at 0 is a valid one. Currently the >>> file not found, the return value will be set to 0. As a result, it is >>> impossible to distinguish between a correct descriptor and a failed >>> return value. Fix it to return -ENOENT instead of 0. >>> >>> Fixes: a69f79300262 ("bus/fslmc: support multi VFIO group") >>> Cc: stable@dpdk.org >>> >>> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> >>> --- >> >> I am unfamiliar with bus/fslmc code but i've taken a quick look, and i've noticed >> that there's another instance of get_group_fd() usage that you're not >> modifying - is that intentional? > > Thank you for your review. The another instance of get_group_fd(), is this it? > int > fslmc_vfio_setup_group(void) { > ... > /* Get the actual group fd */ > ret = rte_vfio_get_group_fd(groupid); > if (ret < 0) > return ret; > vfio_group.fd = ret; > ... > } > I don't think this's necessary. Because it must be a valid descriptor before it can be used. > > Yunjian > >> >> -- >> Thanks, >> Anatoly OK. I'll leave this for fslmc bus maintainers to review, but the patch looks fine to me. Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
diff --git a/drivers/bus/fslmc/fslmc_vfio.c b/drivers/bus/fslmc/fslmc_vfio.c index 970969d2b..04ccf4af6 100644 --- a/drivers/bus/fslmc/fslmc_vfio.c +++ b/drivers/bus/fslmc/fslmc_vfio.c @@ -448,11 +448,14 @@ fslmc_vfio_setup_device(const char *sysfs_base, const char *dev_addr, /* get the actual group fd */ vfio_group_fd = rte_vfio_get_group_fd(iommu_group_no); - if (vfio_group_fd < 0) + if (vfio_group_fd < 0 && vfio_group_fd != -ENOENT) return -1; - /* if group_fd == 0, that means the device isn't managed by VFIO */ - if (vfio_group_fd == 0) { + /* + * if vfio_group_fd == -ENOENT, that means the device + * isn't managed by VFIO + */ + if (vfio_group_fd == -ENOENT) { RTE_LOG(WARNING, EAL, " %s not managed by VFIO driver, skipping\n", dev_addr); return 1;