From: "Yigit, Ferruh" <ferruh.yigit@intel.com>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] VFIO no-iommu
Date: Fri, 18 Dec 2015 10:43:10 +0000 [thread overview]
Message-ID: <20151218104310.GA11371@sivlogin002.ir.intel.com> (raw)
In-Reply-To: <1450370639.2674.93.camel@redhat.com>
On Thu, Dec 17, 2015 at 09:43:59AM -0700, Alex Williamson wrote:
<...>
> > > > > >
> > > > > > Also I need to disable VFIO_CHECK_EXTENSION ioctl, because in
> > > > > > vfio
> > > > > > module,
> > > > > > container->noiommu is not set before doing a
> > > > > > vfio_group_set_container()
> > > > > > and vfio_for_each_iommu_driver selects wrong driver.
> > > > >
> > > > > Running CHECK_EXTENSION on a container without the group
> > > > > attached is
> > > > > only going to tell you what extensions vfio is capable of, not
> > > > > necessarily what extensions are available to you with that
> > > > > group.
> > > > > Is this just a general dpdk- vfio ordering bug?
> > > >
> > > > Yes, that is how VFIO was implemented in DPDK. I was under the
> > > > impression that checking extension before assigning devices was
> > > > the
> > > > correct way to do things, so as to not to try anything we know
> > > > would
> > > > fail anyway. Does this imply that CHECK_EXTENSION needs to be
> > > > called
> > > > on both container and groups (or just on groups)?
> > >
> > > Hmm, in Documentation/vfio.txt we do give the following algorithm:
> > >
> > > if (ioctl(container, VFIO_GET_API_VERSION) !=
> > > VFIO_API_VERSION)
> > > /* Unknown API version */
> > >
> > > if (!ioctl(container, VFIO_CHECK_EXTENSION,
> > > VFIO_TYPE1_IOMMU))
> > > /* Doesn't support the IOMMU driver we want. */
> > > ...
> > >
> > > That's just going to query each iommu driver and we can't yet say
> > > whether
> > > the group the user attaches to the container later will actually
> > > support that
> > > extension until we try to do it, that would come at VFIO_SET_IOMMU.
> > > So is
> > > it perhaps a vfio bug that we're not advertising no-iommu until the
> > > group is
> > > attached? After all, we are capable of it with just an empty
> > > container, just
> > > like we are with type1, but we're going to fail SET_IOMMU for the
> > > wrong
> > > combination.
> > > This is exactly the sort of thing that makes me glad we reverted
> > > it without
> > > feedback from a working user driver. Thanks,
> >
> > Whether it should be considered a "bug" in VFIO or "by design" is up
> > to you, of course, but at least according to the VFIO documentation,
> > we are meant to check for type 1 extension and then attach devices,
> > so it would be expected to get VFIO_NOIOMMU_IOMMU marked as supported
> > even without any devices attached to the container (just like we get
> > type 1 as supported without any devices attached). Having said that,
> > if it was meant to attach devices first and then check the
> > extensions, then perhaps the documentation should also point out that
> > fact (or perhaps I missed that detail in my readings of the docs, in
> > which case my apologies).
>
> Hi Anatoly,
>
> Does the below patch make it behave more like you'd expect. This
> applies to v4.4-rc4, I'd fold this into the base patch if we
> reincorporate it to a future kernel. Thanks,
>
> Alex
>
> commit 88d4dcb6b77624965f0b45b5cd305a2b4a105c94
> Author: Alex Williamson <alex.williamson@redhat.com>
> Date: Wed Dec 16 19:02:01 2015 -0700
>
> vfio: Fix no-iommu CHECK_EXTENSION
>
> Previously the no-iommu iommu driver was only visible when the
> container had an attached no-iommu group. This means that
> CHECK_EXTENSION on and empty container couldn't report the possibility
> of using VFIO_NOIOMMU_IOMMU. We report TYPE1 whether or not the user
> can make use of it with the group, so this is inconsistent. Add the
> no-iommu iommu to the list of iommu drivers when enabled via module
> option, but skip all the others if the container is attached to a
> no-iommu groups. Note that tainting is now done with the "unsafe"
> module callback rather than explictly within vfio.
>
> Also fixes module option and module description name inconsistency.
>
> Also make vfio_noiommu_ops const.
>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Hi Alex,
I got following crash with this update:
[ +0.046973] BUG: unable to handle kernel NULL pointer dereference at (null)
[ +0.000031] IP: [<ffffffff813b92cf>] __list_add+V0x1f/0xc0
[ +0.000024] PGD 0
[ +0.000010] Oops: 0000 [#1] SMP
...
[ +0.000028] Call Trace:
[ +0.000014] [<ffffffff81777301>] __mutex_lock_slowpath+0x91/0x110
[ +0.000022] [<ffffffff817773a3>] mutex_lock+0x23/0x40
[ +0.000020] [<ffffffffa0370c00>] vfio_register_iommu_driver+0x40/0xc0 [vfio]
[ +0.000025] [<ffffffffa0370cd1>] noiommu_param_set+0x51/0x60 [vfio]
[ +0.000022] [<ffffffff810bbe3e>] parse_args+0x1be/0x4a0
[ +0.000021] [<ffffffff81121ea0>] load_module+0xe30/0x2730
[ +0.000942] [<ffffffff8111f500>] ? __symbol_put+0x60/0x60
[ +0.000921] [<ffffffff81223f60>] ? kernel_read+0x50/0x80
[ +0.000917] [<ffffffff811239f9>] SyS_finit_module+0xb9/0xf0
[ +0.000925] [<ffffffff817796ae>] entry_SYSCALL_64_fastpath+0x12/0x71
> -static struct vfio_iommu_driver vfio_noiommu_driver = {
> - .ops = &vfio_noiommu_ops,
> +static int noiommu_param_set(const char *val, const struct kernel_param *kp)
> +{
> + int ret;
> +
> + if (!val)
> + val = "1";
> +
> + ret = strtobool(val, kp->arg);
> + if (ret)
> + return ret;
> +
> + if (noiommu)
> + ret = vfio_register_iommu_driver(&vfio_noiommu_ops);
Is it possible that kernel_param_ops->set() called before module_init() that initializes the mutex.
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/kernel/module.c?id=refs/tags/v4.4-rc5#n3517
> + else
> + vfio_unregister_iommu_driver(&vfio_noiommu_ops);
> +
> + return ret;
> +}
> +
> +static const struct kernel_param_ops noiommu_param_ops = {
> + .flags = KERNEL_PARAM_OPS_FL_NOARG,
> + .set = noiommu_param_set,
> + .get = param_get_bool,
> };
>
Thanks,
ferruh
next prev parent reply other threads:[~2015-12-18 10:43 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-11 16:28 Thomas Monjalon
2015-12-11 22:12 ` Vincent JARDIN
2015-12-11 23:02 ` Alex Williamson
2015-12-15 13:43 ` O'Driscoll, Tim
2015-12-15 16:53 ` Alex Williamson
2015-12-16 4:04 ` Ferruh Yigit
2015-12-16 4:38 ` Alex Williamson
2015-12-16 8:35 ` Burakov, Anatoly
2015-12-16 16:23 ` Burakov, Anatoly
2015-12-16 23:17 ` Thomas Monjalon
2015-12-17 9:52 ` Burakov, Anatoly
2015-12-17 10:09 ` Thomas Monjalon
2015-12-17 19:38 ` Jan Viktorin
2015-12-17 21:16 ` Vincent JARDIN
2015-12-17 23:29 ` Stephen Hemminger
2015-12-16 17:11 ` Alex Williamson
2015-12-16 17:22 ` Burakov, Anatoly
2015-12-17 16:43 ` Alex Williamson
2015-12-18 10:43 ` Yigit, Ferruh [this message]
2015-12-18 14:38 ` Alex Williamson
2015-12-18 21:50 ` Alex Williamson
2015-12-21 11:46 ` Yigit, Ferruh
2015-12-21 12:18 ` [dpdk-dev] [PATCH] vfio: add no-iommu support Ferruh Yigit
2015-12-21 15:15 ` Burakov, Anatoly
2015-12-21 15:26 ` Yigit, Ferruh
2015-12-21 15:28 ` Burakov, Anatoly
2015-12-21 19:22 ` [dpdk-dev] VFIO no-iommu Alex Williamson
2015-12-22 20:20 ` Alex Williamson
2015-12-23 11:19 ` Burakov, Anatoly
2015-12-31 14:30 ` Santosh Shukla
2016-01-14 6:03 ` Jike Song
2016-01-14 6:52 ` Alex Williamson
2016-01-14 8:12 ` Jike Song
2015-12-11 23:20 ` Jan Viktorin
2015-12-15 11:20 ` Alejandro Lucero
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=20151218104310.GA11371@sivlogin002.ir.intel.com \
--to=ferruh.yigit@intel.com \
--cc=alex.williamson@redhat.com \
--cc=dev@dpdk.org \
/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).