Re: [Xen-devel] [PATCH v2 1/2] x86/entry/32: Introduce and use X86_BUG_ESPFIX instead of paravirt_enabled

[Borislav Petkov <bp@xxxxxxxxx>]

On Mon, Feb 29, 2016 at 03:50:19PM -0800, Andy Lutomirski wrote:
> x86_64 has very clean espfix handling on paravirt: espfix64 is set
> up in native_iret, so paravirt systems that override iret bypass
> espfix64 automatically.  This is robust and straightforward.
> 
> x86_32 is messier.  espfix is set up before the IRET paravirt patch
> point, so it can't be directly conditionalized on whether we use
> native_iret.  We also can't easily move it into native_iret without
> regressing performance due to a bizarre consideration.  Specifically,
> on 64-bit kernels, the logic is:
> 
>   if (regs->ss & 0x4)
>           setup_espfix;
> 
> On 32-bit kernels, the logic is:
> 
>   if ((regs->ss & 0x4) && (regs->cs & 0x3) == 3 &&
>       (regs->flags & X86_EFLAGS_VM) == 0)
>           setup_espfix;
> 
> The performance of setup_espfix itself is essentially irrelevant, but
> the comparison happens on every IRET so its performance matters.  On
> x86_64, there's no need for any registers except flags to implement
> the comparison, so we fold the whole thing into native_iret.  On
> x86_32, we don't do that because we need a free register to
> implement the comparison efficiently.  We therefore do espfix setup
> before restoring registers on x86_32.
> 
> This patch gets rid of the explicit paravirt_enabled check by
> introducing X86_BUG_ESPFIX on 32-bit systems and using an ALTERNATIVE
> to skip espfix on paravirt systems where iret != native_iret.  This is
> also messy, but it's at least in line with other things we do.
> 
> This improves espfix performance by removing a branch, but no one
> cares.  More importantly, it removes a paravirt_enabled user, which is
> good because paravirt_enabled is ill-defined and is going away.
> 
> Signed-off-by: Andy Lutomirski <luto@xxxxxxxxxx>
> ---
>  arch/x86/entry/entry_32.S          | 15 ++-------------
>  arch/x86/include/asm/cpufeatures.h |  8 ++++++++
>  arch/x86/kernel/cpu/common.c       | 25 +++++++++++++++++++++++++
>  3 files changed, 35 insertions(+), 13 deletions(-)

Reviewed-by: Borislav Petkov <bp@xxxxxxx>

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

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