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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 14/02/17 10:29, Jan Beulich wrote:
> They're all solely dependent on guest type, so we don't need to repeat
> all the same four pointers in every vCPU control structure. Instead use
> static const structures, and store pointers to them in the domain
> control structure.
>
> Since touching it anyway, take the opportunity and move schedule_tail()
> into the only C file needing it.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

+1.  This has been a niggle on my todo list for ages.

> @@ -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.

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

~Andrew

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