Re: [Xen-devel] [PATCH v4 01/12] x86/mm: allow a privileged PV domain to map guest mfns

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 07/09/17 12:02, Wei Liu wrote:
> On Tue, Sep 05, 2017 at 12:37:05PM +0100, Paul Durrant wrote:
>> In the case where a PV domain is mapping guest resources then it needs make
>> the HYPERVISOR_mmu_update call using DOMID_SELF, rather than the guest
>> domid, so that the passed in gmfn values are correctly treated as mfns
>> rather than gfns present in the guest p2m.
>>
>> This patch removes a check which currently disallows mapping of a page when
>> the owner of the page tables matches the domain passed to
>> HYPERVISOR_mmu_update, but that domain is not the real owner of the page.
>> The check was introduced by patch d3c6a215ca9 ("x86: Clean up
>> get_page_from_l1e() to correctly distinguish between owner-of-pte and
>> owner-of-data-page in all cases") but it's not clear why it was needed.
>>
>> Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
>> ---
>> Cc: Jan Beulich <jbeulich@xxxxxxxx>
>> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> ---
>>  xen/arch/x86/mm.c | 13 ++++++++-----
>>  1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
>> index c94f1e5406..bd8aeac59e 100644
>> --- a/xen/arch/x86/mm.c
>> +++ b/xen/arch/x86/mm.c
>> @@ -1024,12 +1024,15 @@ get_page_from_l1e(
>>                     (real_pg_owner != dom_cow) ) )
>>      {
>>          /*
>> -         * Let privileged domains transfer the right to map their target
>> -         * domain's pages. This is used to allow stub-domain pvfb export to
>> -         * dom0, until pvfb supports granted mappings. At that time this
>> -         * minor hack can go away.
>> +         * If the real page owner is not the domain specified in the
>> +         * hypercall then establish that the specified domain has
>> +         * mapping privilege over the page owner.
>> +         * This is used to allow stub-domain pvfb export to dom0. It is
>> +         * also used to allow a privileged PV domain to map mfns using
>> +         * DOMID_SELF, which is needed for mapping guest resources such
>> +         * grant table frames.
>>           */
>> -        if ( (real_pg_owner == NULL) || (pg_owner == l1e_owner) ||
>> +        if ( (real_pg_owner == NULL) ||
> I still can't quite figure out if it is safe to remove the check.
>
> Looking at the rest of the series, you already have the foreign domid to
> hand when you call the get_resource hypercall. What is wrong with using
> that directly? Why do you need DOMID_SELF in the first place?

The problem is that when passing a foreign domid, the GFN offered by
dom0 gets interpreted using the foreign domU's paging mode.

In the mapping case (for PV guests), Xen has given us a MFN which
logically belongs to domU, which we need to map as an MFN, rather than
having Xen think "Ah - for domU, I need to do a GFN translation for you".

Passing DOMID_SELF causes Xen to treat the mapping with our own paging
mode, not the paging mode of the page owner.

~Andrew

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