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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Mar 19, 2018 at 07:41:42AM -0600, 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>
> Tested-by: Juergen Gross <jgross@xxxxxxxx>
> Reviewed-by: Juergen Gross <jgross@xxxxxxxx>
> ---
> v2: Add ASSERT(!in_irq()).
> 
> --- a/xen/arch/x86/pv/domain.c
> +++ b/xen/arch/x86/pv/domain.c
> @@ -219,10 +219,22 @@ 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)
>  {
> +    ASSERT(!in_irq());
> +
>      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).
> +     */

Not sure I follow the comment. If you're talking about
create_bounce_frame, it wouldn't call this function in the first place,
right?

> +    if ( !force_cr3 && !(v->arch.flags & TF_kernel_mode) )

Also, it takes a bit of mental power to see !(v->arch.flags &
TF_kernel_mode) means the mode Xen is using. Can you maybe just use a
variable at the beginning like

   bool kernel_mode = v->arch.flags & TF_kernel_mode;

and then use it here?

> +        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" );
>  
> @@ -252,13 +264,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);
>  }
>  
>  /*
> 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/xen-devel

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