[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] x86/time: switch platform timer hooks to altcall
On 12/01/2022 08:58, Jan Beulich wrote: > Except in the "clocksource=tsc" case we can replace the indirect calls > involved in accessing the platform timers by direct ones, as they get > established once and never changed. > > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> > --- > Sort of RFC, for both the whether and the how aspects. > > TBD: Overriding X86_FEATURE_ALWAYS is somewhat dangerous; there's only > no issue with e.g. hvm_set_tsc_offset() used later in time.c > because that's an inline function which did already "latch" the > usual value of the macro. But the alternative of introducing an > alternative_call() variant allowing to specify the controlling > feature also doesn't look overly nice to me either. Then again the > .resume hook invocation could be patched unconditionally, as the > TSC clocksource leaves this hook set to NULL. > > --- a/xen/arch/x86/alternative.c > +++ b/xen/arch/x86/alternative.c > @@ -268,8 +268,7 @@ static void init_or_livepatch _apply_alt > * point the branch destination is still NULL, insert "UD2; UD0" > * (for ease of recognition) instead of CALL/JMP. > */ > - if ( a->cpuid == X86_FEATURE_ALWAYS && > - *(int32_t *)(buf + 1) == -5 && > + if ( *(int32_t *)(buf + 1) == -5 && I'm afraid that this must not become conditional. One of the reasons I was hesitant towards the mechanics of altcall in the first place was that it intentionally broke Spectre v2 protections by manually writing out a non-retpoline'd indirect call. This is made safe in practice because all altcall sites either get converted to a direct call, or rewritten to be a UD2. If you want to make altcalls conversions conditional, then the code gen must be rearranged to use INDIRECT_CALL first, but that comes with overheads too because then call callsites would load the function pointer into a register, just to not use it in the patched case. I suspect it will be easier to figure out how to make the TSC case also invariant after boot. ~Andrew
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |