Re: [Xen-devel] [PATCH V2] x86/altp2m: Added xc_altp2m_set_mem_access_multi()

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 03/13/2017 02:19 PM, Jan Beulich wrote:
>>>> On 13.03.17 at 13:01, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>> On 03/10/2017 09:01 PM, Tamas K Lengyel wrote:
>>> On Fri, Mar 10, 2017 at 4:21 AM, Andrew Cooper
>>> <andrew.cooper3@xxxxxxxxxx> wrote:
>>>> On 10/03/17 07:34, Jan Beulich wrote:
>>>>>>>> On 09.03.17 at 18:29, <tamas@xxxxxxxxxxxxx> wrote:
>>>>>> On Thu, Mar 9, 2017 at 9:56 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>>>> However - is this interface supposed to be usable by a guest on itself?
>>>>>>> Arguably the same question would apply to some of the other sub-ops
>>>>>>> too, but anyway.
>>>>>> AFAIK the only op the guest would use on itself is
>>>>>> HVMOP_altp2m_vcpu_enable_notify.
>>>>> Which then means we should move all of them out of here, into a
>>>>> suitable domctl. That will in turn reduce the scope of the bogus
>>>>> interface versioning, which Andrew did point out, quite a bit.
>>>>
>>>> The original usecase for altp2m was for an entirely in-guest agent,
>>>> which is why they got in as hvmops to start with.  I don't think it is
>>>> wise to break that.
>>>>
>>>> I think there needs to be slightly finer grain control, identifying
>>>> whether a domain may use altp2m, and whether it may configure altp2m
>>>> permissions itself.
>>>>
>>>> The nature of altp2m means that using EPTP switching/etc necessarily can
>>>> only happen from inside guest context, but whether you trust the domain
>>>> to make adjustments to the permissions itself depends on your usecase
>>>> and threat model.
>>>>
>>>
>>> So I'm actively using EPT switching and gfn remapping from a
>>> privileged monitor domain (not with VMFUNC). My entire usecase for
>>> altp2m is purely external without any in-guest agents. In fact, I have
>>> to deploy a custom XSM policy to blacklist altp2mhvm_op being issued
>>> from the guest.
>>>
>>> The reason I mentioned HVMOP_altp2m_vcpu_enable_notify as being the
>>> only one I believe that is only accessible from within the guest is
>>> this distinction in arch/x86/hvm/hvm.c:
>>>
>>>     d = ( a.cmd != HVMOP_altp2m_vcpu_enable_notify ) ?
>>>         rcu_lock_domain_by_any_id(a.domain) : rcu_lock_current_domain();
>>>
>>> For the other ops I'm not sure if they were really required to be
>>> accessible from within the guest or not. I'm not even sure using them
>>> would work from the guest with the above check in place. However, if
>>> they do work from the guest then I have no idea how it was supposed to
>>> work for security purposes as any application in that guest could just
>>> issue a hypercall to manipulate it or even turn it off.
>>
>> Thanks to all for the replies! What I'm taking away from this is:
>>
>> 1. The hypercall continuation model proposed by Tamas is fine for HVMOPs.
>>
>> 2. But we're not sure if these should be DOMCTLs or HVMOPs (except for
>> HVMOP_altp2m_vcpu_enable_notify).
>>
>> 3. If we keep them as HVMOPs, the code for handling the set_mem_access()
>> part needs to be duplicated, both for the hypercall continuation / HVMOP
>> hypercall structure part, and for the compat part (since the _multi()
>> function sends arrays / handles to the hypervisor).
>>
>> So an agreement on point 2 is required before being able to proceed.
> 
> I think as long as there's no need for the guest to use an operation
> on itself, it should not be a hvmop. After all, if you make it a domctl
> now and later find a need for it to be called by the guest, we can
> always replace the domctl by a hvmop. If, however, you start out
> with a hvmop, we'll be bound to be supporting it virtually forever.

Since we're on this point, IMHO the xc_altp2m_ prefixed versions of
set_mem_access() and set_mem_access_multi() shouldn't exist at all.
Plain xc_set_mem_access() and xc_set_mem_access_multi() (as DOMCTLs)
should be enough, as long as we also add the view_id as an
extra-parameter, where view ID 0 is (already) the default EPT view.

As it stands now, xc_set_mem_access() can do less than
xc_altp2m_set_mem_access() in that its view ID is always 0, but more
than xc_altp2m_set_mem_access() in that it is able to set more than one
page with a single hypercall, while the underlying hypervisor code is
the same.

Maybe I'm missing something design-wise (obviously if these really do
need to be HVMOPs a separate libxc function is required). Maybe the
altp2m maintainers have a different view of the matter.


Thanks,
Razvan

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