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 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);

...
Copy page contents from old_gfn to new_gfn and inject breakpoints,
make other memory modifications
...

xc_altp2m_change_gfn(xch, domid, altp2m_id, old_gfn, new_gfn);

Without being able to introduce a new gfn into the HVM guest's
physmap, we are unable to create a shadow page. It doesn't break
altp2m remapping itself, it breaks a per-requisite step in introducing
the page to remap to.

Tamas

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