Re: [Xen-devel] [PATCH 2/2] x86: package up context switch hook pointers

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 14.02.17 at 16:26, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 14/02/17 10:29, Jan Beulich wrote:
>> @@ -2066,6 +2073,15 @@ static void __context_switch(void)
>>      per_cpu(curr_vcpu, cpu) = n;
>>  }
>>  
>> +/*
>> + * Schedule tail *should* be a terminal function pointer, but leave a 
>> bugframe
>> + * around just incase it returns, to save going back into the context
>> + * switching code and leaving a far more subtle crash to diagnose.
>> + */
>> +#define schedule_tail(vcpu) do {                          \
>> +        (((vcpu)->domain->arch.ctxt_switch->tail)(vcpu)); \
>> +        BUG();                                            \
>> +    } while (0)
> 
> schedule_tail() is used only twice.  I'd suggest dropping it entirely
> and calling the ->tail() function pointer normally, rather than hiding
> it this.

I had considered this too, and now that you ask for it I'll happily
do so.

> Upon reviewing, this patch, don't we also need a ->same() call in the
> continue_same() path in the previous patch?

No, I did specifically check this already: The call to continue_same()
sits (far) ahead of the clearing of ->is_running, and as long as that
flag is set, vcpu_pause() (or vcpu_sleep_sync(), to be precise) will
spin as necessary.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel