Re: [Xen-devel] [PATCH 2/2] memory: don't hand MFN info to translated guests

[Tamas K Lengyel <tamas.k.lengyel@xxxxxxxxx>]

On Mon, Jun 19, 2017 at 8:52 AM, Julien Grall <julien.grall@xxxxxxx> wrote:
>
>
> On 19/06/17 15:39, Tamas K Lengyel wrote:
>>
>> On Mon, Jun 19, 2017 at 3:09 AM, Julien Grall <julien.grall@xxxxxxx>
>> wrote:
>>>
>>> Hi,
>>>
>>>
>>> On 19/06/17 09:15, Jan Beulich wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 18.06.17 at 21:19, <tamas.k.lengyel@xxxxxxxxx> wrote:
>>>>>
>>>>>
>>>>> On Tue, Apr 4, 2017 at 1:04 PM, Andrew Cooper
>>>>> <andrew.cooper3@xxxxxxxxxx>
>>>>> wrote:
>>>>>>
>>>>>>
>>>>>> On 04/04/17 14:14, Jan Beulich wrote:
>>>>>>>
>>>>>>>
>>>>>>> We shouldn't hand MFN info back from increase-reservation for
>>>>>>> translated domains, just like we don't for populate-physmap and
>>>>>>> memory-exchange. For full symmetry also check for a NULL guest handle
>>>>>>> in populate_physmap() (but note this makes no sense in
>>>>>>> memory_exchange(), as there the array is also an input).
>>>>>>>
>>>>>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>>>>
>>>>>
>>>>>
>>>>> Unfortunately I just had time to do testing with this change and I
>>>>> have to report that introduces a critical regression for my tools.
>>>>> With this change in-place performing increase_reservation on a target
>>>>> domain no longer reports the guest frame number for external tools,
>>>>> thus completely breaking advanced use-cases that require this
>>>>> information to be able to do altp2m gfn remapping. This is a critical
>>>>> step in being able to introduce shadow-pages that are used to hide
>>>>> breakpoints and other memory modifications from the guest.
>>>>
>>>>
>>>>
>>>> While I can see your point, I'm afraid that's not how the
>>>> interface was meant to be used. The mere fact that
>>>> populate-physmap and memory-exchange didn't return the
>>>> MFN(s) suggests to me that you already need to have a way
>>>> to deal with having to find out another way. Or are you
>>>> suggesting you rely on guests not using these interfaces?
>>>>
>>>> As to a solution, I could possibly see us relax the change to
>>>> return the MFN(s) when the current and subject domains differ,
>>>> or even check paging mode of the caller domain instead of the
>>>> subject one (which would mean PVH Dom0 still wouldn't get to
>>>> see them). But if we do, imo we should do this consistently for
>>>> all three operations, rather than just for increase-reservation.
>>>>
>>>>> If at all possible, I would like to request this change not to be part
>>>>> of the 4.9 release.
>>>>
>>>>
>>>>
>>>> Hmm, it's been there for all of the RCs, so I'm not really happy
>>>> to consider the option of reverting at this point in time. But
>>>> Julien will have the final say anyway.
>>>
>>>
>>>
>>> I am a bit confuse with the description of the problem. I understood
>>> "guest
>>> frame number" as GFN. But AFAICT, this hypercall was returning MFN even
>>> for
>>> HVM guests. So how this change is breaking altp2m remapping?
>>
>>
>> For HVM guests this hypercall returns a GFN that can subsequently be
>> populated into the guest physmap:
>>
>> xc_domain_increase_reservation_exact(xch, domid, 1, 0, 0, &new_gfn);
>> xc_domain_populate_physmap_exact(xch, domid, 1, 0, 0, &new_gfn);
>
>
> I am sorry, I can't see how this can return a GFN for the HVM. Looking at
> the implementation of increase_reservation in Xen:
>
> mfn = page_to_mfn(page);
> if ( unlikely(__copy_to_guest_offset(a->extent_list, i, &mfn, 1)) )
>   goto out;
>
> This is an MFN and not a GFN. Except the strict check before, the code has
> not change for a while.
>
> AFAICT, the purpose of increase_reservation is not to allocate a new GFN, it
> will just allocate the host memory for it. At least on ARM we have nothing
> to say "this GFN region is free". I would be surprised that such things
> exists on x86.
>

It returns memory that can be mapped into the guest physmap
subsequently. So I have been referring to it as a GFN that is not
mapped into the physmap - similar to the magic ring pages when they
are in use.

Tamas

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