From: Jerin Jacob <jerin.jacob@caviumnetworks.com>
To: Alejandro Lucero <alejandro.lucero@netronome.com>
Cc: Thomas Monjalon <thomas@monjalon.net>, dev <dev@dpdk.org>,
"Burakov, Anatoly" <anatoly.burakov@intel.com>
Subject: Re: [dpdk-dev] [PATCH] vfio: fix device unplug when several devices per vfio group
Date: Tue, 9 May 2017 09:43:53 +0530 [thread overview]
Message-ID: <20170509041352.GB6207@jerin> (raw)
In-Reply-To: <CAD+H991NEXGCwTYVaOuwPPQX5d09pve6LRLyPOrj64QT33iQkg@mail.gmail.com>
-----Original Message-----
> Date: Mon, 8 May 2017 17:44:37 +0100
> From: Alejandro Lucero <alejandro.lucero@netronome.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> Cc: Thomas Monjalon <thomas@monjalon.net>, dev <dev@dpdk.org>, "Burakov,
> Anatoly" <anatoly.burakov@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] vfio: fix device unplug when several
> devices per vfio group
>
> Hi Jerin,
>
> On Mon, May 8, 2017 at 4:20 PM, Jerin Jacob <jerin.jacob@caviumnetworks.com>
> wrote:
>
> > -----Original Message-----
> > > Date: Sun, 30 Apr 2017 19:29:49 +0200
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > To: Alejandro Lucero <alejandro.lucero@netronome.com>
> > > Cc: dev@dpdk.org, "Burakov, Anatoly" <anatoly.burakov@intel.com>
> > > Subject: Re: [dpdk-dev] [PATCH] vfio: fix device unplug when several
> > > devices per vfio group
> > >
> > > 28/04/2017 15:25, Burakov, Anatoly:
> > > > From: Alejandro Lucero [mailto:alejandro.lucero@netronome.com]
> > > > > VFIO allows a secure way of assigning devices to user space and those
> > > > > devices which can not be isolated from other ones are set in same
> > VFIO
> > > > > group. Releasing or unplugging a device should be aware of remaining
> > > > > devices is the same group for avoiding to close such a group.
> > > > >
> > > > > Fixes: 94c0776b1bad ("vfio: support hotplug")
> > > > >
> > > > > Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
> > > >
> > > > I have tested this on my setup on an old kernel with multiple
> > attach/detaches, and it works (whereas it fails without this patch).
> > > >
> > > > Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > >
> > > Applied, thanks
> >
> > This patch creates issue when large number of PCIe devices connected to
> > system.
> > Found it through git bisect.
> >
> > This issue is, vfio_group_fd goes beyond 64(VFIO_MAX_GROUPS) and writes
> > to wrong memory on following code execution and sub sequentially creates
> > issues in vfio mapping or such.
> >
> vfio_cfg.vfio_groups[vfio_group_fd].devices++;
> >
> > I can increase VFIO_MAX_GROUPS, but I think, it is not correct fix as
> > vfio_group_fd generated from open system call.
> >
> > I add some prints the code for debug. Please find below the output.
> > Any thoughts from VFIO experts?
> >
> >
> That is a silly but serious bug. We are using the file descriptor as the
> index for updating devices counter of a vfio group structure internal to
> DPDK VFIO code. We should be using the vfio_group that file descriptor is
> registered with.
>
> I will send a fix where vfio_group_device_get/put/count functions are
> implemented which take the file descriptor as a parameter and then go
> through the vfio_group array for working with the right one.
>
> Thomas, is this fix in time yet for 17.05? I will send the patch today but
> I can just test it against a system with the "normal" case for VFIO device
> groups. Maybe Jerin or/and Anatoly can test it against the other case.
Thanks Alejandro for the patch.
Tested your patch on failure setup, it works fine after applying your
patch.
IMO, for v17.05, this fix must go in or we need to revert the original offending
patch(a9c349e3a100 ("vfio: fix device unplug when several devices per
group")) as it breaks DPDK running on the system with few PCIe devices connected in.
prev parent reply other threads:[~2017-05-09 4:14 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-26 10:49 Alejandro Lucero
2017-04-28 13:25 ` Burakov, Anatoly
2017-04-30 17:29 ` Thomas Monjalon
2017-05-08 15:20 ` Jerin Jacob
2017-05-08 16:44 ` Alejandro Lucero
2017-05-08 17:59 ` Alejandro Lucero
2017-05-09 4:13 ` Jerin Jacob [this message]
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=20170509041352.GB6207@jerin \
--to=jerin.jacob@caviumnetworks.com \
--cc=alejandro.lucero@netronome.com \
--cc=anatoly.burakov@intel.com \
--cc=dev@dpdk.org \
--cc=thomas@monjalon.net \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).