Re: [Xen-devel] [PATCH v5 01/18] x86/hvm: introduce hvm_copy_context_and_params

2020-01-22 Thread Tamas K Lengyel
On Wed, Jan 22, 2020 at 8:01 AM Jan Beulich  wrote:
>
> On 21.01.2020 18:49, Tamas K Lengyel wrote:
> > Currently the hvm parameters are only accessible via the HVMOP hypercalls. 
> > In
> > this patch we introduce a new function that can copy both the hvm context 
> > and
> > parameters directly into a target domain. No functional changes in existing
> > code.
> >
> > Signed-off-by: Tamas K Lengyel 
>
> In reply to my v4 comments you said "I don't have any objections to the
> things you pointed out." Yet only one aspect was actually changed here.
> It also doesn't help that there's no brief summary of the changes done
> for v5. I guess I'm confused.

Indeed it seems I missed some of your previous requests. I was halfway
through making the modifications but simply forgot to do the rest
after I stepped away. I still don't have any objections to them
though, so will catch up on it in v6.

Thanks,
Tamas

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v5 01/18] x86/hvm: introduce hvm_copy_context_and_params

2020-01-22 Thread Jan Beulich
On 21.01.2020 18:49, Tamas K Lengyel wrote:
> Currently the hvm parameters are only accessible via the HVMOP hypercalls. In
> this patch we introduce a new function that can copy both the hvm context and
> parameters directly into a target domain. No functional changes in existing
> code.
> 
> Signed-off-by: Tamas K Lengyel 

In reply to my v4 comments you said "I don't have any objections to the
things you pointed out." Yet only one aspect was actually changed here.
It also doesn't help that there's no brief summary of the changes done
for v5. I guess I'm confused.

Jan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v5 01/18] x86/hvm: introduce hvm_copy_context_and_params

2020-01-21 Thread Tamas K Lengyel
Currently the hvm parameters are only accessible via the HVMOP hypercalls. In
this patch we introduce a new function that can copy both the hvm context and
parameters directly into a target domain. No functional changes in existing
code.

Signed-off-by: Tamas K Lengyel 
---
 xen/arch/x86/hvm/hvm.c| 240 +-
 xen/include/asm-x86/hvm/hvm.h |   2 +
 2 files changed, 151 insertions(+), 91 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 4723f5d09c..651998e783 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4067,16 +4067,17 @@ static int hvmop_set_evtchn_upcall_vector(
 }
 
 static int hvm_allow_set_param(struct domain *d,
-   const struct xen_hvm_param *a)
+   uint32_t index,
+   uint64_t new_value)
 {
-uint64_t value = d->arch.hvm.params[a->index];
+uint64_t value = d->arch.hvm.params[index];
 int rc;
 
 rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_set_param);
 if ( rc )
 return rc;
 
-switch ( a->index )
+switch ( index )
 {
 /* The following parameters can be set by the guest. */
 case HVM_PARAM_CALLBACK_IRQ:
@@ -4109,7 +4110,7 @@ static int hvm_allow_set_param(struct domain *d,
 if ( rc )
 return rc;
 
-switch ( a->index )
+switch ( index )
 {
 /* The following parameters should only be changed once. */
 case HVM_PARAM_VIRIDIAN:
@@ -4119,7 +4120,7 @@ static int hvm_allow_set_param(struct domain *d,
 case HVM_PARAM_NR_IOREQ_SERVER_PAGES:
 case HVM_PARAM_ALTP2M:
 case HVM_PARAM_MCA_CAP:
-if ( value != 0 && a->value != value )
+if ( value != 0 && new_value != value )
 rc = -EEXIST;
 break;
 default:
@@ -4129,49 +4130,32 @@ static int hvm_allow_set_param(struct domain *d,
 return rc;
 }
 
-static int hvmop_set_param(
-XEN_GUEST_HANDLE_PARAM(xen_hvm_param_t) arg)
+static int hvm_set_param(struct domain *d, uint32_t index, uint64_t value)
 {
 struct domain *curr_d = current->domain;
-struct xen_hvm_param a;
-struct domain *d;
-struct vcpu *v;
 int rc;
+struct vcpu *v;
 
-if ( copy_from_guest(, arg, 1) )
-return -EFAULT;
-
-if ( a.index >= HVM_NR_PARAMS )
+if ( index >= HVM_NR_PARAMS )
 return -EINVAL;
 
-/* Make sure the above bound check is not bypassed during speculation. */
-block_speculation();
-
-d = rcu_lock_domain_by_any_id(a.domid);
-if ( d == NULL )
-return -ESRCH;
-
-rc = -EINVAL;
-if ( !is_hvm_domain(d) )
-goto out;
-
-rc = hvm_allow_set_param(d, );
+rc = hvm_allow_set_param(d, index, value);
 if ( rc )
 goto out;
 
-switch ( a.index )
+switch ( index )
 {
 case HVM_PARAM_CALLBACK_IRQ:
-hvm_set_callback_via(d, a.value);
+hvm_set_callback_via(d, value);
 hvm_latch_shinfo_size(d);
 break;
 case HVM_PARAM_TIMER_MODE:
-if ( a.value > HVMPTM_one_missed_tick_pending )
+if ( value > HVMPTM_one_missed_tick_pending )
 rc = -EINVAL;
 break;
 case HVM_PARAM_VIRIDIAN:
-if ( (a.value & ~HVMPV_feature_mask) ||
- !(a.value & HVMPV_base_freq) )
+if ( (value & ~HVMPV_feature_mask) ||
+ !(value & HVMPV_base_freq) )
 rc = -EINVAL;
 break;
 case HVM_PARAM_IDENT_PT:
@@ -4181,7 +4165,7 @@ static int hvmop_set_param(
  */
 if ( !paging_mode_hap(d) || !cpu_has_vmx )
 {
-d->arch.hvm.params[a.index] = a.value;
+d->arch.hvm.params[index] = value;
 break;
 }
 
@@ -4196,7 +4180,7 @@ static int hvmop_set_param(
 
 rc = 0;
 domain_pause(d);
-d->arch.hvm.params[a.index] = a.value;
+d->arch.hvm.params[index] = value;
 for_each_vcpu ( d, v )
 paging_update_cr3(v, false);
 domain_unpause(d);
@@ -4205,23 +4189,23 @@ static int hvmop_set_param(
 break;
 case HVM_PARAM_DM_DOMAIN:
 /* The only value this should ever be set to is DOMID_SELF */
-if ( a.value != DOMID_SELF )
+if ( value != DOMID_SELF )
 rc = -EINVAL;
 
-a.value = curr_d->domain_id;
+value = curr_d->domain_id;
 break;
 case HVM_PARAM_ACPI_S_STATE:
 rc = 0;
-if ( a.value == 3 )
+if ( value == 3 )
 hvm_s3_suspend(d);
-else if ( a.value == 0 )
+else if ( value == 0 )
 hvm_s3_resume(d);
 else
 rc = -EINVAL;
 
 break;
 case HVM_PARAM_ACPI_IOPORTS_LOCATION:
-rc = pmtimer_change_ioport(d, a.value);
+rc = pmtimer_change_ioport(d, value);
 break;
 case HVM_PARAM_MEMORY_EVENT_CR0:
 case HVM_PARAM_MEMORY_EVENT_CR3:
@@ -4236,24 +4220,24 @@ static int hvmop_set_param(
 rc =