On Vi, 2018-07-13 at 04:34 -0600, Jan Beulich wrote: > > > > > > > > > > > > > On 13.07.18 at 11:04, <aisa...@bitdefender.com> wrote: > > --- a/xen/arch/x86/hvm/viridian.c > > +++ b/xen/arch/x86/hvm/viridian.c > > @@ -1026,20 +1026,26 @@ 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; > > > > - 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, > > - }; > > + 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 hvm_save_entry(VIRIDIAN_VCPU, v->vcpu_id, h, &ctxt); > So you now hand on the return value here, but just to ... > > > > > +} > > + > > +static int viridian_save_vcpu_ctxt(struct domain *d, > > hvm_domain_context_t *h) > > +{ > > + struct vcpu *v; > > + > > + for_each_vcpu( d, v ) { > > + if ( viridian_save_vcpu_ctxt_one(v, h) != 0 ) > > return 1; > ... not pass it on here. Is that really what we want? The vMCE case > does > it differently, for example. Which means - there are certainly > inconsistencies right now, but since you have to touch all of this > anyway, > couldn't you make things end in a more consistent shape after this > rework?
In the past there where either returning the value from hvm_save_entry()(like in the hvm_save_tsc_adjust) or check the return value(like in the hvm_save_cpu_ctxt ) like it did here. I made all the save_one funcs return the value from hvm_save_entry() and then check it in the save func so that they all return in the same way. I don't say that the old way was bad but it was not consistent. And yes I have missed the if() check int he save func in the vmce but I think it's better to have the check in place there and have all the save / save_one funcs consistent. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel