From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752576AbdJTPyY (ORCPT ); Fri, 20 Oct 2017 11:54:24 -0400 Received: from smtp.citrix.com.au ([103.14.252.240]:41613 "EHLO SMTP.CITRIX.COM.AU" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752497AbdJTPyW (ORCPT ); Fri, 20 Oct 2017 11:54:22 -0400 X-IronPort-AV: E=Sophos;i="5.43,405,1503360000"; d="scan'208";a="106420913" From: Paul Durrant To: "'Boris Ostrovsky'" , "x86@kernel.org" , "xen-devel@lists.xenproject.org" , "linux-kernel@vger.kernel.org" CC: Juergen Gross , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" Subject: RE: [Xen-devel] [PATCH] x86/xen: support priv-mapping in an HVM tools domain Thread-Topic: [Xen-devel] [PATCH] x86/xen: support priv-mapping in an HVM tools domain Thread-Index: AQHTSbWGoKz+qSa2A0adTRrMC7a8o6Ls46Og Date: Fri, 20 Oct 2017 15:54:17 +0000 Message-ID: <9cc8c01010a04d4780b3147a4933202e@AMSPEX02CL03.citrite.net> References: <20171019152617.38463-1-paul.durrant@citrix.com> <4e7e63a0-5a77-9dee-bbb7-dfa79c116fc8@oracle.com> <69e93ee8-5d93-35fa-ffc1-d7a7fbbd5599@oracle.com> In-Reply-To: <69e93ee8-5d93-35fa-ffc1-d7a7fbbd5599@oracle.com> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by nfs id v9KFsSSX019628 > -----Original Message----- > From: Xen-devel [mailto:xen-devel-bounces@lists.xen.org] On Behalf Of > Boris Ostrovsky > Sent: 20 October 2017 16:09 > To: Paul Durrant ; x86@kernel.org; xen- > devel@lists.xenproject.org; linux-kernel@vger.kernel.org > Cc: Juergen Gross ; Thomas Gleixner > ; Ingo Molnar ; H. Peter Anvin > > Subject: Re: [Xen-devel] [PATCH] x86/xen: support priv-mapping in an HVM > tools domain > > On 10/20/2017 04:35 AM, Paul Durrant wrote: > >> -----Original Message----- > >> From: Xen-devel [mailto:xen-devel-bounces@lists.xen.org] On Behalf Of > >> Boris Ostrovsky > >> Sent: 19 October 2017 18:45 > >> To: Paul Durrant ; x86@kernel.org; xen- > >> devel@lists.xenproject.org; linux-kernel@vger.kernel.org > >> Cc: Juergen Gross ; Thomas Gleixner > >> ; Ingo Molnar ; H. Peter Anvin > >> > >> Subject: Re: [Xen-devel] [PATCH] x86/xen: support priv-mapping in an > HVM > >> tools domain > >> > >> On 10/19/2017 11:26 AM, Paul Durrant wrote: > >>> If the domain has XENFEAT_auto_translated_physmap then use of the > PV- > >>> specific HYPERVISOR_mmu_update hypercall is clearly incorrect. > >>> > >>> This patch adds checks in xen_remap_domain_gfn_array() and > >>> xen_unmap_domain_gfn_array() which call through to the approprate > >>> xlate_mmu function if the feature is present. A check is also added > >>> to xen_remap_domain_gfn_range() to fail with -EOPNOTSUPP since this > >>> should not be used in an HVM tools domain. > >>> > >>> Signed-off-by: Paul Durrant > >>> --- > >>> Cc: Boris Ostrovsky > >>> Cc: Juergen Gross > >>> Cc: Thomas Gleixner > >>> Cc: Ingo Molnar > >>> Cc: "H. Peter Anvin" > >>> --- > >>> arch/x86/xen/mmu.c | 14 ++++++++++++-- > >>> 1 file changed, 12 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c > >>> index 3e15345abfe7..d33e7dbe3129 100644 > >>> --- a/arch/x86/xen/mmu.c > >>> +++ b/arch/x86/xen/mmu.c > >>> @@ -172,6 +172,9 @@ int xen_remap_domain_gfn_range(struct > >> vm_area_struct *vma, > >>> pgprot_t prot, unsigned domid, > >>> struct page **pages) > >>> { > >>> + if (xen_feature(XENFEAT_auto_translated_physmap)) > >>> + return -EOPNOTSUPP; > >>> + > >> This is never called on XENFEAT_auto_translated_physmap domains, > there > >> is a check in privcmd_ioctl_mmap() for that. > > Yes, that's true but it seems like the wrong place for such a check. I could > remove that one it you'd prefer. > > I actually think that perhaps we could wrap privcmd_ioctl_mmap() with > "#ifdef CONFIG_XEN_PV" (#else return -ENOSYS) and move > xen_remap_domain_gfn_range() to mmu_pv.c. We can then remove it from > ARM > code too. > > > > >>> return do_remap_gfn(vma, addr, &gfn, nr, NULL, prot, domid, > >> pages); > >>> } > >>> EXPORT_SYMBOL_GPL(xen_remap_domain_gfn_range); > >>> @@ -182,6 +185,10 @@ int xen_remap_domain_gfn_array(struct > >> vm_area_struct *vma, > >>> int *err_ptr, pgprot_t prot, > >>> unsigned domid, struct page **pages) > >>> { > >>> + if (xen_feature(XENFEAT_auto_translated_physmap)) > >>> + return xen_xlate_remap_gfn_array(vma, addr, gfn, nr, > >> err_ptr, > >>> + prot, domid, pages); > >>> + > >> So how did this work before? In fact, I don't see any callers of > >> xen_xlate_{re|un}map_gfn_range(). > > I assume mean 'array' for the map since there is no > xen_xlate_remap_gfn_range() function. I'm not quite sure what you're > asking? Without this patch the mmu code in an x86 domain simply assumes > the domain is PV... the xlate code is currently only used via the arm mmu > code (where it clearly knows it's not PV). AFAICS this Is just a straightforward > buggy assumption in the x86 code. > > Looks like this was originally intended for dom0 PVH and was removed by > 063334f. So it should indeed be restored. > Ok, I'll re-work the patch with your suggestion re xen_remap_domain_gfn_range() and send a v2. Thanks, Paul > > -boris > > > > > Paul > > > >> -boris > >> > >> > >>> /* We BUG_ON because it's a programmer error to pass a NULL > >> err_ptr, > >>> * and the consequences later is quite hard to detect what the actual > >>> * cause of "wrong memory was mapped in". > >>> @@ -193,9 +200,12 @@ > >> EXPORT_SYMBOL_GPL(xen_remap_domain_gfn_array); > >>> /* Returns: 0 success */ > >>> int xen_unmap_domain_gfn_range(struct vm_area_struct *vma, > >>> - int numpgs, struct page **pages) > >>> + int nr, struct page **pages) > >>> { > >>> - if (!pages || !xen_feature(XENFEAT_auto_translated_physmap)) > >>> + if (xen_feature(XENFEAT_auto_translated_physmap)) > >>> + return xen_xlate_unmap_gfn_range(vma, nr, pages); > >>> + > >>> + if (!pages) > >>> return 0; > >>> > >>> return -EINVAL; > >> > >> _______________________________________________ > >> Xen-devel mailing list > >> Xen-devel@lists.xen.org > >> https://lists.xen.org/xen-devel > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > https://lists.xen.org/xen-devel