From: Paul Durrant <Paul.Durrant@citrix.com>
To: 'Jan Beulich' <JBeulich@suse.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>,
Kevin Tian <kevin.tian@intel.com>, Wei Liu <wei.liu2@citrix.com>,
George Dunlap <George.Dunlap@citrix.com>
Subject: Re: [PATCH v8 7/7] vtd: add lookup_page method to iommu_ops
Date: Fri, 14 Sep 2018 08:11:05 +0000 [thread overview]
Message-ID: <49ac1db732994cd9a001704bec2e1f75@AMSPEX02CL03.citrite.net> (raw)
In-Reply-To: <5B9B6A3702000078001E87F4@prv1-mh.provo.novell.com>
> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 14 September 2018 08:59
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: George Dunlap <George.Dunlap@citrix.com>; Wei Liu
> <wei.liu2@citrix.com>; Kevin Tian <kevin.tian@intel.com>; xen-devel <xen-
> devel@lists.xenproject.org>
> Subject: RE: [PATCH v8 7/7] vtd: add lookup_page method to iommu_ops
>
> >>> On 13.09.18 at 16:53, <Paul.Durrant@citrix.com> wrote:
> >> From: Jan Beulich [mailto:JBeulich@suse.com]
> >> Sent: 13 September 2018 15:51
> >>
> >> >>> On 13.09.18 at 16:04, <Paul.Durrant@citrix.com> wrote:
> >> >> From: Jan Beulich [mailto:JBeulich@suse.com]
> >> >> Sent: 13 September 2018 14:57
> >> >>
> >> >> >>> On 13.09.18 at 15:50, <Paul.Durrant@citrix.com> wrote:
> >> >> > Ok. I'll spell it out in the header if you think it is non-
> obvious.
> >> >>
> >> >> Obvious or not - do we _have_ any such outer locking in place right
> >> now?
> >> >>
> >> >
> >> > Yes. The locking is either via the P2M or grant table locks for all
> >> current
> >> > uses or map and unmap that I can see.
> >>
> >> But two different locks still means no guarantees at all.
> >>
> >
> > So, are you saying the current implementation is not fit for purpose? Do
> you
> > want me to add locking at the wrapper level?
>
> Well, I haven't looked closely enough to be certain, but I'm afraid there
> might be an issue, and if so I certainly think it needs taking care of
> before
> widening the problem by exposing (more of it) to guests. Of course it
> may well be that addition of another locking layer may have adverse
> effects, to existing code and/or your additions.
>
Given that all uses of map and unmap are under the grant or p2m locks then there should only be an issue if a race occurs between a grant table op and something modifying the p2m, and I doubt such an issue would be limited to leaving just the IOMMU in an incorrect state. This patch does nothing to rule out such a race, but nor does it make things any worse; it is completely orthogonal as it introduces a brand new function with (as yet) no callers.
This new lookup function is not intended for use by any of the existing code executed under grant table or p2m lock though. Once PV-IOMMU becomes operational then all of that automatic map/unmap code will cease to be operational. As you pointed out in review of another patch, I have completely neglected to add suitable locking into the PV-IOMMU code but once I add that then map, unmap and lookup operations coming via hypercall will be protected from each other.
Paul
> Jan
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2018-09-14 8:11 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-13 10:31 [PATCH v8 0/6] paravirtual IOMMU pre-requisites and clean-up Paul Durrant
2018-09-13 10:31 ` [PATCH v8 1/7] iommu: introduce the concept of DFN Paul Durrant
2018-09-13 12:45 ` Jan Beulich
2018-09-13 13:09 ` Paul Durrant
2018-09-13 13:21 ` Paul Durrant
2018-09-13 13:40 ` Jan Beulich
2018-09-13 13:46 ` Paul Durrant
2018-09-13 14:14 ` Paul Durrant
2018-09-13 10:31 ` [PATCH v8 2/7] iommu: make use of type-safe DFN and MFN in exported functions Paul Durrant
2018-09-13 10:31 ` [PATCH v8 3/7] iommu: push use of type-safe DFN and MFN into iommu_ops Paul Durrant
2018-09-13 10:31 ` [PATCH v8 4/7] iommu: don't domain_crash() inside iommu_map/unmap_page() Paul Durrant
2018-09-13 10:31 ` [PATCH v8 5/7] memory: add check_get_page_from_gfn() as a wrapper Paul Durrant
2018-09-13 13:22 ` Jan Beulich
2018-09-13 13:43 ` Paul Durrant
2018-09-13 10:31 ` [PATCH v7 6/6] vtd: add lookup_page method to iommu_ops Paul Durrant
2018-09-13 10:31 ` [PATCH v8 6/7] vtd: add missing check for shared EPT Paul Durrant
2018-09-13 10:31 ` [PATCH v8 7/7] vtd: add lookup_page method to iommu_ops Paul Durrant
2018-09-13 13:29 ` Jan Beulich
2018-09-13 13:34 ` Paul Durrant
2018-09-13 13:44 ` Jan Beulich
2018-09-13 13:50 ` Paul Durrant
2018-09-13 13:56 ` Jan Beulich
2018-09-13 14:04 ` Paul Durrant
2018-09-13 14:50 ` Jan Beulich
2018-09-13 14:53 ` Paul Durrant
2018-09-14 7:58 ` Jan Beulich
2018-09-14 8:11 ` Paul Durrant [this message]
2018-09-14 8:28 ` Jan Beulich
2018-09-14 8:48 ` Paul Durrant
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=49ac1db732994cd9a001704bec2e1f75@AMSPEX02CL03.citrite.net \
--to=paul.durrant@citrix.com \
--cc=George.Dunlap@citrix.com \
--cc=JBeulich@suse.com \
--cc=kevin.tian@intel.com \
--cc=wei.liu2@citrix.com \
--cc=xen-devel@lists.xenproject.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).