Re: [Xen-devel] [PATCH v4] altp2m: Allow the hostp2m entries to be of type p2m_ram_shared

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Mon, Jun 13, 2016 at 3:28 AM, George Dunlap <george.dunlap@xxxxxxxxxx> wrote:
> On 11/06/16 18:55, Tamas K Lengyel wrote:
>> On Thu, May 26, 2016 at 10:17 AM, Tamas K Lengyel <tamas@xxxxxxxxxxxxx> 
>> wrote:
>>>
>>> On May 26, 2016 04:40, "George Dunlap" <george.dunlap@xxxxxxxxxx> wrote:
>>>>
>>>> On 26/05/16 04:55, Tamas K Lengyel wrote:
>>>>> Move sharing locks above altp2m to avoid locking order violation. Allow
>>>>> applying altp2m mem_access settings when the hostp2m entries are shared.
>>>>> Also, do not trigger PoD for hostp2m when setting altp2m mem_access to
>>>>> be
>>>>> in-line with non-altp2m mem_access path. Also allow gfn remapping with
>>>>> p2m_ram_shared type gfns in altp2m views.
>>>>>
>>>>> When using mem_sharing in combination with altp2m, unsharing events
>>>>> overwrite
>>>>> altp2m entries with that of the hostp2m (both remapped entries and
>>>>> mem_access
>>>>> settings). User should take precaution to not share pages where this
>>>>> behavior
>>>>> is undesired.
>>>>
>>>> I'm afraid this is not acceptable.  How could this ever be even remotely
>>>> usable?  If this is a necessary side effect of sharing, then I think the
>>>> original functionality, of un-sharing when setting an altp2m entry is
>>>> the only option (and not allowing sharing to happen when an altp2m is
>>>> present for a particular gfn).
>>>>
>>>> Hmm, but actually this also brings up another tricky point: In an altp2m
>>>> you can change the mfn which backs a gfn.  This would need to be handled
>>>> properly in the reverse map, which it doesn't look like it is at the
>>>> moment.
>>>>
>>>> On the whole, I think if you're going to allow a single gfn to be
>>>> simultaneously shared and allow an altp2m for it, you're going to need
>>>> to do a lot more work.
>>>>
>>>> (Sorry for not catching a lot of this before...)
>>>>
>>>
>>> Well this patch resolves the locking order violation and allows the
>>> xen-access tool's altp2m tests to pass, so it does improve on the current
>>> situation which is a hypervisor crash. To help with the override issue the
>>> user can apply W mem_access permission on the shared hostp2m entries. That
>>> way they get notification through vm_event of the event that leads to
>>> unsharing and can then reapply the altp2m changes. So IMHO this patch is
>>> already quite workable and while it requires more setup from the userside,
>>> the VMM side is OK with this change.
>>
>> Ping. Can I get an update on what the verdict is on this patch?
>
> To get a proper verdict requires actually taking a deeper look and
> thinking carefully about things :-)  Sorry for the delay -- hopefully I
> can get to this this week.

Thanks, no rush ;) Just wanted to keep the discussion on the radar.
The only caveat with what I proposed above with using mem_access to
get notified of unsharing is that the mem_access notification is
currently sent before the unsharing in hvm/hvm.c. This means the user
getting the mem_access notification has to also do the unsharing
before reapplying the altp2m changes as well. It can be done so it
would still work just fine, just has to keep in mind. We could also
swap around the order of things so unsharing happens before the
mem_access notification happens but I don't think it's necessary.

Tamas

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