On 10/20/2017 04:35 AM, Paul Durrant wrote:
>> -----Original Message-----
>> From: Xen-devel [mailto:[email protected]] On Behalf Of
>> Boris Ostrovsky
>> Sent: 19 October 2017 18:45
>> To: Paul Durrant <[email protected]>; [email protected]; xen-
>> [email protected]; [email protected]
>> Cc: Juergen Gross <[email protected]>; Thomas Gleixner
>> <[email protected]>; Ingo Molnar <[email protected]>; H. Peter Anvin
>> <[email protected]>
>> 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 <[email protected]>
>>> ---
>>> Cc: Boris Ostrovsky <[email protected]>
>>> Cc: Juergen Gross <[email protected]>
>>> Cc: Thomas Gleixner <[email protected]>
>>> Cc: Ingo Molnar <[email protected]>
>>> Cc: "H. Peter Anvin" <[email protected]>
>>> ---
>>> 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
>> [email protected]
>> https://lists.xen.org/xen-devel