| Message ID | 1489052332-4983-1-git-send-email-rcojocaru@bitdefender.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 09.03.17 at 10:38, <rcojocaru@bitdefender.com> wrote: > @@ -4535,6 +4536,30 @@ static int do_altp2m_op( > a.u.set_mem_access.view); > break; > > + case HVMOP_altp2m_set_mem_access_multi: > + if ( a.u.set_mem_access_multi.pad || > + a.u.set_mem_access_multi.opaque >= a.u.set_mem_access_multi.nr ) > + { > + rc = -EINVAL; > + break; > + } > + rc = p2m_set_mem_access_multi(d, a.u.set_mem_access_multi.pfn_list, > + a.u.set_mem_access_multi.access_list, > + a.u.set_mem_access_multi.nr, > + a.u.set_mem_access_multi.opaque, > + MEMOP_CMD_MASK, > + a.u.set_mem_access_multi.view); > + if ( rc > 0 ) > + { > + a.u.set_mem_access_multi.opaque = rc; > + if ( __copy_to_guest(arg, &a, 1) ) > + rc = -EFAULT; > + else > + rc = hypercall_create_continuation(__HYPERVISOR_hvm_op, "lh", > + HVMOP_altp2m, arg); > + } > + break; Okay, so this is a hvmop, in which case I'm fine with the continuation model used. 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. > --- a/xen/include/public/hvm/hvm_op.h > +++ b/xen/include/public/hvm/hvm_op.h > @@ -231,6 +231,23 @@ struct xen_hvm_altp2m_set_mem_access { > typedef struct xen_hvm_altp2m_set_mem_access xen_hvm_altp2m_set_mem_access_t; > DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_set_mem_access_t); > > +struct xen_hvm_altp2m_set_mem_access_multi { > + /* view */ > + uint16_t view; > + uint16_t pad; > + /* Number of pages */ > + uint32_t nr; > + /* Used for continuation purposes */ > + uint64_t opaque; > + /* List of pfns to set access for */ > + XEN_GUEST_HANDLE(const_uint64) pfn_list; > + /* Corresponding list of access settings for pfn_list */ > + XEN_GUEST_HANDLE(const_uint8) access_list; I'm afraid these handles aren't going to work for a 32-bit guest. Note how no other hvmop uses handles. Jan
On 03/09/2017 06:56 PM, Jan Beulich wrote: >>>> On 09.03.17 at 10:38, <rcojocaru@bitdefender.com> wrote: >> @@ -4535,6 +4536,30 @@ static int do_altp2m_op( >> a.u.set_mem_access.view); >> break; >> >> + case HVMOP_altp2m_set_mem_access_multi: >> + if ( a.u.set_mem_access_multi.pad || >> + a.u.set_mem_access_multi.opaque >= a.u.set_mem_access_multi.nr ) >> + { >> + rc = -EINVAL; >> + break; >> + } >> + rc = p2m_set_mem_access_multi(d, a.u.set_mem_access_multi.pfn_list, >> + a.u.set_mem_access_multi.access_list, >> + a.u.set_mem_access_multi.nr, >> + a.u.set_mem_access_multi.opaque, >> + MEMOP_CMD_MASK, >> + a.u.set_mem_access_multi.view); >> + if ( rc > 0 ) >> + { >> + a.u.set_mem_access_multi.opaque = rc; >> + if ( __copy_to_guest(arg, &a, 1) ) >> + rc = -EFAULT; >> + else >> + rc = hypercall_create_continuation(__HYPERVISOR_hvm_op, "lh", >> + HVMOP_altp2m, arg); >> + } >> + break; > > Okay, so this is a hvmop, in which case I'm fine with the continuation > model used. > > 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-op > too, but anyway. Not for any of our use cases. The whole point is for dom0 (or another suitably privileged domain) to monitor another guest that consequently can't, by design, evade detection of bad behaviour by acting at a higher privilege level than the protection software. It wouldn't make sense for a domain to be doing this on itself. Maybe Tamas has something in mind, but the short answer is no, it's not. >> --- a/xen/include/public/hvm/hvm_op.h >> +++ b/xen/include/public/hvm/hvm_op.ht >> @@ -231,6 +231,23 @@ struct xen_hvm_altp2m_set_mem_access { >> typedef struct xen_hvm_altp2m_set_mem_access xen_hvm_altp2m_set_mem_access_t; >> DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_set_mem_access_t); >> >> +struct xen_hvm_altp2m_set_mem_access_multi { >> + /* view */ >> + uint16_t view; >> + uint16_t pad; >> + /* Number of pages */ >> + uint32_t nr; >> + /* Used for continuation purposes */ >> + uint64_t opaque; >> + /* List of pfns to set access for */ >> + XEN_GUEST_HANDLE(const_uint64) pfn_list; >> + /* Corresponding list of access settings for pfn_list */ >> + XEN_GUEST_HANDLE(const_uint8) access_list; > > I'm afraid these handles aren't going to work for a 32-bit guest. Note > how no other hvmop uses handles. Right, I guess I'll have to pass these through the compat code then, like the xc_set_mem_access_multi() code does. I'll take a look at what that entails. Thanks, Razvan
On Thu, Mar 9, 2017 at 9:56 AM, Jan Beulich <JBeulich@suse.com> wrote: >>>> On 09.03.17 at 10:38, <rcojocaru@bitdefender.com> wrote: >> @@ -4535,6 +4536,30 @@ static int do_altp2m_op( >> a.u.set_mem_access.view); >> break; >> >> + case HVMOP_altp2m_set_mem_access_multi: >> + if ( a.u.set_mem_access_multi.pad || >> + a.u.set_mem_access_multi.opaque >= a.u.set_mem_access_multi.nr ) >> + { >> + rc = -EINVAL; >> + break; >> + } >> + rc = p2m_set_mem_access_multi(d, a.u.set_mem_access_multi.pfn_list, >> + a.u.set_mem_access_multi.access_list, >> + a.u.set_mem_access_multi.nr, >> + a.u.set_mem_access_multi.opaque, >> + MEMOP_CMD_MASK, >> + a.u.set_mem_access_multi.view); >> + if ( rc > 0 ) >> + { >> + a.u.set_mem_access_multi.opaque = rc; >> + if ( __copy_to_guest(arg, &a, 1) ) >> + rc = -EFAULT; >> + else >> + rc = hypercall_create_continuation(__HYPERVISOR_hvm_op, "lh", >> + HVMOP_altp2m, arg); >> + } >> + break; > > Okay, so this is a hvmop, in which case I'm fine with the continuation > model used. > > 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. Tamas
>>> On 09.03.17 at 18:15, <rcojocaru@bitdefender.com> wrote: > On 03/09/2017 06:56 PM, Jan Beulich wrote: >>>>> On 09.03.17 at 10:38, <rcojocaru@bitdefender.com> wrote: >>> @@ -4535,6 +4536,30 @@ static int do_altp2m_op( >>> a.u.set_mem_access.view); >>> break; >>> >>> + case HVMOP_altp2m_set_mem_access_multi: >>> + if ( a.u.set_mem_access_multi.pad || >>> + a.u.set_mem_access_multi.opaque >= a.u.set_mem_access_multi.nr > ) >>> + { >>> + rc = -EINVAL; >>> + break; >>> + } >>> + rc = p2m_set_mem_access_multi(d, a.u.set_mem_access_multi.pfn_list, >>> + a.u.set_mem_access_multi.access_list, >>> + a.u.set_mem_access_multi.nr, >>> + a.u.set_mem_access_multi.opaque, >>> + MEMOP_CMD_MASK, >>> + a.u.set_mem_access_multi.view); >>> + if ( rc > 0 ) >>> + { >>> + a.u.set_mem_access_multi.opaque = rc; >>> + if ( __copy_to_guest(arg, &a, 1) ) >>> + rc = -EFAULT; >>> + else >>> + rc = hypercall_create_continuation(__HYPERVISOR_hvm_op, > "lh", >>> + HVMOP_altp2m, arg); >>> + } >>> + break; >> >> Okay, so this is a hvmop, in which case I'm fine with the continuation >> model used. >> >> 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-op >> too, but anyway. > > Not for any of our use cases. The whole point is for dom0 (or another > suitably privileged domain) to monitor another guest that consequently > can't, by design, evade detection of bad behaviour by acting at a higher > privilege level than the protection software. It wouldn't make sense for > a domain to be doing this on itself. In which case this should be a domctl. >>> --- a/xen/include/public/hvm/hvm_op.h >>> +++ b/xen/include/public/hvm/hvm_op.ht >>> @@ -231,6 +231,23 @@ struct xen_hvm_altp2m_set_mem_access { >>> typedef struct xen_hvm_altp2m_set_mem_access xen_hvm_altp2m_set_mem_access_t; >>> DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_set_mem_access_t); >>> >>> +struct xen_hvm_altp2m_set_mem_access_multi { >>> + /* view */ >>> + uint16_t view; >>> + uint16_t pad; >>> + /* Number of pages */ >>> + uint32_t nr; >>> + /* Used for continuation purposes */ >>> + uint64_t opaque; >>> + /* List of pfns to set access for */ >>> + XEN_GUEST_HANDLE(const_uint64) pfn_list; >>> + /* Corresponding list of access settings for pfn_list */ >>> + XEN_GUEST_HANDLE(const_uint8) access_list; >> >> I'm afraid these handles aren't going to work for a 32-bit guest. Note >> how no other hvmop uses handles. > > Right, I guess I'll have to pass these through the compat code then, > like the xc_set_mem_access_multi() code does. I'll take a look at what > that entails. Which in turn means you don't need to go through the hassles of this. Jan
>>> On 09.03.17 at 18:29, <tamas@tklengyel.com> wrote: > On Thu, Mar 9, 2017 at 9:56 AM, Jan Beulich <JBeulich@suse.com> 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. Jan
On 03/10/2017 09:31 AM, Jan Beulich wrote: >>>> On 09.03.17 at 18:15, <rcojocaru@bitdefender.com> wrote: >> On 03/09/2017 06:56 PM, Jan Beulich wrote: >>>>>> On 09.03.17 at 10:38, <rcojocaru@bitdefender.com> wrote: >>>> @@ -4535,6 +4536,30 @@ static int do_altp2m_op( >>>> a.u.set_mem_access.view); >>>> break; >>>> >>>> + case HVMOP_altp2m_set_mem_access_multi: >>>> + if ( a.u.set_mem_access_multi.pad || >>>> + a.u.set_mem_access_multi.opaque >= a.u.set_mem_access_multi.nr >> ) >>>> + { >>>> + rc = -EINVAL; >>>> + break; >>>> + } >>>> + rc = p2m_set_mem_access_multi(d, a.u.set_mem_access_multi.pfn_list, >>>> + a.u.set_mem_access_multi.access_list, >>>> + a.u.set_mem_access_multi.nr, >>>> + a.u.set_mem_access_multi.opaque, >>>> + MEMOP_CMD_MASK, >>>> + a.u.set_mem_access_multi.view); >>>> + if ( rc > 0 ) >>>> + { >>>> + a.u.set_mem_access_multi.opaque = rc; >>>> + if ( __copy_to_guest(arg, &a, 1) ) >>>> + rc = -EFAULT; >>>> + else >>>> + rc = hypercall_create_continuation(__HYPERVISOR_hvm_op, >> "lh", >>>> + HVMOP_altp2m, arg); >>>> + } >>>> + break; >>> >>> Okay, so this is a hvmop, in which case I'm fine with the continuation >>> model used. >>> >>> 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-op >>> too, but anyway. >> >> Not for any of our use cases. The whole point is for dom0 (or another >> suitably privileged domain) to monitor another guest that consequently >> can't, by design, evade detection of bad behaviour by acting at a higher >> privilege level than the protection software. It wouldn't make sense for >> a domain to be doing this on itself. > > In which case this should be a domctl. Fair enough, if nobody objects I'll then just modify XENMEM_access_op_set_access_multi to take a view_id as well an just piggyback on that. It already does the right thing underneath. Thanks, Razvan
On 10/03/17 07:34, Jan Beulich wrote: >>>> On 09.03.17 at 18:29, <tamas@tklengyel.com> wrote: >> On Thu, Mar 9, 2017 at 9:56 AM, Jan Beulich <JBeulich@suse.com> 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. ~Andrew
On Fri, Mar 10, 2017 at 4:21 AM, Andrew Cooper <andrew.cooper3@citrix.com> wrote: > On 10/03/17 07:34, Jan Beulich wrote: >>>>> On 09.03.17 at 18:29, <tamas@tklengyel.com> wrote: >>> On Thu, Mar 9, 2017 at 9:56 AM, Jan Beulich <JBeulich@suse.com> 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. Tamas
On 03/10/2017 09:01 PM, Tamas K Lengyel wrote: > On Fri, Mar 10, 2017 at 4:21 AM, Andrew Cooper > <andrew.cooper3@citrix.com> wrote: >> On 10/03/17 07:34, Jan Beulich wrote: >>>>>> On 09.03.17 at 18:29, <tamas@tklengyel.com> wrote: >>>> On Thu, Mar 9, 2017 at 9:56 AM, Jan Beulich <JBeulich@suse.com> 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. Thanks, Razvan
>>> On 13.03.17 at 13:01, <rcojocaru@bitdefender.com> 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@citrix.com> wrote: >>> On 10/03/17 07:34, Jan Beulich wrote: >>>>>>> On 09.03.17 at 18:29, <tamas@tklengyel.com> wrote: >>>>> On Thu, Mar 9, 2017 at 9:56 AM, Jan Beulich <JBeulich@suse.com> 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. Jan
On 03/13/2017 02:19 PM, Jan Beulich wrote: >>>> On 13.03.17 at 13:01, <rcojocaru@bitdefender.com> 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@citrix.com> wrote: >>>> On 10/03/17 07:34, Jan Beulich wrote: >>>>>>>> On 09.03.17 at 18:29, <tamas@tklengyel.com> wrote: >>>>>> On Thu, Mar 9, 2017 at 9:56 AM, Jan Beulich <JBeulich@suse.com> 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
>>> On 13.03.17 at 13:29, <rcojocaru@bitdefender.com> wrote: > On 03/13/2017 02:19 PM, Jan Beulich wrote: >> 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. "altp2m maintainers"? Maintainership is covered by the x86/mm/ entry afaict, so perhaps you mean the original altp2m authors. In any event - you didn't Cc anyone of them. Jan
On 03/13/2017 02:40 PM, Jan Beulich wrote: >>>> On 13.03.17 at 13:29, <rcojocaru@bitdefender.com> wrote: >> On 03/13/2017 02:19 PM, Jan Beulich wrote: >>> 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. > > "altp2m maintainers"? Maintainership is covered by the x86/mm/ > entry afaict, so perhaps you mean the original altp2m authors. In > any event - you didn't Cc anyone of them. I've used scripts/get_maintainer.pl on the patch, and CCd everyone in the output. Consulting the original authors is a very good idea, I've CCd Ravi Sahita here, perhaps he can help us out. Thanks, Razvan
On Mon, Mar 13, 2017 at 6:29 AM, Razvan Cojocaru <rcojocaru@bitdefender.com> wrote: > On 03/13/2017 02:19 PM, Jan Beulich wrote: >>>>> On 13.03.17 at 13:01, <rcojocaru@bitdefender.com> 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@citrix.com> wrote: >>>>> On 10/03/17 07:34, Jan Beulich wrote: >>>>>>>>> On 09.03.17 at 18:29, <tamas@tklengyel.com> wrote: >>>>>>> On Thu, Mar 9, 2017 at 9:56 AM, Jan Beulich <JBeulich@suse.com> 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. Yeap, I remember suggesting that the two set_mem_access interfaces should be merged when altp2m was being contributed. Unfortunately we were not yet maintainers to make that suggestion a requirement so it was let in without that change.. > > 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. > I think altp2m is still considered experimental at this point.. With that said I'm not sure if the altp2m HVMOPs need to be considered as set-in-stone as other HVMOPs might be. I would also like to see the mem_access setting interfaces merged. Tamas
On 03/13/2017 07:17 PM, Tamas K Lengyel wrote: > On Mon, Mar 13, 2017 at 6:29 AM, Razvan Cojocaru > <rcojocaru@bitdefender.com> wrote: >> On 03/13/2017 02:19 PM, Jan Beulich wrote: >>>>>> On 13.03.17 at 13:01, <rcojocaru@bitdefender.com> 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@citrix.com> wrote: >>>>>> On 10/03/17 07:34, Jan Beulich wrote: >>>>>>>>>> On 09.03.17 at 18:29, <tamas@tklengyel.com> wrote: >>>>>>>> On Thu, Mar 9, 2017 at 9:56 AM, Jan Beulich <JBeulich@suse.com> 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. > > Yeap, I remember suggesting that the two set_mem_access interfaces > should be merged when altp2m was being contributed. Unfortunately we > were not yet maintainers to make that suggestion a requirement so it > was let in without that change.. > >> >> 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. >> > > I think altp2m is still considered experimental at this point.. With > that said I'm not sure if the altp2m HVMOPs need to be considered as > set-in-stone as other HVMOPs might be. I would also like to see the > mem_access setting interfaces merged. Then I'll rework the patch to remove the special altp2m functions and add an extra parameter to the regular xc_set_mem_access() functions (while also increasing the DOMCTL version macro value). Unless somebody objects, of course. Thanks, Razvan
On 03/09/2017 06:56 PM, Jan Beulich wrote: >>>> On 09.03.17 at 10:38, <rcojocaru@bitdefender.com> wrote: >> @@ -4535,6 +4536,30 @@ static int do_altp2m_op( >> a.u.set_mem_access.view); >> break; >> >> + case HVMOP_altp2m_set_mem_access_multi: >> + if ( a.u.set_mem_access_multi.pad || >> + a.u.set_mem_access_multi.opaque >= a.u.set_mem_access_multi.nr ) >> + { >> + rc = -EINVAL; >> + break; >> + } >> + rc = p2m_set_mem_access_multi(d, a.u.set_mem_access_multi.pfn_list, >> + a.u.set_mem_access_multi.access_list, >> + a.u.set_mem_access_multi.nr, >> + a.u.set_mem_access_multi.opaque, >> + MEMOP_CMD_MASK, >> + a.u.set_mem_access_multi.view); >> + if ( rc > 0 ) >> + { >> + a.u.set_mem_access_multi.opaque = rc; >> + if ( __copy_to_guest(arg, &a, 1) ) >> + rc = -EFAULT; >> + else >> + rc = hypercall_create_continuation(__HYPERVISOR_hvm_op, "lh", >> + HVMOP_altp2m, arg); >> + } >> + break; > > Okay, so this is a hvmop, in which case I'm fine with the continuation > model used. > > 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. > >> --- a/xen/include/public/hvm/hvm_op.h >> +++ b/xen/include/public/hvm/hvm_op.h >> @@ -231,6 +231,23 @@ struct xen_hvm_altp2m_set_mem_access { >> typedef struct xen_hvm_altp2m_set_mem_access xen_hvm_altp2m_set_mem_access_t; >> DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_set_mem_access_t); >> >> +struct xen_hvm_altp2m_set_mem_access_multi { >> + /* view */ >> + uint16_t view; >> + uint16_t pad; >> + /* Number of pages */ >> + uint32_t nr; >> + /* Used for continuation purposes */ >> + uint64_t opaque; >> + /* List of pfns to set access for */ >> + XEN_GUEST_HANDLE(const_uint64) pfn_list; >> + /* Corresponding list of access settings for pfn_list */ >> + XEN_GUEST_HANDLE(const_uint8) access_list; > > I'm afraid these handles aren't going to work for a 32-bit guest. Note > how no other hvmop uses handles. OK, so at this point, since Ravi has not expressed a preference, but Andrew has said that these should be accessible from the guest as well, I assume we should move forward with this as a HVMOP, addressing the comment above regarding compatibility. (Just to help the discussion, here is the original patch: https://patchwork.kernel.org/patch/9612799/). I do prefer the DOMCTL version, but of course the operation will not be available for the guest then and we may just as well make it a HVMOP from the beginnig. If there are any objections, please let me know. Thanks, Razvan
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h index a48981a..645b5bd 100644 --- a/tools/libxc/include/xenctrl.h +++ b/tools/libxc/include/xenctrl.h @@ -1903,6 +1903,9 @@ int xc_altp2m_switch_to_view(xc_interface *handle, domid_t domid, int xc_altp2m_set_mem_access(xc_interface *handle, domid_t domid, uint16_t view_id, xen_pfn_t gfn, xenmem_access_t access); +int xc_altp2m_set_mem_access_multi(xc_interface *handle, domid_t domid, + uint16_t view_id, uint8_t *access, + uint64_t *pages, uint32_t nr); int xc_altp2m_change_gfn(xc_interface *handle, domid_t domid, uint16_t view_id, xen_pfn_t old_gfn, xen_pfn_t new_gfn); diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c index 0639632..f202ca1 100644 --- a/tools/libxc/xc_altp2m.c +++ b/tools/libxc/xc_altp2m.c @@ -188,6 +188,47 @@ int xc_altp2m_set_mem_access(xc_interface *handle, domid_t domid, return rc; } +int xc_altp2m_set_mem_access_multi(xc_interface *xch, domid_t domid, + uint16_t view_id, uint8_t *access, + uint64_t *pages, uint32_t nr) +{ + int rc; + + DECLARE_HYPERCALL_BUFFER(xen_hvm_altp2m_op_t, arg); + DECLARE_HYPERCALL_BOUNCE(access, nr, XC_HYPERCALL_BUFFER_BOUNCE_IN); + DECLARE_HYPERCALL_BOUNCE(pages, nr * sizeof(uint64_t), + XC_HYPERCALL_BUFFER_BOUNCE_IN); + + arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg)); + if ( arg == NULL ) + return -1; + + arg->version = HVMOP_ALTP2M_INTERFACE_VERSION; + arg->cmd = HVMOP_altp2m_set_mem_access_multi; + arg->domain = domid; + arg->u.set_mem_access_multi.view = view_id; + arg->u.set_mem_access_multi.nr = nr; + + if ( xc_hypercall_bounce_pre(xch, pages) || + xc_hypercall_bounce_pre(xch, access) ) + { + PERROR("Could not bounce memory for HVMOP_altp2m_set_mem_access_multi"); + return -1; + } + + set_xen_guest_handle(arg->u.set_mem_access_multi.pfn_list, pages); + set_xen_guest_handle(arg->u.set_mem_access_multi.access_list, access); + + rc = xencall2(xch->xcall, __HYPERVISOR_hvm_op, HVMOP_altp2m, + HYPERCALL_BUFFER_AS_ARG(arg)); + + xc_hypercall_buffer_free(xch, arg); + xc_hypercall_bounce_post(xch, access); + xc_hypercall_bounce_post(xch, pages); + + return rc; +} + int xc_altp2m_change_gfn(xc_interface *handle, domid_t domid, uint16_t view_id, xen_pfn_t old_gfn, xen_pfn_t new_gfn) diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index ccfae4f..12b34da 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -4419,6 +4419,7 @@ static int do_altp2m_op( case HVMOP_altp2m_destroy_p2m: case HVMOP_altp2m_switch_p2m: case HVMOP_altp2m_set_mem_access: + case HVMOP_altp2m_set_mem_access_multi: case HVMOP_altp2m_change_gfn: break; default: @@ -4535,6 +4536,30 @@ static int do_altp2m_op( a.u.set_mem_access.view); break; + case HVMOP_altp2m_set_mem_access_multi: + if ( a.u.set_mem_access_multi.pad || + a.u.set_mem_access_multi.opaque >= a.u.set_mem_access_multi.nr ) + { + rc = -EINVAL; + break; + } + rc = p2m_set_mem_access_multi(d, a.u.set_mem_access_multi.pfn_list, + a.u.set_mem_access_multi.access_list, + a.u.set_mem_access_multi.nr, + a.u.set_mem_access_multi.opaque, + MEMOP_CMD_MASK, + a.u.set_mem_access_multi.view); + if ( rc > 0 ) + { + a.u.set_mem_access_multi.opaque = rc; + if ( __copy_to_guest(arg, &a, 1) ) + rc = -EFAULT; + else + rc = hypercall_create_continuation(__HYPERVISOR_hvm_op, "lh", + HVMOP_altp2m, arg); + } + break; + case HVMOP_altp2m_change_gfn: if ( a.u.change_gfn.pad1 || a.u.change_gfn.pad2 ) rc = -EINVAL; diff --git a/xen/include/public/hvm/hvm_op.h b/xen/include/public/hvm/hvm_op.h index bc00ef0..f2429f8 100644 --- a/xen/include/public/hvm/hvm_op.h +++ b/xen/include/public/hvm/hvm_op.h @@ -231,6 +231,23 @@ struct xen_hvm_altp2m_set_mem_access { typedef struct xen_hvm_altp2m_set_mem_access xen_hvm_altp2m_set_mem_access_t; DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_set_mem_access_t); +struct xen_hvm_altp2m_set_mem_access_multi { + /* view */ + uint16_t view; + uint16_t pad; + /* Number of pages */ + uint32_t nr; + /* Used for continuation purposes */ + uint64_t opaque; + /* List of pfns to set access for */ + XEN_GUEST_HANDLE(const_uint64) pfn_list; + /* Corresponding list of access settings for pfn_list */ + XEN_GUEST_HANDLE(const_uint8) access_list; +}; +typedef struct xen_hvm_altp2m_set_mem_access_multi + xen_hvm_altp2m_set_mem_access_multi_t; +DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_set_mem_access_multi_t); + struct xen_hvm_altp2m_change_gfn { /* view */ uint16_t view; @@ -262,15 +279,18 @@ struct xen_hvm_altp2m_op { #define HVMOP_altp2m_set_mem_access 7 /* Change a p2m entry to have a different gfn->mfn mapping */ #define HVMOP_altp2m_change_gfn 8 +/* Set access for an array of pages */ +#define HVMOP_altp2m_set_mem_access_multi 9 domid_t domain; uint16_t pad1; uint32_t pad2; union { - struct xen_hvm_altp2m_domain_state domain_state; - struct xen_hvm_altp2m_vcpu_enable_notify enable_notify; - struct xen_hvm_altp2m_view view; - struct xen_hvm_altp2m_set_mem_access set_mem_access; - struct xen_hvm_altp2m_change_gfn change_gfn; + struct xen_hvm_altp2m_domain_state domain_state; + struct xen_hvm_altp2m_vcpu_enable_notify enable_notify; + struct xen_hvm_altp2m_view view; + struct xen_hvm_altp2m_set_mem_access set_mem_access; + struct xen_hvm_altp2m_change_gfn change_gfn; + struct xen_hvm_altp2m_set_mem_access_multi set_mem_access_multi; uint8_t pad[64]; } u; };
For the default EPT view we have xc_set_mem_access_multi(), which is able to set an array of pages to an array of access rights with a single hypercall. However, this functionality was lacking for the altp2m subsystem, which could only set page restrictions for one page at a time. This patch addresses the gap. Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com> --- Changes since V1: - Replaced the mask-based hypercall continuation code with code using an opaque member to store the index. --- tools/libxc/include/xenctrl.h | 3 +++ tools/libxc/xc_altp2m.c | 41 +++++++++++++++++++++++++++++++++++++++++ xen/arch/x86/hvm/hvm.c | 25 +++++++++++++++++++++++++ xen/include/public/hvm/hvm_op.h | 30 +++++++++++++++++++++++++----- 4 files changed, 94 insertions(+), 5 deletions(-)