Hi Anatoly, On Tue, May 9, 2017 at 4:18 PM, Burakov, Anatoly <anatoly.bura...@intel.com> wrote:
> Hi Alejandro, > > > From: Alejandro Lucero [mailto:alejandro.luc...@netronome.com] > > Sent: Monday, May 8, 2017 6:44 PM > > To: dev@dpdk.org > > Cc: Burakov, Anatoly <anatoly.bura...@intel.com>; > > jerin.ja...@caviumnetworks.com; tho...@monjalon.net > > Subject: [PATCH] vfio: use right index when tracking devices in a vfio > group > > > > Previous fix for properly handling devices from the same VFIO group > > introduced another bug where the file descriptor of a kernel vfio group > is > > used as the index for tracking number of devices of a vfio group struct > > handled by dpdk vfio code. Instead of the file descriptor itself, the > vfio group > > object that file descriptor is registered with has to be used. > > > > This patch introduces specific functions for incrementing or decrementing > > the device counter for a specific vfio group using the vfio file > descriptor as a > > parameter. Note the code is not optimized as the vfio group is found > > sequentially going through the vfio group array but this should not be a > > problem as this is not related to packet handling at all. > > > > Fixes: a9c349e3a100 ("vfio: fix device unplug when several devices per > > group") > > > > Signed-off-by: Alejandro Lucero <alejandro.luc...@netronome.com> > > --- > > lib/librte_eal/linuxapp/eal/eal_vfio.c | 60 > > +++++++++++++++++++++++++++------- > > 1 file changed, 49 insertions(+), 11 deletions(-) > > > > diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c > > b/lib/librte_eal/linuxapp/eal/eal_vfio.c > > index d3eae20..21d126f 100644 > > --- a/lib/librte_eal/linuxapp/eal/eal_vfio.c > > +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c > > @@ -172,6 +172,44 @@ > > return -1; > > } > > > > + > > +static int > > +get_vfio_group_idx(int vfio_group_fd) > > +{ > > + int i; > > + for (i = 0; i < VFIO_MAX_GROUPS; i++) > > + if (vfio_cfg.vfio_groups[i].fd == vfio_group_fd) > > + return i; > > + return -1; > > +} > > + > > +static void > > +vfio_group_device_get(int vfio_group_fd) { > > + int i; > > + > > + i = get_vfio_group_idx(vfio_group_fd); > > + vfio_cfg.vfio_groups[i].devices++; > > Maybe add a check for I < 0? > > Right. I doubted about adding such a check, since being < 0 or > VFIO_MAX_GROUPS are bad news. But better to avoid using a wrong index which could led to a random write elsewhere. I will send another patch version with that check and a RTE_LOG(ERR ... call. And same for the other functions. Thanks > > +} > > + > > +static void > > +vfio_group_device_put(int vfio_group_fd) { > > + int i; > > + > > + i = get_vfio_group_idx(vfio_group_fd); > > + vfio_cfg.vfio_groups[i].devices--; > > Same here. > > > +} > > + > > +static int > > +vfio_group_device_count(int vfio_group_fd) { > > + int i; > > + > > + i = get_vfio_group_idx(vfio_group_fd); > > + return vfio_cfg.vfio_groups[i].devices; } > > And here. > > > + > > int > > clear_group(int vfio_group_fd) > > { > > @@ -180,15 +218,14 @@ > > > > if (internal_config.process_type == RTE_PROC_PRIMARY) { > > > > - for (i = 0; i < VFIO_MAX_GROUPS; i++) > > - if (vfio_cfg.vfio_groups[i].fd == vfio_group_fd) { > > - vfio_cfg.vfio_groups[i].group_no = -1; > > - vfio_cfg.vfio_groups[i].fd = -1; > > - vfio_cfg.vfio_groups[i].devices = 0; > > - vfio_cfg.vfio_active_groups--; > > - return 0; > > - } > > - return -1; > > + i = get_vfio_group_idx(vfio_group_fd); > > + if ( i < 0) > > + return -1; > > + vfio_cfg.vfio_groups[i].group_no = -1; > > + vfio_cfg.vfio_groups[i].fd = -1; > > + vfio_cfg.vfio_groups[i].devices = 0; > > + vfio_cfg.vfio_active_groups--; > > + return 0; > > } > > > > /* This is just for SECONDARY processes */ @@ -358,7 +395,7 @@ > > clear_group(vfio_group_fd); > > return -1; > > } > > - vfio_cfg.vfio_groups[vfio_group_fd].devices++; > > + vfio_group_device_get(vfio_group_fd); > > > > return 0; > > } > > @@ -406,7 +443,8 @@ > > /* An VFIO group can have several devices attached. Just when there > > is > > * no devices remaining should the group be closed. > > */ > > - if (--vfio_cfg.vfio_groups[vfio_group_fd].devices == 0) { > > + vfio_group_device_put(vfio_group_fd); > > + if (!vfio_group_device_count(vfio_group_fd)) { > > > > if (close(vfio_group_fd) < 0) { > > RTE_LOG(INFO, EAL, "Error when closing > > vfio_group_fd for %s\n", > > -- > > 1.9.1 > >
