* [PATCH 0/2] x86/hvm: HVMOP_get/set_param improvements
@ 2015-04-24 16:21 Paul Durrant
2015-04-24 16:21 ` [PATCH 1/3] x86/hvm: give HVMOP_set_param and HVMOP_get_param their own functions Paul Durrant
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Paul Durrant @ 2015-04-24 16:21 UTC (permalink / raw)
To: xen-devel; +Cc: Paul Durrant
The following 3 patches re-structure the code implementing HVMOP_set_param
and HVMOP_get_param.
Patch #1 gives each operation its own function
Patch #2 splits out checks for getting/setting non-reflexive params and
setting params with change-once semantics, as well as the XSM check into
separate functions
Patch #3 adds all remaining ioreq server params to non-reflexive lists
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/3] x86/hvm: give HVMOP_set_param and HVMOP_get_param their own functions 2015-04-24 16:21 [PATCH 0/2] x86/hvm: HVMOP_get/set_param improvements Paul Durrant @ 2015-04-24 16:21 ` Paul Durrant 2015-04-24 17:11 ` Andrew Cooper 2015-04-24 16:21 ` [PATCH 2/3] x86/hvm: introduce functions for HVMOP_get/set_param allowance checks Paul Durrant ` (2 subsequent siblings) 3 siblings, 1 reply; 12+ messages in thread From: Paul Durrant @ 2015-04-24 16:21 UTC (permalink / raw) To: xen-devel; +Cc: Andrew Cooper, Paul Durrant, Keir Fraser, Jan Beulich The level of switch nesting in those ops is getting unreadable. Giving them their own functions does introduce some code duplication in the the pre-op checks but the overall result is easier to follow. This patch is code movement. There is no functional change. Signed-off-by: Paul Durrant <paul.durrant@citrix.com> Cc: Keir Fraser <keir@xen.org> Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> --- xen/arch/x86/hvm/hvm.c | 562 ++++++++++++++++++++++++++---------------------- 1 file changed, 300 insertions(+), 262 deletions(-) diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 3c62b80..87c47b1 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -5640,6 +5640,300 @@ static int hvmop_set_evtchn_upcall_vector( return 0; } +static int hvmop_set_param( + XEN_GUEST_HANDLE_PARAM(xen_hvm_param_t) arg) +{ + struct xen_hvm_param a; + struct domain *d; + struct vcpu *v; + int rc = 0; + + if ( copy_from_guest(&a, arg, 1) ) + return -EFAULT; + + if ( a.index >= HVM_NR_PARAMS ) + return -EINVAL; + + d = rcu_lock_domain_by_any_id(a.domid); + if ( d == NULL ) + return -ESRCH; + + rc = -EINVAL; + if ( !has_hvm_container_domain(d) ) + goto out; + + if ( is_pvh_domain(d) + && (a.index != HVM_PARAM_CALLBACK_IRQ) ) + goto out; + + rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_set_param); + if ( rc ) + goto out; + + switch ( a.index ) + { + case HVM_PARAM_CALLBACK_IRQ: + hvm_set_callback_via(d, a.value); + hvm_latch_shinfo_size(d); + break; + case HVM_PARAM_TIMER_MODE: + if ( a.value > HVMPTM_one_missed_tick_pending ) + rc = -EINVAL; + break; + case HVM_PARAM_VIRIDIAN: + /* This should only ever be set once by the tools and read by the guest. */ + rc = -EPERM; + if ( d == current->domain ) + break; + + if ( a.value != d->arch.hvm_domain.params[a.index] ) + { + rc = -EEXIST; + if ( d->arch.hvm_domain.params[a.index] != 0 ) + break; + + rc = -EINVAL; + if ( (a.value & ~HVMPV_feature_mask) || + !(a.value & HVMPV_base_freq) ) + break; + } + + rc = 0; + break; + case HVM_PARAM_IDENT_PT: + /* Not reflexive, as we must domain_pause(). */ + rc = -EPERM; + if ( d == current->domain ) + break; + + rc = -EINVAL; + if ( d->arch.hvm_domain.params[a.index] != 0 ) + break; + + rc = 0; + if ( !paging_mode_hap(d) ) + break; + + /* + * Update GUEST_CR3 in each VMCS to point at identity map. + * All foreign updates to guest state must synchronise on + * the domctl_lock. + */ + rc = -ERESTART; + if ( !domctl_lock_acquire() ) + break; + + rc = 0; + domain_pause(d); + d->arch.hvm_domain.params[a.index] = a.value; + for_each_vcpu ( d, v ) + paging_update_cr3(v); + domain_unpause(d); + + domctl_lock_release(); + break; + case HVM_PARAM_DM_DOMAIN: + /* Not reflexive, as we may need to domain_pause(). */ + rc = -EPERM; + if ( d == current->domain ) + break; + + if ( a.value == DOMID_SELF ) + a.value = current->domain->domain_id; + + rc = hvm_set_dm_domain(d, a.value); + break; + case HVM_PARAM_ACPI_S_STATE: + /* Not reflexive, as we must domain_pause(). */ + rc = -EPERM; + if ( current->domain == d ) + break; + + rc = 0; + if ( a.value == 3 ) + hvm_s3_suspend(d); + else if ( a.value == 0 ) + hvm_s3_resume(d); + else + rc = -EINVAL; + + break; + case HVM_PARAM_ACPI_IOPORTS_LOCATION: + rc = pmtimer_change_ioport(d, a.value); + break; + case HVM_PARAM_MEMORY_EVENT_CR0: + case HVM_PARAM_MEMORY_EVENT_CR3: + case HVM_PARAM_MEMORY_EVENT_CR4: + if ( d == current->domain ) + rc = -EPERM; + break; + case HVM_PARAM_MEMORY_EVENT_INT3: + case HVM_PARAM_MEMORY_EVENT_SINGLE_STEP: + case HVM_PARAM_MEMORY_EVENT_MSR: + if ( d == current->domain ) + { + rc = -EPERM; + break; + } + if ( a.value & HVMPME_onchangeonly ) + rc = -EINVAL; + break; + case HVM_PARAM_NESTEDHVM: + rc = xsm_hvm_param_nested(XSM_PRIV, d); + if ( rc ) + break; + if ( a.value > 1 ) + rc = -EINVAL; + /* Remove the check below once we have + * shadow-on-shadow. + */ + if ( cpu_has_svm && !paging_mode_hap(d) && a.value ) + rc = -EINVAL; + /* Set up NHVM state for any vcpus that are already up */ + if ( a.value && + !d->arch.hvm_domain.params[HVM_PARAM_NESTEDHVM] ) + for_each_vcpu(d, v) + if ( rc == 0 ) + rc = nestedhvm_vcpu_initialise(v); + if ( !a.value || rc ) + for_each_vcpu(d, v) + nestedhvm_vcpu_destroy(v); + break; + case HVM_PARAM_BUFIOREQ_EVTCHN: + rc = -EINVAL; + break; + case HVM_PARAM_TRIPLE_FAULT_REASON: + if ( a.value > SHUTDOWN_MAX ) + rc = -EINVAL; + break; + case HVM_PARAM_IOREQ_SERVER_PFN: + if ( d == current->domain ) + { + rc = -EPERM; + break; + } + d->arch.hvm_domain.ioreq_gmfn.base = a.value; + break; + case HVM_PARAM_NR_IOREQ_SERVER_PAGES: + { + unsigned int i; + + if ( d == current->domain ) + { + rc = -EPERM; + break; + } + if ( a.value == 0 || + a.value > sizeof(d->arch.hvm_domain.ioreq_gmfn.mask) * 8 ) + { + rc = -EINVAL; + break; + } + for ( i = 0; i < a.value; i++ ) + set_bit(i, &d->arch.hvm_domain.ioreq_gmfn.mask); + + break; + } + } + + if ( rc != 0 ) + goto out; + + d->arch.hvm_domain.params[a.index] = a.value; + + switch( a.index ) + { + case HVM_PARAM_MEMORY_EVENT_INT3: + case HVM_PARAM_MEMORY_EVENT_SINGLE_STEP: + { + domain_pause(d); + domain_unpause(d); /* Causes guest to latch new status */ + break; + } + case HVM_PARAM_MEMORY_EVENT_CR3: + { + for_each_vcpu ( d, v ) + hvm_funcs.update_guest_cr(v, 0); /* Latches new CR3 mask through CR0 code */ + break; + } + } + + HVM_DBG_LOG(DBG_LEVEL_HCALL, "set param %u = %"PRIx64, + a.index, a.value); + + out: + rcu_unlock_domain(d); + return rc; +} + +static int hvmop_get_param( + XEN_GUEST_HANDLE_PARAM(xen_hvm_param_t) arg) +{ + struct xen_hvm_param a; + struct domain *d; + int rc = 0; + + if ( copy_from_guest(&a, arg, 1) ) + return -EFAULT; + + if ( a.index >= HVM_NR_PARAMS ) + return -EINVAL; + + d = rcu_lock_domain_by_any_id(a.domid); + if ( d == NULL ) + return -ESRCH; + + rc = -EINVAL; + if ( !has_hvm_container_domain(d) ) + goto out; + + if ( is_pvh_domain(d) + && (a.index != HVM_PARAM_CALLBACK_IRQ) ) + goto out; + + rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_get_param); + if ( rc ) + goto out; + + switch ( a.index ) + { + case HVM_PARAM_ACPI_S_STATE: + a.value = d->arch.hvm_domain.is_s3_suspended ? 3 : 0; + break; + case HVM_PARAM_IOREQ_SERVER_PFN: + case HVM_PARAM_NR_IOREQ_SERVER_PAGES: + if ( d == current->domain ) + { + rc = -EPERM; + goto out; + } + case HVM_PARAM_IOREQ_PFN: + case HVM_PARAM_BUFIOREQ_PFN: + case HVM_PARAM_BUFIOREQ_EVTCHN: { + domid_t domid; + + /* May need to create server */ + domid = d->arch.hvm_domain.params[HVM_PARAM_DM_DOMAIN]; + rc = hvm_create_ioreq_server(d, domid, 1, 1, NULL); + if ( rc != 0 && rc != -EEXIST ) + goto out; + /*FALLTHRU*/ + } + default: + a.value = d->arch.hvm_domain.params[a.index]; + break; + } + + rc = __copy_to_guest(arg, &a, 1) ? -EFAULT : 0; + + HVM_DBG_LOG(DBG_LEVEL_HCALL, "get param %u = %"PRIx64, + a.index, a.value); + + out: + rcu_unlock_domain(d); + return rc; +} + /* * Note that this value is effectively part of the ABI, even if we don't need * to make it a formal part of it: A guest suspended for migration in the @@ -5651,7 +5945,6 @@ static int hvmop_set_evtchn_upcall_vector( long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) { - struct domain *curr_d = current->domain; unsigned long start_iter, mask; long rc = 0; @@ -5705,269 +5998,14 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) break; case HVMOP_set_param: - case HVMOP_get_param: - { - struct xen_hvm_param a; - struct domain *d; - struct vcpu *v; - - if ( copy_from_guest(&a, arg, 1) ) - return -EFAULT; - - if ( a.index >= HVM_NR_PARAMS ) - return -EINVAL; - - d = rcu_lock_domain_by_any_id(a.domid); - if ( d == NULL ) - return -ESRCH; - - rc = -EINVAL; - if ( !has_hvm_container_domain(d) ) - goto param_fail; - - if ( is_pvh_domain(d) - && (a.index != HVM_PARAM_CALLBACK_IRQ) ) - goto param_fail; - - rc = xsm_hvm_param(XSM_TARGET, d, op); - if ( rc ) - goto param_fail; - - if ( op == HVMOP_set_param ) - { - rc = 0; - - switch ( a.index ) - { - case HVM_PARAM_CALLBACK_IRQ: - hvm_set_callback_via(d, a.value); - hvm_latch_shinfo_size(d); - break; - case HVM_PARAM_TIMER_MODE: - if ( a.value > HVMPTM_one_missed_tick_pending ) - rc = -EINVAL; - break; - case HVM_PARAM_VIRIDIAN: - /* This should only ever be set once by the tools and read by the guest. */ - rc = -EPERM; - if ( curr_d == d ) - break; - - if ( a.value != d->arch.hvm_domain.params[a.index] ) - { - rc = -EEXIST; - if ( d->arch.hvm_domain.params[a.index] != 0 ) - break; - - rc = -EINVAL; - if ( (a.value & ~HVMPV_feature_mask) || - !(a.value & HVMPV_base_freq) ) - break; - } - - rc = 0; - break; - case HVM_PARAM_IDENT_PT: - /* Not reflexive, as we must domain_pause(). */ - rc = -EPERM; - if ( curr_d == d ) - break; - - rc = -EINVAL; - if ( d->arch.hvm_domain.params[a.index] != 0 ) - break; - - rc = 0; - if ( !paging_mode_hap(d) ) - break; - - /* - * Update GUEST_CR3 in each VMCS to point at identity map. - * All foreign updates to guest state must synchronise on - * the domctl_lock. - */ - rc = -ERESTART; - if ( !domctl_lock_acquire() ) - break; - - rc = 0; - domain_pause(d); - d->arch.hvm_domain.params[a.index] = a.value; - for_each_vcpu ( d, v ) - paging_update_cr3(v); - domain_unpause(d); - - domctl_lock_release(); - break; - case HVM_PARAM_DM_DOMAIN: - /* Not reflexive, as we may need to domain_pause(). */ - rc = -EPERM; - if ( curr_d == d ) - break; - - if ( a.value == DOMID_SELF ) - a.value = curr_d->domain_id; - - rc = hvm_set_dm_domain(d, a.value); - break; - case HVM_PARAM_ACPI_S_STATE: - /* Not reflexive, as we must domain_pause(). */ - rc = -EPERM; - if ( curr_d == d ) - break; - - rc = 0; - if ( a.value == 3 ) - hvm_s3_suspend(d); - else if ( a.value == 0 ) - hvm_s3_resume(d); - else - rc = -EINVAL; - - break; - case HVM_PARAM_ACPI_IOPORTS_LOCATION: - rc = pmtimer_change_ioport(d, a.value); - break; - case HVM_PARAM_MEMORY_EVENT_CR0: - case HVM_PARAM_MEMORY_EVENT_CR3: - case HVM_PARAM_MEMORY_EVENT_CR4: - if ( d == current->domain ) - rc = -EPERM; - break; - case HVM_PARAM_MEMORY_EVENT_INT3: - case HVM_PARAM_MEMORY_EVENT_SINGLE_STEP: - case HVM_PARAM_MEMORY_EVENT_MSR: - if ( d == current->domain ) - { - rc = -EPERM; - break; - } - if ( a.value & HVMPME_onchangeonly ) - rc = -EINVAL; - break; - case HVM_PARAM_NESTEDHVM: - rc = xsm_hvm_param_nested(XSM_PRIV, d); - if ( rc ) - break; - if ( a.value > 1 ) - rc = -EINVAL; - /* Remove the check below once we have - * shadow-on-shadow. - */ - if ( cpu_has_svm && !paging_mode_hap(d) && a.value ) - rc = -EINVAL; - /* Set up NHVM state for any vcpus that are already up */ - if ( a.value && - !d->arch.hvm_domain.params[HVM_PARAM_NESTEDHVM] ) - for_each_vcpu(d, v) - if ( rc == 0 ) - rc = nestedhvm_vcpu_initialise(v); - if ( !a.value || rc ) - for_each_vcpu(d, v) - nestedhvm_vcpu_destroy(v); - break; - case HVM_PARAM_BUFIOREQ_EVTCHN: - rc = -EINVAL; - break; - case HVM_PARAM_TRIPLE_FAULT_REASON: - if ( a.value > SHUTDOWN_MAX ) - rc = -EINVAL; - break; - case HVM_PARAM_IOREQ_SERVER_PFN: - if ( d == current->domain ) - { - rc = -EPERM; - break; - } - d->arch.hvm_domain.ioreq_gmfn.base = a.value; - break; - case HVM_PARAM_NR_IOREQ_SERVER_PAGES: - { - unsigned int i; - - if ( d == current->domain ) - { - rc = -EPERM; - break; - } - if ( a.value == 0 || - a.value > sizeof(d->arch.hvm_domain.ioreq_gmfn.mask) * 8 ) - { - rc = -EINVAL; - break; - } - for ( i = 0; i < a.value; i++ ) - set_bit(i, &d->arch.hvm_domain.ioreq_gmfn.mask); - - break; - } - } - - if ( rc == 0 ) - { - d->arch.hvm_domain.params[a.index] = a.value; - - switch( a.index ) - { - case HVM_PARAM_MEMORY_EVENT_INT3: - case HVM_PARAM_MEMORY_EVENT_SINGLE_STEP: - { - domain_pause(d); - domain_unpause(d); /* Causes guest to latch new status */ - break; - } - case HVM_PARAM_MEMORY_EVENT_CR3: - { - for_each_vcpu ( d, v ) - hvm_funcs.update_guest_cr(v, 0); /* Latches new CR3 mask through CR0 code */ - break; - } - } - - } - - } - else - { - switch ( a.index ) - { - case HVM_PARAM_ACPI_S_STATE: - a.value = d->arch.hvm_domain.is_s3_suspended ? 3 : 0; - break; - case HVM_PARAM_IOREQ_SERVER_PFN: - case HVM_PARAM_NR_IOREQ_SERVER_PAGES: - if ( d == current->domain ) - { - rc = -EPERM; - break; - } - case HVM_PARAM_IOREQ_PFN: - case HVM_PARAM_BUFIOREQ_PFN: - case HVM_PARAM_BUFIOREQ_EVTCHN: { - domid_t domid; - - /* May need to create server */ - domid = d->arch.hvm_domain.params[HVM_PARAM_DM_DOMAIN]; - rc = hvm_create_ioreq_server(d, domid, 1, 1, NULL); - if ( rc != 0 && rc != -EEXIST ) - goto param_fail; - /*FALLTHRU*/ - } - default: - a.value = d->arch.hvm_domain.params[a.index]; - break; - } - rc = __copy_to_guest(arg, &a, 1) ? -EFAULT : 0; - } - - HVM_DBG_LOG(DBG_LEVEL_HCALL, "%s param %u = %"PRIx64, - op == HVMOP_set_param ? "set" : "get", - a.index, a.value); + rc = hvmop_set_param( + guest_handle_cast(arg, xen_hvm_param_t)); + break; - param_fail: - rcu_unlock_domain(d); + case HVMOP_get_param: + rc = hvmop_get_param( + guest_handle_cast(arg, xen_hvm_param_t)); break; - } case HVMOP_set_pci_intx_level: rc = hvmop_set_pci_intx_level( -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] x86/hvm: give HVMOP_set_param and HVMOP_get_param their own functions 2015-04-24 16:21 ` [PATCH 1/3] x86/hvm: give HVMOP_set_param and HVMOP_get_param their own functions Paul Durrant @ 2015-04-24 17:11 ` Andrew Cooper 2015-04-29 2:16 ` Paul Durrant 0 siblings, 1 reply; 12+ messages in thread From: Andrew Cooper @ 2015-04-24 17:11 UTC (permalink / raw) To: Paul Durrant, xen-devel; +Cc: Keir Fraser, Jan Beulich On 24/04/15 17:21, Paul Durrant wrote: > The level of switch nesting in those ops is getting unreadable. Giving > them their own functions does introduce some code duplication in the > the pre-op checks but the overall result is easier to follow. > > This patch is code movement. There is no functional change. > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com> > Cc: Keir Fraser <keir@xen.org> > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Andrew Cooper <andrew.cooper3@citrix.com> Please latch current at the top of the function, and fix Xen style during motion. (newlines between break and case lines, drop superfluous braces, brace layout around get bufiorq_evtchn). With that fixed, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > xen/arch/x86/hvm/hvm.c | 562 ++++++++++++++++++++++++++---------------------- > 1 file changed, 300 insertions(+), 262 deletions(-) > > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > index 3c62b80..87c47b1 100644 > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -5640,6 +5640,300 @@ static int hvmop_set_evtchn_upcall_vector( > return 0; > } > > +static int hvmop_set_param( > + XEN_GUEST_HANDLE_PARAM(xen_hvm_param_t) arg) > +{ > + struct xen_hvm_param a; > + struct domain *d; > + struct vcpu *v; > + int rc = 0; > + > + if ( copy_from_guest(&a, arg, 1) ) > + return -EFAULT; > + > + if ( a.index >= HVM_NR_PARAMS ) > + return -EINVAL; > + > + d = rcu_lock_domain_by_any_id(a.domid); > + if ( d == NULL ) > + return -ESRCH; > + > + rc = -EINVAL; > + if ( !has_hvm_container_domain(d) ) > + goto out; > + > + if ( is_pvh_domain(d) > + && (a.index != HVM_PARAM_CALLBACK_IRQ) ) > + goto out; > + > + rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_set_param); > + if ( rc ) > + goto out; > + > + switch ( a.index ) > + { > + case HVM_PARAM_CALLBACK_IRQ: > + hvm_set_callback_via(d, a.value); > + hvm_latch_shinfo_size(d); > + break; > + case HVM_PARAM_TIMER_MODE: > + if ( a.value > HVMPTM_one_missed_tick_pending ) > + rc = -EINVAL; > + break; > + case HVM_PARAM_VIRIDIAN: > + /* This should only ever be set once by the tools and read by the guest. */ > + rc = -EPERM; > + if ( d == current->domain ) > + break; > + > + if ( a.value != d->arch.hvm_domain.params[a.index] ) > + { > + rc = -EEXIST; > + if ( d->arch.hvm_domain.params[a.index] != 0 ) > + break; > + > + rc = -EINVAL; > + if ( (a.value & ~HVMPV_feature_mask) || > + !(a.value & HVMPV_base_freq) ) > + break; > + } > + > + rc = 0; > + break; > + case HVM_PARAM_IDENT_PT: > + /* Not reflexive, as we must domain_pause(). */ > + rc = -EPERM; > + if ( d == current->domain ) > + break; > + > + rc = -EINVAL; > + if ( d->arch.hvm_domain.params[a.index] != 0 ) > + break; > + > + rc = 0; > + if ( !paging_mode_hap(d) ) > + break; > + > + /* > + * Update GUEST_CR3 in each VMCS to point at identity map. > + * All foreign updates to guest state must synchronise on > + * the domctl_lock. > + */ > + rc = -ERESTART; > + if ( !domctl_lock_acquire() ) > + break; > + > + rc = 0; > + domain_pause(d); > + d->arch.hvm_domain.params[a.index] = a.value; > + for_each_vcpu ( d, v ) > + paging_update_cr3(v); > + domain_unpause(d); > + > + domctl_lock_release(); > + break; > + case HVM_PARAM_DM_DOMAIN: > + /* Not reflexive, as we may need to domain_pause(). */ > + rc = -EPERM; > + if ( d == current->domain ) > + break; > + > + if ( a.value == DOMID_SELF ) > + a.value = current->domain->domain_id; > + > + rc = hvm_set_dm_domain(d, a.value); > + break; > + case HVM_PARAM_ACPI_S_STATE: > + /* Not reflexive, as we must domain_pause(). */ > + rc = -EPERM; > + if ( current->domain == d ) > + break; > + > + rc = 0; > + if ( a.value == 3 ) > + hvm_s3_suspend(d); > + else if ( a.value == 0 ) > + hvm_s3_resume(d); > + else > + rc = -EINVAL; > + > + break; > + case HVM_PARAM_ACPI_IOPORTS_LOCATION: > + rc = pmtimer_change_ioport(d, a.value); > + break; > + case HVM_PARAM_MEMORY_EVENT_CR0: > + case HVM_PARAM_MEMORY_EVENT_CR3: > + case HVM_PARAM_MEMORY_EVENT_CR4: > + if ( d == current->domain ) > + rc = -EPERM; > + break; > + case HVM_PARAM_MEMORY_EVENT_INT3: > + case HVM_PARAM_MEMORY_EVENT_SINGLE_STEP: > + case HVM_PARAM_MEMORY_EVENT_MSR: > + if ( d == current->domain ) > + { > + rc = -EPERM; > + break; > + } > + if ( a.value & HVMPME_onchangeonly ) > + rc = -EINVAL; > + break; > + case HVM_PARAM_NESTEDHVM: > + rc = xsm_hvm_param_nested(XSM_PRIV, d); > + if ( rc ) > + break; > + if ( a.value > 1 ) > + rc = -EINVAL; > + /* Remove the check below once we have > + * shadow-on-shadow. > + */ > + if ( cpu_has_svm && !paging_mode_hap(d) && a.value ) > + rc = -EINVAL; > + /* Set up NHVM state for any vcpus that are already up */ > + if ( a.value && > + !d->arch.hvm_domain.params[HVM_PARAM_NESTEDHVM] ) > + for_each_vcpu(d, v) > + if ( rc == 0 ) > + rc = nestedhvm_vcpu_initialise(v); > + if ( !a.value || rc ) > + for_each_vcpu(d, v) > + nestedhvm_vcpu_destroy(v); > + break; > + case HVM_PARAM_BUFIOREQ_EVTCHN: > + rc = -EINVAL; > + break; > + case HVM_PARAM_TRIPLE_FAULT_REASON: > + if ( a.value > SHUTDOWN_MAX ) > + rc = -EINVAL; > + break; > + case HVM_PARAM_IOREQ_SERVER_PFN: > + if ( d == current->domain ) > + { > + rc = -EPERM; > + break; > + } > + d->arch.hvm_domain.ioreq_gmfn.base = a.value; > + break; > + case HVM_PARAM_NR_IOREQ_SERVER_PAGES: > + { > + unsigned int i; > + > + if ( d == current->domain ) > + { > + rc = -EPERM; > + break; > + } > + if ( a.value == 0 || > + a.value > sizeof(d->arch.hvm_domain.ioreq_gmfn.mask) * 8 ) > + { > + rc = -EINVAL; > + break; > + } > + for ( i = 0; i < a.value; i++ ) > + set_bit(i, &d->arch.hvm_domain.ioreq_gmfn.mask); > + > + break; > + } > + } > + > + if ( rc != 0 ) > + goto out; > + > + d->arch.hvm_domain.params[a.index] = a.value; > + > + switch( a.index ) > + { > + case HVM_PARAM_MEMORY_EVENT_INT3: > + case HVM_PARAM_MEMORY_EVENT_SINGLE_STEP: > + { > + domain_pause(d); > + domain_unpause(d); /* Causes guest to latch new status */ > + break; > + } > + case HVM_PARAM_MEMORY_EVENT_CR3: > + { > + for_each_vcpu ( d, v ) > + hvm_funcs.update_guest_cr(v, 0); /* Latches new CR3 mask through CR0 code */ > + break; > + } > + } > + > + HVM_DBG_LOG(DBG_LEVEL_HCALL, "set param %u = %"PRIx64, > + a.index, a.value); > + > + out: > + rcu_unlock_domain(d); > + return rc; > +} > + > +static int hvmop_get_param( > + XEN_GUEST_HANDLE_PARAM(xen_hvm_param_t) arg) > +{ > + struct xen_hvm_param a; > + struct domain *d; > + int rc = 0; > + > + if ( copy_from_guest(&a, arg, 1) ) > + return -EFAULT; > + > + if ( a.index >= HVM_NR_PARAMS ) > + return -EINVAL; > + > + d = rcu_lock_domain_by_any_id(a.domid); > + if ( d == NULL ) > + return -ESRCH; > + > + rc = -EINVAL; > + if ( !has_hvm_container_domain(d) ) > + goto out; > + > + if ( is_pvh_domain(d) > + && (a.index != HVM_PARAM_CALLBACK_IRQ) ) > + goto out; > + > + rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_get_param); > + if ( rc ) > + goto out; > + > + switch ( a.index ) > + { > + case HVM_PARAM_ACPI_S_STATE: > + a.value = d->arch.hvm_domain.is_s3_suspended ? 3 : 0; > + break; > + case HVM_PARAM_IOREQ_SERVER_PFN: > + case HVM_PARAM_NR_IOREQ_SERVER_PAGES: > + if ( d == current->domain ) > + { > + rc = -EPERM; > + goto out; > + } > + case HVM_PARAM_IOREQ_PFN: > + case HVM_PARAM_BUFIOREQ_PFN: > + case HVM_PARAM_BUFIOREQ_EVTCHN: { > + domid_t domid; > + > + /* May need to create server */ > + domid = d->arch.hvm_domain.params[HVM_PARAM_DM_DOMAIN]; > + rc = hvm_create_ioreq_server(d, domid, 1, 1, NULL); > + if ( rc != 0 && rc != -EEXIST ) > + goto out; > + /*FALLTHRU*/ > + } > + default: > + a.value = d->arch.hvm_domain.params[a.index]; > + break; > + } > + > + rc = __copy_to_guest(arg, &a, 1) ? -EFAULT : 0; > + > + HVM_DBG_LOG(DBG_LEVEL_HCALL, "get param %u = %"PRIx64, > + a.index, a.value); > + > + out: > + rcu_unlock_domain(d); > + return rc; > +} > + > /* > * Note that this value is effectively part of the ABI, even if we don't need > * to make it a formal part of it: A guest suspended for migration in the > @@ -5651,7 +5945,6 @@ static int hvmop_set_evtchn_upcall_vector( > long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) > > { > - struct domain *curr_d = current->domain; > unsigned long start_iter, mask; > long rc = 0; > > @@ -5705,269 +5998,14 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) > break; > > case HVMOP_set_param: > - case HVMOP_get_param: > - { > - struct xen_hvm_param a; > - struct domain *d; > - struct vcpu *v; > - > - if ( copy_from_guest(&a, arg, 1) ) > - return -EFAULT; > - > - if ( a.index >= HVM_NR_PARAMS ) > - return -EINVAL; > - > - d = rcu_lock_domain_by_any_id(a.domid); > - if ( d == NULL ) > - return -ESRCH; > - > - rc = -EINVAL; > - if ( !has_hvm_container_domain(d) ) > - goto param_fail; > - > - if ( is_pvh_domain(d) > - && (a.index != HVM_PARAM_CALLBACK_IRQ) ) > - goto param_fail; > - > - rc = xsm_hvm_param(XSM_TARGET, d, op); > - if ( rc ) > - goto param_fail; > - > - if ( op == HVMOP_set_param ) > - { > - rc = 0; > - > - switch ( a.index ) > - { > - case HVM_PARAM_CALLBACK_IRQ: > - hvm_set_callback_via(d, a.value); > - hvm_latch_shinfo_size(d); > - break; > - case HVM_PARAM_TIMER_MODE: > - if ( a.value > HVMPTM_one_missed_tick_pending ) > - rc = -EINVAL; > - break; > - case HVM_PARAM_VIRIDIAN: > - /* This should only ever be set once by the tools and read by the guest. */ > - rc = -EPERM; > - if ( curr_d == d ) > - break; > - > - if ( a.value != d->arch.hvm_domain.params[a.index] ) > - { > - rc = -EEXIST; > - if ( d->arch.hvm_domain.params[a.index] != 0 ) > - break; > - > - rc = -EINVAL; > - if ( (a.value & ~HVMPV_feature_mask) || > - !(a.value & HVMPV_base_freq) ) > - break; > - } > - > - rc = 0; > - break; > - case HVM_PARAM_IDENT_PT: > - /* Not reflexive, as we must domain_pause(). */ > - rc = -EPERM; > - if ( curr_d == d ) > - break; > - > - rc = -EINVAL; > - if ( d->arch.hvm_domain.params[a.index] != 0 ) > - break; > - > - rc = 0; > - if ( !paging_mode_hap(d) ) > - break; > - > - /* > - * Update GUEST_CR3 in each VMCS to point at identity map. > - * All foreign updates to guest state must synchronise on > - * the domctl_lock. > - */ > - rc = -ERESTART; > - if ( !domctl_lock_acquire() ) > - break; > - > - rc = 0; > - domain_pause(d); > - d->arch.hvm_domain.params[a.index] = a.value; > - for_each_vcpu ( d, v ) > - paging_update_cr3(v); > - domain_unpause(d); > - > - domctl_lock_release(); > - break; > - case HVM_PARAM_DM_DOMAIN: > - /* Not reflexive, as we may need to domain_pause(). */ > - rc = -EPERM; > - if ( curr_d == d ) > - break; > - > - if ( a.value == DOMID_SELF ) > - a.value = curr_d->domain_id; > - > - rc = hvm_set_dm_domain(d, a.value); > - break; > - case HVM_PARAM_ACPI_S_STATE: > - /* Not reflexive, as we must domain_pause(). */ > - rc = -EPERM; > - if ( curr_d == d ) > - break; > - > - rc = 0; > - if ( a.value == 3 ) > - hvm_s3_suspend(d); > - else if ( a.value == 0 ) > - hvm_s3_resume(d); > - else > - rc = -EINVAL; > - > - break; > - case HVM_PARAM_ACPI_IOPORTS_LOCATION: > - rc = pmtimer_change_ioport(d, a.value); > - break; > - case HVM_PARAM_MEMORY_EVENT_CR0: > - case HVM_PARAM_MEMORY_EVENT_CR3: > - case HVM_PARAM_MEMORY_EVENT_CR4: > - if ( d == current->domain ) > - rc = -EPERM; > - break; > - case HVM_PARAM_MEMORY_EVENT_INT3: > - case HVM_PARAM_MEMORY_EVENT_SINGLE_STEP: > - case HVM_PARAM_MEMORY_EVENT_MSR: > - if ( d == current->domain ) > - { > - rc = -EPERM; > - break; > - } > - if ( a.value & HVMPME_onchangeonly ) > - rc = -EINVAL; > - break; > - case HVM_PARAM_NESTEDHVM: > - rc = xsm_hvm_param_nested(XSM_PRIV, d); > - if ( rc ) > - break; > - if ( a.value > 1 ) > - rc = -EINVAL; > - /* Remove the check below once we have > - * shadow-on-shadow. > - */ > - if ( cpu_has_svm && !paging_mode_hap(d) && a.value ) > - rc = -EINVAL; > - /* Set up NHVM state for any vcpus that are already up */ > - if ( a.value && > - !d->arch.hvm_domain.params[HVM_PARAM_NESTEDHVM] ) > - for_each_vcpu(d, v) > - if ( rc == 0 ) > - rc = nestedhvm_vcpu_initialise(v); > - if ( !a.value || rc ) > - for_each_vcpu(d, v) > - nestedhvm_vcpu_destroy(v); > - break; > - case HVM_PARAM_BUFIOREQ_EVTCHN: > - rc = -EINVAL; > - break; > - case HVM_PARAM_TRIPLE_FAULT_REASON: > - if ( a.value > SHUTDOWN_MAX ) > - rc = -EINVAL; > - break; > - case HVM_PARAM_IOREQ_SERVER_PFN: > - if ( d == current->domain ) > - { > - rc = -EPERM; > - break; > - } > - d->arch.hvm_domain.ioreq_gmfn.base = a.value; > - break; > - case HVM_PARAM_NR_IOREQ_SERVER_PAGES: > - { > - unsigned int i; > - > - if ( d == current->domain ) > - { > - rc = -EPERM; > - break; > - } > - if ( a.value == 0 || > - a.value > sizeof(d->arch.hvm_domain.ioreq_gmfn.mask) * 8 ) > - { > - rc = -EINVAL; > - break; > - } > - for ( i = 0; i < a.value; i++ ) > - set_bit(i, &d->arch.hvm_domain.ioreq_gmfn.mask); > - > - break; > - } > - } > - > - if ( rc == 0 ) > - { > - d->arch.hvm_domain.params[a.index] = a.value; > - > - switch( a.index ) > - { > - case HVM_PARAM_MEMORY_EVENT_INT3: > - case HVM_PARAM_MEMORY_EVENT_SINGLE_STEP: > - { > - domain_pause(d); > - domain_unpause(d); /* Causes guest to latch new status */ > - break; > - } > - case HVM_PARAM_MEMORY_EVENT_CR3: > - { > - for_each_vcpu ( d, v ) > - hvm_funcs.update_guest_cr(v, 0); /* Latches new CR3 mask through CR0 code */ > - break; > - } > - } > - > - } > - > - } > - else > - { > - switch ( a.index ) > - { > - case HVM_PARAM_ACPI_S_STATE: > - a.value = d->arch.hvm_domain.is_s3_suspended ? 3 : 0; > - break; > - case HVM_PARAM_IOREQ_SERVER_PFN: > - case HVM_PARAM_NR_IOREQ_SERVER_PAGES: > - if ( d == current->domain ) > - { > - rc = -EPERM; > - break; > - } > - case HVM_PARAM_IOREQ_PFN: > - case HVM_PARAM_BUFIOREQ_PFN: > - case HVM_PARAM_BUFIOREQ_EVTCHN: { > - domid_t domid; > - > - /* May need to create server */ > - domid = d->arch.hvm_domain.params[HVM_PARAM_DM_DOMAIN]; > - rc = hvm_create_ioreq_server(d, domid, 1, 1, NULL); > - if ( rc != 0 && rc != -EEXIST ) > - goto param_fail; > - /*FALLTHRU*/ > - } > - default: > - a.value = d->arch.hvm_domain.params[a.index]; > - break; > - } > - rc = __copy_to_guest(arg, &a, 1) ? -EFAULT : 0; > - } > - > - HVM_DBG_LOG(DBG_LEVEL_HCALL, "%s param %u = %"PRIx64, > - op == HVMOP_set_param ? "set" : "get", > - a.index, a.value); > + rc = hvmop_set_param( > + guest_handle_cast(arg, xen_hvm_param_t)); > + break; > > - param_fail: > - rcu_unlock_domain(d); > + case HVMOP_get_param: > + rc = hvmop_get_param( > + guest_handle_cast(arg, xen_hvm_param_t)); > break; > - } > > case HVMOP_set_pci_intx_level: > rc = hvmop_set_pci_intx_level( ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] x86/hvm: give HVMOP_set_param and HVMOP_get_param their own functions 2015-04-24 17:11 ` Andrew Cooper @ 2015-04-29 2:16 ` Paul Durrant 0 siblings, 0 replies; 12+ messages in thread From: Paul Durrant @ 2015-04-29 2:16 UTC (permalink / raw) To: Andrew Cooper, xen-devel@lists.xenproject.org; +Cc: Keir (Xen.org), Jan Beulich > -----Original Message----- > From: Andrew Cooper [mailto:andrew.cooper3@citrix.com] > Sent: 25 April 2015 01:12 > To: Paul Durrant; xen-devel@lists.xenproject.org > Cc: Keir (Xen.org); Jan Beulich > Subject: Re: [PATCH 1/3] x86/hvm: give HVMOP_set_param and > HVMOP_get_param their own functions > > On 24/04/15 17:21, Paul Durrant wrote: > > The level of switch nesting in those ops is getting unreadable. Giving > > them their own functions does introduce some code duplication in the > > the pre-op checks but the overall result is easier to follow. > > > > This patch is code movement. There is no functional change. > > > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com> > > Cc: Keir Fraser <keir@xen.org> > > Cc: Jan Beulich <jbeulich@suse.com> > > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > > Please latch current at the top of the function, and fix Xen style > during motion. (newlines between break and case lines, drop superfluous > braces, brace layout around get bufiorq_evtchn). > Sure, will do. > With that fixed, Reviewed-by: Andrew Cooper > <andrew.cooper3@citrix.com> > Cool. Paul > > --- > > xen/arch/x86/hvm/hvm.c | 562 ++++++++++++++++++++++++++---------- > ------------ > > 1 file changed, 300 insertions(+), 262 deletions(-) > > > > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > > index 3c62b80..87c47b1 100644 > > --- a/xen/arch/x86/hvm/hvm.c > > +++ b/xen/arch/x86/hvm/hvm.c > > @@ -5640,6 +5640,300 @@ static int hvmop_set_evtchn_upcall_vector( > > return 0; > > } > > > > +static int hvmop_set_param( > > + XEN_GUEST_HANDLE_PARAM(xen_hvm_param_t) arg) > > +{ > > + struct xen_hvm_param a; > > + struct domain *d; > > + struct vcpu *v; > > + int rc = 0; > > + > > + if ( copy_from_guest(&a, arg, 1) ) > > + return -EFAULT; > > + > > + if ( a.index >= HVM_NR_PARAMS ) > > + return -EINVAL; > > + > > + d = rcu_lock_domain_by_any_id(a.domid); > > + if ( d == NULL ) > > + return -ESRCH; > > + > > + rc = -EINVAL; > > + if ( !has_hvm_container_domain(d) ) > > + goto out; > > + > > + if ( is_pvh_domain(d) > > + && (a.index != HVM_PARAM_CALLBACK_IRQ) ) > > + goto out; > > + > > + rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_set_param); > > + if ( rc ) > > + goto out; > > + > > + switch ( a.index ) > > + { > > + case HVM_PARAM_CALLBACK_IRQ: > > + hvm_set_callback_via(d, a.value); > > + hvm_latch_shinfo_size(d); > > + break; > > + case HVM_PARAM_TIMER_MODE: > > + if ( a.value > HVMPTM_one_missed_tick_pending ) > > + rc = -EINVAL; > > + break; > > + case HVM_PARAM_VIRIDIAN: > > + /* This should only ever be set once by the tools and read by the > guest. */ > > + rc = -EPERM; > > + if ( d == current->domain ) > > + break; > > + > > + if ( a.value != d->arch.hvm_domain.params[a.index] ) > > + { > > + rc = -EEXIST; > > + if ( d->arch.hvm_domain.params[a.index] != 0 ) > > + break; > > + > > + rc = -EINVAL; > > + if ( (a.value & ~HVMPV_feature_mask) || > > + !(a.value & HVMPV_base_freq) ) > > + break; > > + } > > + > > + rc = 0; > > + break; > > + case HVM_PARAM_IDENT_PT: > > + /* Not reflexive, as we must domain_pause(). */ > > + rc = -EPERM; > > + if ( d == current->domain ) > > + break; > > + > > + rc = -EINVAL; > > + if ( d->arch.hvm_domain.params[a.index] != 0 ) > > + break; > > + > > + rc = 0; > > + if ( !paging_mode_hap(d) ) > > + break; > > + > > + /* > > + * Update GUEST_CR3 in each VMCS to point at identity map. > > + * All foreign updates to guest state must synchronise on > > + * the domctl_lock. > > + */ > > + rc = -ERESTART; > > + if ( !domctl_lock_acquire() ) > > + break; > > + > > + rc = 0; > > + domain_pause(d); > > + d->arch.hvm_domain.params[a.index] = a.value; > > + for_each_vcpu ( d, v ) > > + paging_update_cr3(v); > > + domain_unpause(d); > > + > > + domctl_lock_release(); > > + break; > > + case HVM_PARAM_DM_DOMAIN: > > + /* Not reflexive, as we may need to domain_pause(). */ > > + rc = -EPERM; > > + if ( d == current->domain ) > > + break; > > + > > + if ( a.value == DOMID_SELF ) > > + a.value = current->domain->domain_id; > > + > > + rc = hvm_set_dm_domain(d, a.value); > > + break; > > + case HVM_PARAM_ACPI_S_STATE: > > + /* Not reflexive, as we must domain_pause(). */ > > + rc = -EPERM; > > + if ( current->domain == d ) > > + break; > > + > > + rc = 0; > > + if ( a.value == 3 ) > > + hvm_s3_suspend(d); > > + else if ( a.value == 0 ) > > + hvm_s3_resume(d); > > + else > > + rc = -EINVAL; > > + > > + break; > > + case HVM_PARAM_ACPI_IOPORTS_LOCATION: > > + rc = pmtimer_change_ioport(d, a.value); > > + break; > > + case HVM_PARAM_MEMORY_EVENT_CR0: > > + case HVM_PARAM_MEMORY_EVENT_CR3: > > + case HVM_PARAM_MEMORY_EVENT_CR4: > > + if ( d == current->domain ) > > + rc = -EPERM; > > + break; > > + case HVM_PARAM_MEMORY_EVENT_INT3: > > + case HVM_PARAM_MEMORY_EVENT_SINGLE_STEP: > > + case HVM_PARAM_MEMORY_EVENT_MSR: > > + if ( d == current->domain ) > > + { > > + rc = -EPERM; > > + break; > > + } > > + if ( a.value & HVMPME_onchangeonly ) > > + rc = -EINVAL; > > + break; > > + case HVM_PARAM_NESTEDHVM: > > + rc = xsm_hvm_param_nested(XSM_PRIV, d); > > + if ( rc ) > > + break; > > + if ( a.value > 1 ) > > + rc = -EINVAL; > > + /* Remove the check below once we have > > + * shadow-on-shadow. > > + */ > > + if ( cpu_has_svm && !paging_mode_hap(d) && a.value ) > > + rc = -EINVAL; > > + /* Set up NHVM state for any vcpus that are already up */ > > + if ( a.value && > > + !d->arch.hvm_domain.params[HVM_PARAM_NESTEDHVM] ) > > + for_each_vcpu(d, v) > > + if ( rc == 0 ) > > + rc = nestedhvm_vcpu_initialise(v); > > + if ( !a.value || rc ) > > + for_each_vcpu(d, v) > > + nestedhvm_vcpu_destroy(v); > > + break; > > + case HVM_PARAM_BUFIOREQ_EVTCHN: > > + rc = -EINVAL; > > + break; > > + case HVM_PARAM_TRIPLE_FAULT_REASON: > > + if ( a.value > SHUTDOWN_MAX ) > > + rc = -EINVAL; > > + break; > > + case HVM_PARAM_IOREQ_SERVER_PFN: > > + if ( d == current->domain ) > > + { > > + rc = -EPERM; > > + break; > > + } > > + d->arch.hvm_domain.ioreq_gmfn.base = a.value; > > + break; > > + case HVM_PARAM_NR_IOREQ_SERVER_PAGES: > > + { > > + unsigned int i; > > + > > + if ( d == current->domain ) > > + { > > + rc = -EPERM; > > + break; > > + } > > + if ( a.value == 0 || > > + a.value > sizeof(d->arch.hvm_domain.ioreq_gmfn.mask) * 8 ) > > + { > > + rc = -EINVAL; > > + break; > > + } > > + for ( i = 0; i < a.value; i++ ) > > + set_bit(i, &d->arch.hvm_domain.ioreq_gmfn.mask); > > + > > + break; > > + } > > + } > > + > > + if ( rc != 0 ) > > + goto out; > > + > > + d->arch.hvm_domain.params[a.index] = a.value; > > + > > + switch( a.index ) > > + { > > + case HVM_PARAM_MEMORY_EVENT_INT3: > > + case HVM_PARAM_MEMORY_EVENT_SINGLE_STEP: > > + { > > + domain_pause(d); > > + domain_unpause(d); /* Causes guest to latch new status */ > > + break; > > + } > > + case HVM_PARAM_MEMORY_EVENT_CR3: > > + { > > + for_each_vcpu ( d, v ) > > + hvm_funcs.update_guest_cr(v, 0); /* Latches new CR3 mask > through CR0 code */ > > + break; > > + } > > + } > > + > > + HVM_DBG_LOG(DBG_LEVEL_HCALL, "set param %u = %"PRIx64, > > + a.index, a.value); > > + > > + out: > > + rcu_unlock_domain(d); > > + return rc; > > +} > > + > > +static int hvmop_get_param( > > + XEN_GUEST_HANDLE_PARAM(xen_hvm_param_t) arg) > > +{ > > + struct xen_hvm_param a; > > + struct domain *d; > > + int rc = 0; > > + > > + if ( copy_from_guest(&a, arg, 1) ) > > + return -EFAULT; > > + > > + if ( a.index >= HVM_NR_PARAMS ) > > + return -EINVAL; > > + > > + d = rcu_lock_domain_by_any_id(a.domid); > > + if ( d == NULL ) > > + return -ESRCH; > > + > > + rc = -EINVAL; > > + if ( !has_hvm_container_domain(d) ) > > + goto out; > > + > > + if ( is_pvh_domain(d) > > + && (a.index != HVM_PARAM_CALLBACK_IRQ) ) > > + goto out; > > + > > + rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_get_param); > > + if ( rc ) > > + goto out; > > + > > + switch ( a.index ) > > + { > > + case HVM_PARAM_ACPI_S_STATE: > > + a.value = d->arch.hvm_domain.is_s3_suspended ? 3 : 0; > > + break; > > + case HVM_PARAM_IOREQ_SERVER_PFN: > > + case HVM_PARAM_NR_IOREQ_SERVER_PAGES: > > + if ( d == current->domain ) > > + { > > + rc = -EPERM; > > + goto out; > > + } > > + case HVM_PARAM_IOREQ_PFN: > > + case HVM_PARAM_BUFIOREQ_PFN: > > + case HVM_PARAM_BUFIOREQ_EVTCHN: { > > + domid_t domid; > > + > > + /* May need to create server */ > > + domid = d->arch.hvm_domain.params[HVM_PARAM_DM_DOMAIN]; > > + rc = hvm_create_ioreq_server(d, domid, 1, 1, NULL); > > + if ( rc != 0 && rc != -EEXIST ) > > + goto out; > > + /*FALLTHRU*/ > > + } > > + default: > > + a.value = d->arch.hvm_domain.params[a.index]; > > + break; > > + } > > + > > + rc = __copy_to_guest(arg, &a, 1) ? -EFAULT : 0; > > + > > + HVM_DBG_LOG(DBG_LEVEL_HCALL, "get param %u = %"PRIx64, > > + a.index, a.value); > > + > > + out: > > + rcu_unlock_domain(d); > > + return rc; > > +} > > + > > /* > > * Note that this value is effectively part of the ABI, even if we don't need > > * to make it a formal part of it: A guest suspended for migration in the > > @@ -5651,7 +5945,6 @@ static int hvmop_set_evtchn_upcall_vector( > > long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) > arg) > > > > { > > - struct domain *curr_d = current->domain; > > unsigned long start_iter, mask; > > long rc = 0; > > > > @@ -5705,269 +5998,14 @@ long do_hvm_op(unsigned long op, > XEN_GUEST_HANDLE_PARAM(void) arg) > > break; > > > > case HVMOP_set_param: > > - case HVMOP_get_param: > > - { > > - struct xen_hvm_param a; > > - struct domain *d; > > - struct vcpu *v; > > - > > - if ( copy_from_guest(&a, arg, 1) ) > > - return -EFAULT; > > - > > - if ( a.index >= HVM_NR_PARAMS ) > > - return -EINVAL; > > - > > - d = rcu_lock_domain_by_any_id(a.domid); > > - if ( d == NULL ) > > - return -ESRCH; > > - > > - rc = -EINVAL; > > - if ( !has_hvm_container_domain(d) ) > > - goto param_fail; > > - > > - if ( is_pvh_domain(d) > > - && (a.index != HVM_PARAM_CALLBACK_IRQ) ) > > - goto param_fail; > > - > > - rc = xsm_hvm_param(XSM_TARGET, d, op); > > - if ( rc ) > > - goto param_fail; > > - > > - if ( op == HVMOP_set_param ) > > - { > > - rc = 0; > > - > > - switch ( a.index ) > > - { > > - case HVM_PARAM_CALLBACK_IRQ: > > - hvm_set_callback_via(d, a.value); > > - hvm_latch_shinfo_size(d); > > - break; > > - case HVM_PARAM_TIMER_MODE: > > - if ( a.value > HVMPTM_one_missed_tick_pending ) > > - rc = -EINVAL; > > - break; > > - case HVM_PARAM_VIRIDIAN: > > - /* This should only ever be set once by the tools and read by the > guest. */ > > - rc = -EPERM; > > - if ( curr_d == d ) > > - break; > > - > > - if ( a.value != d->arch.hvm_domain.params[a.index] ) > > - { > > - rc = -EEXIST; > > - if ( d->arch.hvm_domain.params[a.index] != 0 ) > > - break; > > - > > - rc = -EINVAL; > > - if ( (a.value & ~HVMPV_feature_mask) || > > - !(a.value & HVMPV_base_freq) ) > > - break; > > - } > > - > > - rc = 0; > > - break; > > - case HVM_PARAM_IDENT_PT: > > - /* Not reflexive, as we must domain_pause(). */ > > - rc = -EPERM; > > - if ( curr_d == d ) > > - break; > > - > > - rc = -EINVAL; > > - if ( d->arch.hvm_domain.params[a.index] != 0 ) > > - break; > > - > > - rc = 0; > > - if ( !paging_mode_hap(d) ) > > - break; > > - > > - /* > > - * Update GUEST_CR3 in each VMCS to point at identity map. > > - * All foreign updates to guest state must synchronise on > > - * the domctl_lock. > > - */ > > - rc = -ERESTART; > > - if ( !domctl_lock_acquire() ) > > - break; > > - > > - rc = 0; > > - domain_pause(d); > > - d->arch.hvm_domain.params[a.index] = a.value; > > - for_each_vcpu ( d, v ) > > - paging_update_cr3(v); > > - domain_unpause(d); > > - > > - domctl_lock_release(); > > - break; > > - case HVM_PARAM_DM_DOMAIN: > > - /* Not reflexive, as we may need to domain_pause(). */ > > - rc = -EPERM; > > - if ( curr_d == d ) > > - break; > > - > > - if ( a.value == DOMID_SELF ) > > - a.value = curr_d->domain_id; > > - > > - rc = hvm_set_dm_domain(d, a.value); > > - break; > > - case HVM_PARAM_ACPI_S_STATE: > > - /* Not reflexive, as we must domain_pause(). */ > > - rc = -EPERM; > > - if ( curr_d == d ) > > - break; > > - > > - rc = 0; > > - if ( a.value == 3 ) > > - hvm_s3_suspend(d); > > - else if ( a.value == 0 ) > > - hvm_s3_resume(d); > > - else > > - rc = -EINVAL; > > - > > - break; > > - case HVM_PARAM_ACPI_IOPORTS_LOCATION: > > - rc = pmtimer_change_ioport(d, a.value); > > - break; > > - case HVM_PARAM_MEMORY_EVENT_CR0: > > - case HVM_PARAM_MEMORY_EVENT_CR3: > > - case HVM_PARAM_MEMORY_EVENT_CR4: > > - if ( d == current->domain ) > > - rc = -EPERM; > > - break; > > - case HVM_PARAM_MEMORY_EVENT_INT3: > > - case HVM_PARAM_MEMORY_EVENT_SINGLE_STEP: > > - case HVM_PARAM_MEMORY_EVENT_MSR: > > - if ( d == current->domain ) > > - { > > - rc = -EPERM; > > - break; > > - } > > - if ( a.value & HVMPME_onchangeonly ) > > - rc = -EINVAL; > > - break; > > - case HVM_PARAM_NESTEDHVM: > > - rc = xsm_hvm_param_nested(XSM_PRIV, d); > > - if ( rc ) > > - break; > > - if ( a.value > 1 ) > > - rc = -EINVAL; > > - /* Remove the check below once we have > > - * shadow-on-shadow. > > - */ > > - if ( cpu_has_svm && !paging_mode_hap(d) && a.value ) > > - rc = -EINVAL; > > - /* Set up NHVM state for any vcpus that are already up */ > > - if ( a.value && > > - !d->arch.hvm_domain.params[HVM_PARAM_NESTEDHVM] ) > > - for_each_vcpu(d, v) > > - if ( rc == 0 ) > > - rc = nestedhvm_vcpu_initialise(v); > > - if ( !a.value || rc ) > > - for_each_vcpu(d, v) > > - nestedhvm_vcpu_destroy(v); > > - break; > > - case HVM_PARAM_BUFIOREQ_EVTCHN: > > - rc = -EINVAL; > > - break; > > - case HVM_PARAM_TRIPLE_FAULT_REASON: > > - if ( a.value > SHUTDOWN_MAX ) > > - rc = -EINVAL; > > - break; > > - case HVM_PARAM_IOREQ_SERVER_PFN: > > - if ( d == current->domain ) > > - { > > - rc = -EPERM; > > - break; > > - } > > - d->arch.hvm_domain.ioreq_gmfn.base = a.value; > > - break; > > - case HVM_PARAM_NR_IOREQ_SERVER_PAGES: > > - { > > - unsigned int i; > > - > > - if ( d == current->domain ) > > - { > > - rc = -EPERM; > > - break; > > - } > > - if ( a.value == 0 || > > - a.value > sizeof(d->arch.hvm_domain.ioreq_gmfn.mask) * 8 ) > > - { > > - rc = -EINVAL; > > - break; > > - } > > - for ( i = 0; i < a.value; i++ ) > > - set_bit(i, &d->arch.hvm_domain.ioreq_gmfn.mask); > > - > > - break; > > - } > > - } > > - > > - if ( rc == 0 ) > > - { > > - d->arch.hvm_domain.params[a.index] = a.value; > > - > > - switch( a.index ) > > - { > > - case HVM_PARAM_MEMORY_EVENT_INT3: > > - case HVM_PARAM_MEMORY_EVENT_SINGLE_STEP: > > - { > > - domain_pause(d); > > - domain_unpause(d); /* Causes guest to latch new status */ > > - break; > > - } > > - case HVM_PARAM_MEMORY_EVENT_CR3: > > - { > > - for_each_vcpu ( d, v ) > > - hvm_funcs.update_guest_cr(v, 0); /* Latches new CR3 mask > through CR0 code */ > > - break; > > - } > > - } > > - > > - } > > - > > - } > > - else > > - { > > - switch ( a.index ) > > - { > > - case HVM_PARAM_ACPI_S_STATE: > > - a.value = d->arch.hvm_domain.is_s3_suspended ? 3 : 0; > > - break; > > - case HVM_PARAM_IOREQ_SERVER_PFN: > > - case HVM_PARAM_NR_IOREQ_SERVER_PAGES: > > - if ( d == current->domain ) > > - { > > - rc = -EPERM; > > - break; > > - } > > - case HVM_PARAM_IOREQ_PFN: > > - case HVM_PARAM_BUFIOREQ_PFN: > > - case HVM_PARAM_BUFIOREQ_EVTCHN: { > > - domid_t domid; > > - > > - /* May need to create server */ > > - domid = d- > >arch.hvm_domain.params[HVM_PARAM_DM_DOMAIN]; > > - rc = hvm_create_ioreq_server(d, domid, 1, 1, NULL); > > - if ( rc != 0 && rc != -EEXIST ) > > - goto param_fail; > > - /*FALLTHRU*/ > > - } > > - default: > > - a.value = d->arch.hvm_domain.params[a.index]; > > - break; > > - } > > - rc = __copy_to_guest(arg, &a, 1) ? -EFAULT : 0; > > - } > > - > > - HVM_DBG_LOG(DBG_LEVEL_HCALL, "%s param %u = %"PRIx64, > > - op == HVMOP_set_param ? "set" : "get", > > - a.index, a.value); > > + rc = hvmop_set_param( > > + guest_handle_cast(arg, xen_hvm_param_t)); > > + break; > > > > - param_fail: > > - rcu_unlock_domain(d); > > + case HVMOP_get_param: > > + rc = hvmop_get_param( > > + guest_handle_cast(arg, xen_hvm_param_t)); > > break; > > - } > > > > case HVMOP_set_pci_intx_level: > > rc = hvmop_set_pci_intx_level( ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/3] x86/hvm: introduce functions for HVMOP_get/set_param allowance checks 2015-04-24 16:21 [PATCH 0/2] x86/hvm: HVMOP_get/set_param improvements Paul Durrant 2015-04-24 16:21 ` [PATCH 1/3] x86/hvm: give HVMOP_set_param and HVMOP_get_param their own functions Paul Durrant @ 2015-04-24 16:21 ` Paul Durrant 2015-04-24 17:19 ` Andrew Cooper 2015-04-29 12:29 ` Jan Beulich 2015-04-24 16:21 ` [PATCH 3/3] x86/hvm: disallow guest get and set of all ioreq server HVM params Paul Durrant 2015-04-24 16:23 ` [PATCH 0/2] x86/hvm: HVMOP_get/set_param improvements Paul Durrant 3 siblings, 2 replies; 12+ messages in thread From: Paul Durrant @ 2015-04-24 16:21 UTC (permalink / raw) To: xen-devel; +Cc: Andrew Cooper, Paul Durrant, Keir Fraser, Jan Beulich Some parameters can only (validly) be set once. Some cannot be set by a guest for its own domain. Consolidate these checks, along with the XSM check, in a new hvm_allow_set_param() function for clarity. Also, introduce hvm_allow_get_param() for similar reasons. Signed-off-by: Paul Durrant <paul.durrant@citrix.com> Cc: Keir Fraser <keir@xen.org> Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> --- xen/arch/x86/hvm/hvm.c | 141 +++++++++++++++++++++++++++--------------------- 1 file changed, 79 insertions(+), 62 deletions(-) diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 87c47b1..23c604d 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -5640,6 +5640,57 @@ static int hvmop_set_evtchn_upcall_vector( return 0; } +static int hvm_allow_set_param(struct domain *d, + struct xen_hvm_param *a) +{ + uint64_t value = d->arch.hvm_domain.params[a->index]; + int rc = 0; + + rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_set_param); + if ( rc ) + return rc; + + /* The following parameters cannot be set by the guest */ + switch (a->index) + { + case HVM_PARAM_VIRIDIAN: + case HVM_PARAM_IDENT_PT: + case HVM_PARAM_DM_DOMAIN: + case HVM_PARAM_ACPI_S_STATE: + case HVM_PARAM_MEMORY_EVENT_CR0: + case HVM_PARAM_MEMORY_EVENT_CR3: + case HVM_PARAM_MEMORY_EVENT_CR4: + case HVM_PARAM_MEMORY_EVENT_INT3: + case HVM_PARAM_MEMORY_EVENT_SINGLE_STEP: + case HVM_PARAM_MEMORY_EVENT_MSR: + case HVM_PARAM_IOREQ_SERVER_PFN: + case HVM_PARAM_NR_IOREQ_SERVER_PAGES: + if ( d == current->domain ) + rc = -EPERM; + break; + default: + break; + } + + if ( rc ) + return rc; + + /* The following parameters can only be changed once */ + switch (a->index) + { + case HVM_PARAM_VIRIDIAN: + case HVM_PARAM_IOREQ_SERVER_PFN: + case HVM_PARAM_NR_IOREQ_SERVER_PAGES: + if (value != 0 && a->value != value) + rc = -EEXIST; + break; + default: + break; + } + + return rc; +} + static int hvmop_set_param( XEN_GUEST_HANDLE_PARAM(xen_hvm_param_t) arg) { @@ -5666,7 +5717,7 @@ static int hvmop_set_param( && (a.index != HVM_PARAM_CALLBACK_IRQ) ) goto out; - rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_set_param); + rc = hvm_allow_set_param(d, &a); if ( rc ) goto out; @@ -5681,31 +5732,11 @@ static int hvmop_set_param( rc = -EINVAL; break; case HVM_PARAM_VIRIDIAN: - /* This should only ever be set once by the tools and read by the guest. */ - rc = -EPERM; - if ( d == current->domain ) - break; - - if ( a.value != d->arch.hvm_domain.params[a.index] ) - { - rc = -EEXIST; - if ( d->arch.hvm_domain.params[a.index] != 0 ) - break; - + if ( (a.value & ~HVMPV_feature_mask) || + !(a.value & HVMPV_base_freq) ) rc = -EINVAL; - if ( (a.value & ~HVMPV_feature_mask) || - !(a.value & HVMPV_base_freq) ) - break; - } - - rc = 0; break; case HVM_PARAM_IDENT_PT: - /* Not reflexive, as we must domain_pause(). */ - rc = -EPERM; - if ( d == current->domain ) - break; - rc = -EINVAL; if ( d->arch.hvm_domain.params[a.index] != 0 ) break; @@ -5733,22 +5764,12 @@ static int hvmop_set_param( domctl_lock_release(); break; case HVM_PARAM_DM_DOMAIN: - /* Not reflexive, as we may need to domain_pause(). */ - rc = -EPERM; - if ( d == current->domain ) - break; - if ( a.value == DOMID_SELF ) a.value = current->domain->domain_id; rc = hvm_set_dm_domain(d, a.value); break; case HVM_PARAM_ACPI_S_STATE: - /* Not reflexive, as we must domain_pause(). */ - rc = -EPERM; - if ( current->domain == d ) - break; - rc = 0; if ( a.value == 3 ) hvm_s3_suspend(d); @@ -5761,20 +5782,9 @@ static int hvmop_set_param( case HVM_PARAM_ACPI_IOPORTS_LOCATION: rc = pmtimer_change_ioport(d, a.value); break; - case HVM_PARAM_MEMORY_EVENT_CR0: - case HVM_PARAM_MEMORY_EVENT_CR3: - case HVM_PARAM_MEMORY_EVENT_CR4: - if ( d == current->domain ) - rc = -EPERM; - break; case HVM_PARAM_MEMORY_EVENT_INT3: case HVM_PARAM_MEMORY_EVENT_SINGLE_STEP: case HVM_PARAM_MEMORY_EVENT_MSR: - if ( d == current->domain ) - { - rc = -EPERM; - break; - } if ( a.value & HVMPME_onchangeonly ) rc = -EINVAL; break; @@ -5807,22 +5817,12 @@ static int hvmop_set_param( rc = -EINVAL; break; case HVM_PARAM_IOREQ_SERVER_PFN: - if ( d == current->domain ) - { - rc = -EPERM; - break; - } d->arch.hvm_domain.ioreq_gmfn.base = a.value; break; case HVM_PARAM_NR_IOREQ_SERVER_PAGES: { unsigned int i; - if ( d == current->domain ) - { - rc = -EPERM; - break; - } if ( a.value == 0 || a.value > sizeof(d->arch.hvm_domain.ioreq_gmfn.mask) * 8 ) { @@ -5866,6 +5866,30 @@ static int hvmop_set_param( return rc; } +static int hvm_allow_get_param(struct domain *d, + struct xen_hvm_param *a) +{ + int rc = 0; + + rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_get_param); + if ( rc ) + return rc; + + /* The following parameters cannot be read by the guest */ + switch (a->index) + { + case HVM_PARAM_IOREQ_SERVER_PFN: + case HVM_PARAM_NR_IOREQ_SERVER_PAGES: + if ( d == current->domain ) + rc = -EPERM; + break; + default: + break; + } + + return rc; +} + static int hvmop_get_param( XEN_GUEST_HANDLE_PARAM(xen_hvm_param_t) arg) { @@ -5891,7 +5915,7 @@ static int hvmop_get_param( && (a.index != HVM_PARAM_CALLBACK_IRQ) ) goto out; - rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_get_param); + rc = hvm_allow_get_param(d, &a); if ( rc ) goto out; @@ -5900,13 +5924,6 @@ static int hvmop_get_param( case HVM_PARAM_ACPI_S_STATE: a.value = d->arch.hvm_domain.is_s3_suspended ? 3 : 0; break; - case HVM_PARAM_IOREQ_SERVER_PFN: - case HVM_PARAM_NR_IOREQ_SERVER_PAGES: - if ( d == current->domain ) - { - rc = -EPERM; - goto out; - } case HVM_PARAM_IOREQ_PFN: case HVM_PARAM_BUFIOREQ_PFN: case HVM_PARAM_BUFIOREQ_EVTCHN: { -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] x86/hvm: introduce functions for HVMOP_get/set_param allowance checks 2015-04-24 16:21 ` [PATCH 2/3] x86/hvm: introduce functions for HVMOP_get/set_param allowance checks Paul Durrant @ 2015-04-24 17:19 ` Andrew Cooper 2015-04-29 2:23 ` Paul Durrant 2015-04-29 12:29 ` Jan Beulich 1 sibling, 1 reply; 12+ messages in thread From: Andrew Cooper @ 2015-04-24 17:19 UTC (permalink / raw) To: Paul Durrant, xen-devel; +Cc: Keir Fraser, Jan Beulich On 24/04/15 17:21, Paul Durrant wrote: > Some parameters can only (validly) be set once. Some cannot be set > by a guest for its own domain. Consolidate these checks, along with > the XSM check, in a new hvm_allow_set_param() function for clarity. > > Also, introduce hvm_allow_get_param() for similar reasons. > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com> > Cc: Keir Fraser <keir@xen.org> > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > --- > xen/arch/x86/hvm/hvm.c | 141 +++++++++++++++++++++++++++--------------------- > 1 file changed, 79 insertions(+), 62 deletions(-) > > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > index 87c47b1..23c604d 100644 > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -5640,6 +5640,57 @@ static int hvmop_set_evtchn_upcall_vector( > return 0; > } > > +static int hvm_allow_set_param(struct domain *d, > + struct xen_hvm_param *a) const > +{ > + uint64_t value = d->arch.hvm_domain.params[a->index]; > + int rc = 0; No need for the 0 initialiser (and I know Coverity will complain about it). > + > + rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_set_param); > + if ( rc ) > + return rc; > + > + /* The following parameters cannot be set by the guest */ > + switch (a->index) style > + { > + case HVM_PARAM_VIRIDIAN: > + case HVM_PARAM_IDENT_PT: > + case HVM_PARAM_DM_DOMAIN: > + case HVM_PARAM_ACPI_S_STATE: > + case HVM_PARAM_MEMORY_EVENT_CR0: > + case HVM_PARAM_MEMORY_EVENT_CR3: > + case HVM_PARAM_MEMORY_EVENT_CR4: > + case HVM_PARAM_MEMORY_EVENT_INT3: > + case HVM_PARAM_MEMORY_EVENT_SINGLE_STEP: > + case HVM_PARAM_MEMORY_EVENT_MSR: > + case HVM_PARAM_IOREQ_SERVER_PFN: > + case HVM_PARAM_NR_IOREQ_SERVER_PAGES: > + if ( d == current->domain ) > + rc = -EPERM; > + break; > + default: > + break; Please can we turn the guest-settable list into a whitelist rather than a blacklist. We have had several XSAs in the past where a guest has been able to modify a value it shouldn't have because new params were default writable by everyone. > + } > + > + if ( rc ) > + return rc; > + > + /* The following parameters can only be changed once */ > + switch (a->index) > + { > + case HVM_PARAM_VIRIDIAN: > + case HVM_PARAM_IOREQ_SERVER_PFN: > + case HVM_PARAM_NR_IOREQ_SERVER_PAGES: > + if (value != 0 && a->value != value) > + rc = -EEXIST; > + break; > + default: > + break; > + } > + > + return rc; > +} > + > static int hvmop_set_param( > XEN_GUEST_HANDLE_PARAM(xen_hvm_param_t) arg) > { > @@ -5666,7 +5717,7 @@ static int hvmop_set_param( > && (a.index != HVM_PARAM_CALLBACK_IRQ) ) > goto out; > > - rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_set_param); > + rc = hvm_allow_set_param(d, &a); > if ( rc ) > goto out; > > @@ -5681,31 +5732,11 @@ static int hvmop_set_param( > rc = -EINVAL; > break; > case HVM_PARAM_VIRIDIAN: > - /* This should only ever be set once by the tools and read by the guest. */ > - rc = -EPERM; > - if ( d == current->domain ) > - break; > - > - if ( a.value != d->arch.hvm_domain.params[a.index] ) > - { > - rc = -EEXIST; > - if ( d->arch.hvm_domain.params[a.index] != 0 ) > - break; > - > + if ( (a.value & ~HVMPV_feature_mask) || > + !(a.value & HVMPV_base_freq) ) > rc = -EINVAL; > - if ( (a.value & ~HVMPV_feature_mask) || > - !(a.value & HVMPV_base_freq) ) > - break; > - } > - > - rc = 0; > break; > case HVM_PARAM_IDENT_PT: > - /* Not reflexive, as we must domain_pause(). */ > - rc = -EPERM; > - if ( d == current->domain ) > - break; > - > rc = -EINVAL; > if ( d->arch.hvm_domain.params[a.index] != 0 ) > break; > @@ -5733,22 +5764,12 @@ static int hvmop_set_param( > domctl_lock_release(); > break; > case HVM_PARAM_DM_DOMAIN: > - /* Not reflexive, as we may need to domain_pause(). */ > - rc = -EPERM; > - if ( d == current->domain ) > - break; > - > if ( a.value == DOMID_SELF ) > a.value = current->domain->domain_id; > > rc = hvm_set_dm_domain(d, a.value); > break; > case HVM_PARAM_ACPI_S_STATE: > - /* Not reflexive, as we must domain_pause(). */ > - rc = -EPERM; > - if ( current->domain == d ) > - break; > - > rc = 0; > if ( a.value == 3 ) > hvm_s3_suspend(d); > @@ -5761,20 +5782,9 @@ static int hvmop_set_param( > case HVM_PARAM_ACPI_IOPORTS_LOCATION: > rc = pmtimer_change_ioport(d, a.value); > break; > - case HVM_PARAM_MEMORY_EVENT_CR0: > - case HVM_PARAM_MEMORY_EVENT_CR3: > - case HVM_PARAM_MEMORY_EVENT_CR4: > - if ( d == current->domain ) > - rc = -EPERM; > - break; > case HVM_PARAM_MEMORY_EVENT_INT3: > case HVM_PARAM_MEMORY_EVENT_SINGLE_STEP: > case HVM_PARAM_MEMORY_EVENT_MSR: > - if ( d == current->domain ) > - { > - rc = -EPERM; > - break; > - } > if ( a.value & HVMPME_onchangeonly ) > rc = -EINVAL; > break; > @@ -5807,22 +5817,12 @@ static int hvmop_set_param( > rc = -EINVAL; > break; > case HVM_PARAM_IOREQ_SERVER_PFN: > - if ( d == current->domain ) > - { > - rc = -EPERM; > - break; > - } > d->arch.hvm_domain.ioreq_gmfn.base = a.value; > break; > case HVM_PARAM_NR_IOREQ_SERVER_PAGES: > { > unsigned int i; > > - if ( d == current->domain ) > - { > - rc = -EPERM; > - break; > - } > if ( a.value == 0 || > a.value > sizeof(d->arch.hvm_domain.ioreq_gmfn.mask) * 8 ) > { > @@ -5866,6 +5866,30 @@ static int hvmop_set_param( > return rc; > } > > +static int hvm_allow_get_param(struct domain *d, > + struct xen_hvm_param *a) > +{ > + int rc = 0; > + > + rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_get_param); > + if ( rc ) > + return rc; > + > + /* The following parameters cannot be read by the guest */ > + switch (a->index) > + { > + case HVM_PARAM_IOREQ_SERVER_PFN: > + case HVM_PARAM_NR_IOREQ_SERVER_PAGES: > + if ( d == current->domain ) > + rc = -EPERM; > + break; > + default: > + break; Again, this would be safer as a whitelist. ~Andrew > + } > + > + return rc; > +} > + > static int hvmop_get_param( > XEN_GUEST_HANDLE_PARAM(xen_hvm_param_t) arg) > { > @@ -5891,7 +5915,7 @@ static int hvmop_get_param( > && (a.index != HVM_PARAM_CALLBACK_IRQ) ) > goto out; > > - rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_get_param); > + rc = hvm_allow_get_param(d, &a); > if ( rc ) > goto out; > > @@ -5900,13 +5924,6 @@ static int hvmop_get_param( > case HVM_PARAM_ACPI_S_STATE: > a.value = d->arch.hvm_domain.is_s3_suspended ? 3 : 0; > break; > - case HVM_PARAM_IOREQ_SERVER_PFN: > - case HVM_PARAM_NR_IOREQ_SERVER_PAGES: > - if ( d == current->domain ) > - { > - rc = -EPERM; > - goto out; > - } > case HVM_PARAM_IOREQ_PFN: > case HVM_PARAM_BUFIOREQ_PFN: > case HVM_PARAM_BUFIOREQ_EVTCHN: { ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] x86/hvm: introduce functions for HVMOP_get/set_param allowance checks 2015-04-24 17:19 ` Andrew Cooper @ 2015-04-29 2:23 ` Paul Durrant 0 siblings, 0 replies; 12+ messages in thread From: Paul Durrant @ 2015-04-29 2:23 UTC (permalink / raw) To: Andrew Cooper, xen-devel@lists.xenproject.org; +Cc: Keir (Xen.org), Jan Beulich > -----Original Message----- > From: Andrew Cooper [mailto:andrew.cooper3@citrix.com] > Sent: 25 April 2015 01:19 > To: Paul Durrant; xen-devel@lists.xenproject.org > Cc: Keir (Xen.org); Jan Beulich > Subject: Re: [PATCH 2/3] x86/hvm: introduce functions for > HVMOP_get/set_param allowance checks > > On 24/04/15 17:21, Paul Durrant wrote: > > Some parameters can only (validly) be set once. Some cannot be set > > by a guest for its own domain. Consolidate these checks, along with > > the XSM check, in a new hvm_allow_set_param() function for clarity. > > > > Also, introduce hvm_allow_get_param() for similar reasons. > > > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com> > > Cc: Keir Fraser <keir@xen.org> > > Cc: Jan Beulich <jbeulich@suse.com> > > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > > --- > > xen/arch/x86/hvm/hvm.c | 141 +++++++++++++++++++++++++++-------- > ------------- > > 1 file changed, 79 insertions(+), 62 deletions(-) > > > > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > > index 87c47b1..23c604d 100644 > > --- a/xen/arch/x86/hvm/hvm.c > > +++ b/xen/arch/x86/hvm/hvm.c > > @@ -5640,6 +5640,57 @@ static int hvmop_set_evtchn_upcall_vector( > > return 0; > > } > > > > +static int hvm_allow_set_param(struct domain *d, > > + struct xen_hvm_param *a) > > const > Good point. > > +{ > > + uint64_t value = d->arch.hvm_domain.params[a->index]; > > + int rc = 0; > > No need for the 0 initialiser (and I know Coverity will complain about it). > > > + > > + rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_set_param); > > + if ( rc ) > > + return rc; > > + > > + /* The following parameters cannot be set by the guest */ > > + switch (a->index) > > style > Oops. > > + { > > + case HVM_PARAM_VIRIDIAN: > > + case HVM_PARAM_IDENT_PT: > > + case HVM_PARAM_DM_DOMAIN: > > + case HVM_PARAM_ACPI_S_STATE: > > + case HVM_PARAM_MEMORY_EVENT_CR0: > > + case HVM_PARAM_MEMORY_EVENT_CR3: > > + case HVM_PARAM_MEMORY_EVENT_CR4: > > + case HVM_PARAM_MEMORY_EVENT_INT3: > > + case HVM_PARAM_MEMORY_EVENT_SINGLE_STEP: > > + case HVM_PARAM_MEMORY_EVENT_MSR: > > + case HVM_PARAM_IOREQ_SERVER_PFN: > > + case HVM_PARAM_NR_IOREQ_SERVER_PAGES: > > + if ( d == current->domain ) > > + rc = -EPERM; > > + break; > > + default: > > + break; > > Please can we turn the guest-settable list into a whitelist rather than > a blacklist. We have had several XSAs in the past where a guest has > been able to modify a value it shouldn't have because new params were > default writable by everyone. > Yes, I think that's probably a good idea. > > + } > > + > > + if ( rc ) > > + return rc; > > + > > + /* The following parameters can only be changed once */ > > + switch (a->index) > > + { > > + case HVM_PARAM_VIRIDIAN: > > + case HVM_PARAM_IOREQ_SERVER_PFN: > > + case HVM_PARAM_NR_IOREQ_SERVER_PAGES: > > + if (value != 0 && a->value != value) > > + rc = -EEXIST; > > + break; > > + default: > > + break; > > + } > > + > > + return rc; > > +} > > + > > static int hvmop_set_param( > > XEN_GUEST_HANDLE_PARAM(xen_hvm_param_t) arg) > > { > > @@ -5666,7 +5717,7 @@ static int hvmop_set_param( > > && (a.index != HVM_PARAM_CALLBACK_IRQ) ) > > goto out; > > > > - rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_set_param); > > + rc = hvm_allow_set_param(d, &a); > > if ( rc ) > > goto out; > > > > @@ -5681,31 +5732,11 @@ static int hvmop_set_param( > > rc = -EINVAL; > > break; > > case HVM_PARAM_VIRIDIAN: > > - /* This should only ever be set once by the tools and read by the > guest. */ > > - rc = -EPERM; > > - if ( d == current->domain ) > > - break; > > - > > - if ( a.value != d->arch.hvm_domain.params[a.index] ) > > - { > > - rc = -EEXIST; > > - if ( d->arch.hvm_domain.params[a.index] != 0 ) > > - break; > > - > > + if ( (a.value & ~HVMPV_feature_mask) || > > + !(a.value & HVMPV_base_freq) ) > > rc = -EINVAL; > > - if ( (a.value & ~HVMPV_feature_mask) || > > - !(a.value & HVMPV_base_freq) ) > > - break; > > - } > > - > > - rc = 0; > > break; > > case HVM_PARAM_IDENT_PT: > > - /* Not reflexive, as we must domain_pause(). */ > > - rc = -EPERM; > > - if ( d == current->domain ) > > - break; > > - > > rc = -EINVAL; > > if ( d->arch.hvm_domain.params[a.index] != 0 ) > > break; > > @@ -5733,22 +5764,12 @@ static int hvmop_set_param( > > domctl_lock_release(); > > break; > > case HVM_PARAM_DM_DOMAIN: > > - /* Not reflexive, as we may need to domain_pause(). */ > > - rc = -EPERM; > > - if ( d == current->domain ) > > - break; > > - > > if ( a.value == DOMID_SELF ) > > a.value = current->domain->domain_id; > > > > rc = hvm_set_dm_domain(d, a.value); > > break; > > case HVM_PARAM_ACPI_S_STATE: > > - /* Not reflexive, as we must domain_pause(). */ > > - rc = -EPERM; > > - if ( current->domain == d ) > > - break; > > - > > rc = 0; > > if ( a.value == 3 ) > > hvm_s3_suspend(d); > > @@ -5761,20 +5782,9 @@ static int hvmop_set_param( > > case HVM_PARAM_ACPI_IOPORTS_LOCATION: > > rc = pmtimer_change_ioport(d, a.value); > > break; > > - case HVM_PARAM_MEMORY_EVENT_CR0: > > - case HVM_PARAM_MEMORY_EVENT_CR3: > > - case HVM_PARAM_MEMORY_EVENT_CR4: > > - if ( d == current->domain ) > > - rc = -EPERM; > > - break; > > case HVM_PARAM_MEMORY_EVENT_INT3: > > case HVM_PARAM_MEMORY_EVENT_SINGLE_STEP: > > case HVM_PARAM_MEMORY_EVENT_MSR: > > - if ( d == current->domain ) > > - { > > - rc = -EPERM; > > - break; > > - } > > if ( a.value & HVMPME_onchangeonly ) > > rc = -EINVAL; > > break; > > @@ -5807,22 +5817,12 @@ static int hvmop_set_param( > > rc = -EINVAL; > > break; > > case HVM_PARAM_IOREQ_SERVER_PFN: > > - if ( d == current->domain ) > > - { > > - rc = -EPERM; > > - break; > > - } > > d->arch.hvm_domain.ioreq_gmfn.base = a.value; > > break; > > case HVM_PARAM_NR_IOREQ_SERVER_PAGES: > > { > > unsigned int i; > > > > - if ( d == current->domain ) > > - { > > - rc = -EPERM; > > - break; > > - } > > if ( a.value == 0 || > > a.value > sizeof(d->arch.hvm_domain.ioreq_gmfn.mask) * 8 ) > > { > > @@ -5866,6 +5866,30 @@ static int hvmop_set_param( > > return rc; > > } > > > > +static int hvm_allow_get_param(struct domain *d, > > + struct xen_hvm_param *a) > > +{ > > + int rc = 0; > > + > > + rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_get_param); > > + if ( rc ) > > + return rc; > > + > > + /* The following parameters cannot be read by the guest */ > > + switch (a->index) > > + { > > + case HVM_PARAM_IOREQ_SERVER_PFN: > > + case HVM_PARAM_NR_IOREQ_SERVER_PAGES: > > + if ( d == current->domain ) > > + rc = -EPERM; > > + break; > > + default: > > + break; > > Again, this would be safer as a whitelist. > Hmm, maybe. I'll code it that way in v2 but it does seem to go against the general idea of HVM params (which AFAICT were originally analogous to start_info for PV guests i.e. for passing info from Xen to guests). Paul > ~Andrew > > > + } > > + > > + return rc; > > +} > > + > > static int hvmop_get_param( > > XEN_GUEST_HANDLE_PARAM(xen_hvm_param_t) arg) > > { > > @@ -5891,7 +5915,7 @@ static int hvmop_get_param( > > && (a.index != HVM_PARAM_CALLBACK_IRQ) ) > > goto out; > > > > - rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_get_param); > > + rc = hvm_allow_get_param(d, &a); > > if ( rc ) > > goto out; > > > > @@ -5900,13 +5924,6 @@ static int hvmop_get_param( > > case HVM_PARAM_ACPI_S_STATE: > > a.value = d->arch.hvm_domain.is_s3_suspended ? 3 : 0; > > break; > > - case HVM_PARAM_IOREQ_SERVER_PFN: > > - case HVM_PARAM_NR_IOREQ_SERVER_PAGES: > > - if ( d == current->domain ) > > - { > > - rc = -EPERM; > > - goto out; > > - } > > case HVM_PARAM_IOREQ_PFN: > > case HVM_PARAM_BUFIOREQ_PFN: > > case HVM_PARAM_BUFIOREQ_EVTCHN: { ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] x86/hvm: introduce functions for HVMOP_get/set_param allowance checks 2015-04-24 16:21 ` [PATCH 2/3] x86/hvm: introduce functions for HVMOP_get/set_param allowance checks Paul Durrant 2015-04-24 17:19 ` Andrew Cooper @ 2015-04-29 12:29 ` Jan Beulich 2015-05-01 13:39 ` Paul Durrant 2015-05-01 14:00 ` Paul Durrant 1 sibling, 2 replies; 12+ messages in thread From: Jan Beulich @ 2015-04-29 12:29 UTC (permalink / raw) To: paul.durrant; +Cc: andrew.cooper3, keir, xen-devel >>> Paul Durrant <paul.durrant@citrix.com> 04/24/15 6:35 PM >>> >+ /* The following parameters cannot be set by the guest */ Please add a stop ad the end of at least sentence-like comments. >case HVM_PARAM_DM_DOMAIN: >- /* Not reflexive, as we may need to domain_pause(). */ Especially when the reason for not allowing guest access is other than a simple permission thing, I think for documentation purposes it would be useful to retain these comments. a.value = current->domain->domain_id; >+ /* The following parameters cannot be read by the guest */ Perhaps "cannot" isn't the right term here - "shouldn't"? Jan ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] x86/hvm: introduce functions for HVMOP_get/set_param allowance checks 2015-04-29 12:29 ` Jan Beulich @ 2015-05-01 13:39 ` Paul Durrant 2015-05-01 14:00 ` Paul Durrant 1 sibling, 0 replies; 12+ messages in thread From: Paul Durrant @ 2015-05-01 13:39 UTC (permalink / raw) To: Jan Beulich; +Cc: Andrew Cooper, Keir (Xen.org), xen-devel@lists.xenproject.org > -----Original Message----- > From: Jan Beulich [mailto:jbeulich@suse.com] > Sent: 29 April 2015 13:29 > To: Paul Durrant > Cc: Andrew Cooper; xen-devel@lists.xenproject.org; Keir (Xen.org) > Subject: Re: [PATCH 2/3] x86/hvm: introduce functions for > HVMOP_get/set_param allowance checks > > >>> Paul Durrant <paul.durrant@citrix.com> 04/24/15 6:35 PM >>> > >+ /* The following parameters cannot be set by the guest */ > > Please add a stop ad the end of at least sentence-like comments. > Ok. I tend to skip full stops for single lines and only use them for multi-line. Just my habit though... > >case HVM_PARAM_DM_DOMAIN: > >- /* Not reflexive, as we may need to domain_pause(). */ > > Especially when the reason for not allowing guest access is other than a > simple permission thing, I think for documentation purposes it would be > useful to retain these comments. > I have acted upon Andrew's request for whitelists in v3 of the series, so I think this point is moot. I'll wait for your comments on v3 when I've posted it (which should be today with any luck). > a.value = current->domain->domain_id; > > >+ /* The following parameters cannot be read by the guest */ > > Perhaps "cannot" isn't the right term here - "shouldn't"? > Yes, you are probably correct. Paul > Jan ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] x86/hvm: introduce functions for HVMOP_get/set_param allowance checks 2015-04-29 12:29 ` Jan Beulich 2015-05-01 13:39 ` Paul Durrant @ 2015-05-01 14:00 ` Paul Durrant 1 sibling, 0 replies; 12+ messages in thread From: Paul Durrant @ 2015-05-01 14:00 UTC (permalink / raw) To: Jan Beulich; +Cc: Andrew Cooper, Keir (Xen.org), xen-devel@lists.xenproject.org > -----Original Message----- > From: Paul Durrant > Sent: 01 May 2015 14:39 > To: 'Jan Beulich' > Cc: Andrew Cooper; xen-devel@lists.xenproject.org; Keir (Xen.org) > Subject: RE: [PATCH 2/3] x86/hvm: introduce functions for > HVMOP_get/set_param allowance checks > > > -----Original Message----- > > From: Jan Beulich [mailto:jbeulich@suse.com] > > Sent: 29 April 2015 13:29 > > To: Paul Durrant > > Cc: Andrew Cooper; xen-devel@lists.xenproject.org; Keir (Xen.org) > > Subject: Re: [PATCH 2/3] x86/hvm: introduce functions for > > HVMOP_get/set_param allowance checks > > > > >>> Paul Durrant <paul.durrant@citrix.com> 04/24/15 6:35 PM >>> > > >+ /* The following parameters cannot be set by the guest */ > > > > Please add a stop ad the end of at least sentence-like comments. > > > > Ok. I tend to skip full stops for single lines and only use them for multi-line. > Just my habit though... > > > >case HVM_PARAM_DM_DOMAIN: > > >- /* Not reflexive, as we may need to domain_pause(). */ > > > > Especially when the reason for not allowing guest access is other than a > > simple permission thing, I think for documentation purposes it would be > > useful to retain these comments. > > > > I have acted upon Andrew's request for whitelists in v3 of the series, so I > think this point is moot. I'll wait for your comments on v3 when I've posted it > (which should be today with any luck). s/v3/v2 > > > a.value = current->domain->domain_id; > > > > >+ /* The following parameters cannot be read by the guest */ > > > > Perhaps "cannot" isn't the right term here - "shouldn't"? > > > > Yes, you are probably correct. > > Paul > > > Jan ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] x86/hvm: disallow guest get and set of all ioreq server HVM params 2015-04-24 16:21 [PATCH 0/2] x86/hvm: HVMOP_get/set_param improvements Paul Durrant 2015-04-24 16:21 ` [PATCH 1/3] x86/hvm: give HVMOP_set_param and HVMOP_get_param their own functions Paul Durrant 2015-04-24 16:21 ` [PATCH 2/3] x86/hvm: introduce functions for HVMOP_get/set_param allowance checks Paul Durrant @ 2015-04-24 16:21 ` Paul Durrant 2015-04-24 16:23 ` [PATCH 0/2] x86/hvm: HVMOP_get/set_param improvements Paul Durrant 3 siblings, 0 replies; 12+ messages in thread From: Paul Durrant @ 2015-04-24 16:21 UTC (permalink / raw) To: xen-devel; +Cc: Andrew Cooper, Paul Durrant, Keir Fraser, Jan Beulich A guest has no need to touch these parameters and reading HVM_PARAM_IOREQ_PFN, HVM_PARAM_BUFIOREQ_PFN, or HVM_PARAM_BUFIOREQ_EVTCHN may cause Xen to create a default ioreq server where one did not already exist. Signed-off-by: Paul Durrant <paul.durrant@citrix.com> Cc: Keir Fraser <keir@xen.org> Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> --- xen/arch/x86/hvm/hvm.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 23c604d..b51c1d5 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -5665,6 +5665,9 @@ static int hvm_allow_set_param(struct domain *d, case HVM_PARAM_MEMORY_EVENT_MSR: case HVM_PARAM_IOREQ_SERVER_PFN: case HVM_PARAM_NR_IOREQ_SERVER_PAGES: + case HVM_PARAM_IOREQ_PFN: + case HVM_PARAM_BUFIOREQ_PFN: + case HVM_PARAM_BUFIOREQ_EVTCHN: if ( d == current->domain ) rc = -EPERM; break; @@ -5880,6 +5883,10 @@ static int hvm_allow_get_param(struct domain *d, { case HVM_PARAM_IOREQ_SERVER_PFN: case HVM_PARAM_NR_IOREQ_SERVER_PAGES: + case HVM_PARAM_IOREQ_PFN: + case HVM_PARAM_BUFIOREQ_PFN: + case HVM_PARAM_BUFIOREQ_EVTCHN: + case HVM_PARAM_DM_DOMAIN: if ( d == current->domain ) rc = -EPERM; break; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] x86/hvm: HVMOP_get/set_param improvements 2015-04-24 16:21 [PATCH 0/2] x86/hvm: HVMOP_get/set_param improvements Paul Durrant ` (2 preceding siblings ...) 2015-04-24 16:21 ` [PATCH 3/3] x86/hvm: disallow guest get and set of all ioreq server HVM params Paul Durrant @ 2015-04-24 16:23 ` Paul Durrant 3 siblings, 0 replies; 12+ messages in thread From: Paul Durrant @ 2015-04-24 16:23 UTC (permalink / raw) To: Paul Durrant, xen-devel@lists.xenproject.org The subject line should obviously be prefixed [PATCH 0/3], but the content is at least correct. Apologies for that, Paul > -----Original Message----- > From: Paul Durrant [mailto:paul.durrant@citrix.com] > Sent: 24 April 2015 17:21 > To: xen-devel@lists.xenproject.org > Cc: Paul Durrant > Subject: [PATCH 0/2] x86/hvm: HVMOP_get/set_param improvements > > The following 3 patches re-structure the code implementing > HVMOP_set_param > and HVMOP_get_param. > > Patch #1 gives each operation its own function > > Patch #2 splits out checks for getting/setting non-reflexive params and > setting params with change-once semantics, as well as the XSM check into > separate functions > > Patch #3 adds all remaining ioreq server params to non-reflexive lists ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-05-01 14:01 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-04-24 16:21 [PATCH 0/2] x86/hvm: HVMOP_get/set_param improvements Paul Durrant 2015-04-24 16:21 ` [PATCH 1/3] x86/hvm: give HVMOP_set_param and HVMOP_get_param their own functions Paul Durrant 2015-04-24 17:11 ` Andrew Cooper 2015-04-29 2:16 ` Paul Durrant 2015-04-24 16:21 ` [PATCH 2/3] x86/hvm: introduce functions for HVMOP_get/set_param allowance checks Paul Durrant 2015-04-24 17:19 ` Andrew Cooper 2015-04-29 2:23 ` Paul Durrant 2015-04-29 12:29 ` Jan Beulich 2015-05-01 13:39 ` Paul Durrant 2015-05-01 14:00 ` Paul Durrant 2015-04-24 16:21 ` [PATCH 3/3] x86/hvm: disallow guest get and set of all ioreq server HVM params Paul Durrant 2015-04-24 16:23 ` [PATCH 0/2] x86/hvm: HVMOP_get/set_param improvements Paul Durrant
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.