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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 02.07.18 at 15:15, <aisaila@xxxxxxxxxxxxxxx> wrote:
> On Lu, 2018-07-02 at 09:38 +0000, Paul Durrant wrote:
>> > From: Alexandru Isaila [mailto:aisaila@xxxxxxxxxxxxxxx]
>> > Sent: 28 June 2018 10:26
>> > --- 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;
>> Jan also queried the return values from the _save_one functions. I
>> assume you need to make your 'continue' value reasonably global such
>> that it can still be used when you move the iteration up in the outer
>> save function. He suggested passing through the return of
>> hvm_save_entry() but we also need to be sure it won't clash will the
>> 'continue' value in future so it may now be necessary to actually
>> define what the possible return values are.
>>
> It returns 0 for ok and -1 for error so I think the return check could
> be removed.
> 
> But whit this we will introduce a potential fail point if someone
> changes the return of hvm_save_entry(). At this moment the patch works
> both ways. Its up to Jan to have the last word here if he is sure that
> I should just return hvm_save_entry().

So far I've not seen an approach that does not alter current behavior
and at the same time allows for the CONTINUE extension you mean to
introduce. One option is to have affected layers return an enum instead
of a pseudo-boolean.

Jan



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