Re: [Xen-devel] [PATCH v8 07/11] x86/hvm: Introduce viridian_save_vcpu_ctxt_one() func

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Alexandru Isaila [mailto:aisaila@xxxxxxxxxxxxxxx]
> Sent: 25 June 2018 15:51
> To: xen-devel@xxxxxxxxxxxxx
> Cc: Ian Jackson <Ian.Jackson@xxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>;
> jbeulich@xxxxxxxx; Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Paul
> Durrant <Paul.Durrant@xxxxxxxxxx>; Alexandru Isaila
> <aisaila@xxxxxxxxxxxxxxx>
> Subject: [PATCH v8 07/11] x86/hvm: Introduce
> viridian_save_vcpu_ctxt_one() func
> 
> This is used to save data from a single instance.
> 
> Signed-off-by: Alexandru Isaila <aisaila@xxxxxxxxxxxxxxx>
> ---
>  xen/arch/x86/hvm/viridian.c | 26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
> index 694eae6..ed164ab 100644
> --- a/xen/arch/x86/hvm/viridian.c
> +++ b/xen/arch/x86/hvm/viridian.c
> @@ -1026,20 +1026,30 @@ static int viridian_load_domain_ctxt(struct
> domain *d, hvm_domain_context_t *h)
>  HVM_REGISTER_SAVE_RESTORE(VIRIDIAN_DOMAIN,
> viridian_save_domain_ctxt,
>                            viridian_load_domain_ctxt, 1, HVMSR_PER_DOM);
> 
> -static int viridian_save_vcpu_ctxt(struct domain *d,
> hvm_domain_context_t *h)
> +static int viridian_save_vcpu_ctxt_one(struct vcpu *v,
> hvm_domain_context_t *h)
>  {
> -    struct vcpu *v;
> +    struct hvm_viridian_vcpu_context ctxt;
> 
> -    if ( !is_viridian_domain(d) )
> +    if ( !is_viridian_domain(v->domain) )
>          return 0;
> 
> +    memset(&ctxt, 0, sizeof(ctxt));
> +    ctxt.vp_assist_msr = v->arch.hvm_vcpu.viridian.vp_assist.msr.raw;
> +    ctxt.vp_assist_pending = v->arch.hvm_vcpu.viridian.vp_assist.pending;
> +
> +    if ( hvm_save_entry(VIRIDIAN_VCPU, v->vcpu_id, h, &ctxt) != 0 )
> +        return 1;
> +    return 0;

Unlike the other patch I commented on, here you are actually processing the 
result of hvm_save_entry() by testing it for non-zero. It all seems very 
arbitrary.

> +}
> +
> +static int viridian_save_vcpu_ctxt(struct domain *d,
> hvm_domain_context_t *h)
> +{
> +    struct vcpu *v;
> +
>      for_each_vcpu( d, v ) {
> -        struct hvm_viridian_vcpu_context ctxt = {
> -            .vp_assist_msr = v->arch.hvm_vcpu.viridian.vp_assist.msr.raw,
> -            .vp_assist_pending = v->arch.hvm_vcpu.viridian.vp_assist.pending,
> -        };
> +        int rc = viridian_save_vcpu_ctxt_one(v, h);
> 
> -        if ( hvm_save_entry(VIRIDIAN_VCPU, v->vcpu_id, h, &ctxt) != 0 )
> +        if ( rc != 0 )
>              return 1;
>      }
> 
> --
> 2.7.4


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