[PATCH] x86/xen: support priv-mapping in an HVM tools domain

[PATCH] x86/xen: support priv-mapping in an HVM tools domain

[Paul Durrant]

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.durrant@citrix.com>
---
Boris Ostrovsky <boris.ostrovsky@oracle.com>
Juergen Gross <jgross@suse.com>
Thomas Gleixner <tglx@linutronix.de>
Ingo Molnar <mingo@redhat.com>
"H. Peter Anvin" <hpa@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;
+
 	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);
+
 	/* 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;
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH] x86/xen: support priv-mapping in an HVM tools domain

[Paul Durrant]

Apologies... I misformatted this. I will re-send.

  Paul
> -----Original Message-----
> From: Paul Durrant [mailto:paul.durrant@citrix.com]
> Sent: 19 October 2017 16:24
> To: x86@kernel.org; xen-devel@lists.xenproject.org; linux-
> kernel@vger.kernel.org
> Cc: Paul Durrant <Paul.Durrant@citrix.com>
> Subject: [PATCH] x86/xen: support priv-mapping in an HVM tools domain
> 
> 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.durrant@citrix.com>
> ---
> Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Juergen Gross <jgross@suse.com>
> Thomas Gleixner <tglx@linutronix.de>
> Ingo Molnar <mingo@redhat.com>
> "H. Peter Anvin" <hpa@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;
> +
>  	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);
> +
>  	/* 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;
> --
> 2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH] x86/xen: support priv-mapping in an HVM tools domain

[Paul Durrant]

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.durrant@citrix.com>
---
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@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;
+
 	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);
+
 	/* 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;
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH] x86/xen: support priv-mapping in an HVM tools domain

[Boris Ostrovsky]

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.durrant@citrix.com>
> ---
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@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.

>  	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().

-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

Re: [PATCH] x86/xen: support priv-mapping in an HVM tools domain

[Paul Durrant]

> -----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 <Paul.Durrant@citrix.com>; x86@kernel.org; xen-
> devel@lists.xenproject.org; linux-kernel@vger.kernel.org
> Cc: Juergen Gross <jgross@suse.com>; Thomas Gleixner
> <tglx@linutronix.de>; Ingo Molnar <mingo@redhat.com>; H. Peter Anvin
> <hpa@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.durrant@citrix.com>
> > ---
> > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> > Cc: Juergen Gross <jgross@suse.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: "H. Peter Anvin" <hpa@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.

> 
> >  	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.

  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

Re: [PATCH] x86/xen: support priv-mapping in an HVM tools domain

[Boris Ostrovsky]

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 <Paul.Durrant@citrix.com>; x86@kernel.org; xen-
>> devel@lists.xenproject.org; linux-kernel@vger.kernel.org
>> Cc: Juergen Gross <jgross@suse.com>; Thomas Gleixner
>> <tglx@linutronix.de>; Ingo Molnar <mingo@redhat.com>; H. Peter Anvin
>> <hpa@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.durrant@citrix.com>
>>> ---
>>> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>>> Cc: Juergen Gross <jgross@suse.com>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Cc: Ingo Molnar <mingo@redhat.com>
>>> Cc: "H. Peter Anvin" <hpa@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.


-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

Re: [PATCH] x86/xen: support priv-mapping in an HVM tools domain

[Paul Durrant]

> -----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 <Paul.Durrant@citrix.com>; x86@kernel.org; xen-
> devel@lists.xenproject.org; linux-kernel@vger.kernel.org
> Cc: Juergen Gross <jgross@suse.com>; Thomas Gleixner
> <tglx@linutronix.de>; Ingo Molnar <mingo@redhat.com>; H. Peter Anvin
> <hpa@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-bounces@lists.xen.org] On Behalf Of
> >> Boris Ostrovsky
> >> Sent: 19 October 2017 18:45
> >> To: Paul Durrant <Paul.Durrant@citrix.com>; x86@kernel.org; xen-
> >> devel@lists.xenproject.org; linux-kernel@vger.kernel.org
> >> Cc: Juergen Gross <jgross@suse.com>; Thomas Gleixner
> >> <tglx@linutronix.de>; Ingo Molnar <mingo@redhat.com>; H. Peter Anvin
> >> <hpa@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.durrant@citrix.com>
> >>> ---
> >>> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> >>> Cc: Juergen Gross <jgross@suse.com>
> >>> Cc: Thomas Gleixner <tglx@linutronix.de>
> >>> Cc: Ingo Molnar <mingo@redhat.com>
> >>> Cc: "H. Peter Anvin" <hpa@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-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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel