> -----Original Message-----
> From: Xen-devel [mailto:xen-devel-boun...@lists.xen.org] On Behalf Of
> Boris Ostrovsky
> Sent: 20 October 2017 16:09
> To: Paul Durrant <paul.durr...@citrix.com>; x...@kernel.org; xen-
> de...@lists.xenproject.org; linux-kernel@vger.kernel.org
> Cc: Juergen Gross <jgr...@suse.com>; Thomas Gleixner
> <t...@linutronix.de>; Ingo Molnar <mi...@redhat.com>; H. Peter Anvin
> <h...@zytor.com>
> 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-boun...@lists.xen.org] On Behalf Of
> >> Boris Ostrovsky
> >> Sent: 19 October 2017 18:45
> >> To: Paul Durrant <paul.durr...@citrix.com>; x...@kernel.org; xen-
> >> de...@lists.xenproject.org; linux-kernel@vger.kernel.org
> >> Cc: Juergen Gross <jgr...@suse.com>; Thomas Gleixner
> >> <t...@linutronix.de>; Ingo Molnar <mi...@redhat.com>; H. Peter Anvin
> >> <h...@zytor.com>
> >> 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 <paul.durr...@citrix.com>
> >>> ---
> >>> Cc: Boris Ostrovsky <boris.ostrov...@oracle.com>
> >>> Cc: Juergen Gross <jgr...@suse.com>
> >>> Cc: Thomas Gleixner <t...@linutronix.de>
> >>> Cc: Ingo Molnar <mi...@redhat.com>
> >>> Cc: "H. Peter Anvin" <h...@zytor.com>
> >>> ---
> >>> 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-de...@lists.xen.org
> >> https://lists.xen.org/xen-devel
>
>
> _______________________________________________
> Xen-devel mailing list
> xen-de...@lists.xen.org
> https://lists.xen.org/xen-devel
