Re: [Xen-devel] [PATCH 4/4] x86: avoid double CR3 reload when switching to guest user mode

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 23/01/18 10:38, Jan Beulich wrote:
> When XPTI is active, the CR3 load in restore_all_guest is sufficient
> when switching to user mode, improving in particular system call and
> page fault exit paths for the guest.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

While I can see the utility of this, we are starting to get into
complicated territory as to which cr3 is loaded.  Also, the name
"toggle" is no longer strictly accurate.

That being said, all of these helpers are only used in synchronous
contexts as far as I can tell, so some ASSERT(!in_irq()) would probably
go a long way.

>
> --- a/xen/arch/x86/pv/domain.c
> +++ b/xen/arch/x86/pv/domain.c
> @@ -220,10 +220,20 @@ int pv_domain_initialise(struct domain *
>      return rc;
>  }
>  
> -static void _toggle_guest_pt(struct vcpu *v)
> +static void _toggle_guest_pt(struct vcpu *v, bool force_cr3)
>  {
>      v->arch.flags ^= TF_kernel_mode;
>      update_cr3(v);
> +
> +    /*
> +     * There's no need to load CR3 here when it is going to be loaded on the
> +     * way out to guest mode again anyway, and when the page tables we're
> +     * currently on are the kernel ones (whereas when switching to kernel
> +     * mode we need to be able to write a bounce frame onto the kernel 
> stack).
> +     */
> +    if ( !force_cr3 && !(v->arch.flags & TF_kernel_mode) )
> +        return;
> +
>      /* Don't flush user global mappings from the TLB. Don't tick TLB clock. 
> */
>      asm volatile ( "mov %0, %%cr3" : : "r" (v->arch.cr3) : "memory" );
>  
> @@ -253,13 +263,13 @@ void toggle_guest_mode(struct vcpu *v)
>      }
>      asm volatile ( "swapgs" );
>  
> -    _toggle_guest_pt(v);
> +    _toggle_guest_pt(v, cpu_has_no_xpti);
>  }
>  
>  void toggle_guest_pt(struct vcpu *v)
>  {
>      if ( !is_pv_32bit_vcpu(v) )
> -        _toggle_guest_pt(v);
> +        _toggle_guest_pt(v, true);

This can be converted as well.  The only callers are the LDT-fault and
I/O perm check, both when we are currently on user pagetables, needing
to switch to kernel briefly, then back to user.

However, it would be better to drop the parameter and feed
cpu_has_no_xpti into the conditional above which explains why it is safe
to do.

~Andrew

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