Re: [Xen-devel] [PATCH V6] x86/altp2m: Hypercall to set altp2m view visibility

[Isaila Alexandru <aisaila@bitdefender.com>]

On 27.03.2020 04:30, Tian, Kevin wrote:
>> From: Isaila Alexandru <aisaila@bitdefender.com>
>> Sent: Tuesday, March 24, 2020 6:46 PM
>>
>>
>> Hi Kevin and sorry for the long reply time,
>>
>> On 10.03.2020 04:04,  sTian, Kevin wrote:
>>>> From: Alexandru Stefan ISAILA <aisaila@bitdefender.com>
>>>> Sent: Tuesday, March 3, 2020 8:23 PM
>>>>
>>>> At this moment a guest can call vmfunc to change the altp2m view. This
>>>> should be limited in order to avoid any unwanted view switch.
>>>
>>> I look forward to more elaboration of the motivation, especially for one
>>> who doesn't track altp2m closely like me. For example, do_altp2m_op
>>> mentions three modes: external, internal, coordinated. Then is this patch
>>> trying to limit the view switch in all three modes or just one of them?
>>> from the definition clearly external disallows guest to change any view
>>> (then why do we want per-view visibility control) while the latter two
>>> both allows guest to switch the view. later you noted some exception
>>> with mixed (internal) mode. then is this restriction pushed just for
>>> limited (coordinated) mode?
>>>
>>
>> As you stated, there are some exceptions with mixed (internal) mode.
>> This restriction is clearly used for coordinated mode but it also
>> restricts view switching in the external mode as well. I had a good
>> example to start with, let's say we have one external agent in dom0 that
>> uses view1 and view2 and the logic requires the switch between the
>> views. At this point VMFUNC is available to the guest so with a simple
>> asm code it can witch to view 0. At this time the external agent is not
>> aware that the view has switched and further more view0 was not supposed
>> to be in the main logic so it crashes. This example can be extended to
>> any number of views. I hope it can paint a more clear picture of what
>> this patch is trying to achive.
> 
> Can VMFUNC be hidden and disabled when external mode is being used?
> or is it because the mode can be dynamically switched after a VM is
> launched so you have to restrict the views in this way?

Like you said, there is a problem if the mode is dynamically switched.

>>
>>> btw I'm not sure why altp2m invents two names per mode, and their
>>> mapping looks a bit weird. e.g. isn't 'coordinated' mode sound more
>>> like 'mixed' mode?
>>
>> Yes that is true, it si a bit weird.
>>
>>>
>>>>
>>>> The new xc_altp2m_set_visibility() solves this by making views invisible
>>>> to vmfunc.
>>>
>>> if one doesn't want to make view visible to vmfunc, why can't he just
>>> avoids registering the view at the first place? Are you aiming for a
>>> scenario that dom0 may register 10 views, with 5 views visible to
>>> vmfunc with the other 5 views switched by dom0 itself?
>>
>> That is one scenario, another can be that dom0 has a number of views
>> created and in some time it wants to be sure that only some of the views
>> can be switched, saving the rest and making them visible when the time
>> is right. Sure the example given up is another example.
>>
> 
> Can you update the patch description and resend? I'll take another look then.

Ok, I will update the description for the next version.

Thanks,
Alex