Re: [Xen-devel] [PATCH v7 08/15] x86/cpu: Remove loop form vmce_save_vcpu_ctxt() func

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Alexandru Isaila [mailto:aisaila@xxxxxxxxxxxxxxx]
> Sent: 07 June 2018 15:59
> 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 v7 08/15] x86/cpu: Remove loop form
> vmce_save_vcpu_ctxt() func
> 
> Signed-off-by: Alexandru Isaila <aisaila@xxxxxxxxxxxxxxx>
> ---
>  xen/arch/x86/cpu/mcheck/vmce.c | 27 +++++++--------------------
>  1 file changed, 7 insertions(+), 20 deletions(-)
> 
> diff --git a/xen/arch/x86/cpu/mcheck/vmce.c
> b/xen/arch/x86/cpu/mcheck/vmce.c
> index 404f27e..ead1f73 100644
> --- a/xen/arch/x86/cpu/mcheck/vmce.c
> +++ b/xen/arch/x86/cpu/mcheck/vmce.c
> @@ -349,30 +349,17 @@ int vmce_wrmsr(uint32_t msr, uint64_t val)
>      return ret;
>  }
> 
> -static void vmce_save_vcpu_ctxt_one(struct vcpu *v, struct
> hvm_vmce_vcpu *ctxt)
> -{
> -    ctxt->caps = v->arch.vmce.mcg_cap;
> -    ctxt->mci_ctl2_bank0 = v->arch.vmce.bank[0].mci_ctl2;
> -    ctxt->mci_ctl2_bank1 = v->arch.vmce.bank[1].mci_ctl2;
> -    ctxt->mcg_ext_ctl = v->arch.vmce.mcg_ext_ctl;
> -}
> -
>  static int vmce_save_vcpu_ctxt(struct domain *d, hvm_domain_context_t
> *h)
>  {
> -    struct vcpu *v;
> -    int err = 0;
> -
> -    for_each_vcpu ( d, v )
> -    {
> -        struct hvm_vmce_vcpu ctxt;
> +    struct hvm_vmce_vcpu ctxt;
> +    struct vcpu *v = NULL;
> 
> -        vmce_save_vcpu_ctxt_one(v, &ctxt);
> -        err = hvm_save_entry(VMCE_VCPU, v->vcpu_id, h, &ctxt);
> -        if ( err )
> -            break;
> -    }
> +    ctxt.caps = v->arch.vmce.mcg_cap;

There's a typo in the commit title (s/form/from), but I don't understand what 
you're doing here. You set v to NULL above and dereference it below. AFAICT, 
until patch #15 is applied context saving will be completely broken.

  Paul

> +    ctxt.mci_ctl2_bank0 = v->arch.vmce.bank[0].mci_ctl2;
> +    ctxt.mci_ctl2_bank1 = v->arch.vmce.bank[1].mci_ctl2;
> +    ctxt.mcg_ext_ctl = v->arch.vmce.mcg_ext_ctl;
> 
> -    return err;
> +    return hvm_save_entry(VMCE_VCPU, v->vcpu_id, h, &ctxt);
>  }
> 
>  static int vmce_load_vcpu_ctxt(struct domain *d, hvm_domain_context_t
> *h)
> --
> 2.7.4


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