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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.03.18 at 14:20, <wei.liu2@xxxxxxxxxx> wrote:
> On Mon, Mar 19, 2018 at 07:41:42AM -0600, Jan Beulich wrote:
>> --- 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?

Right. The comment is talking about what may happen after we
return from here.

>> +    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?

Except for the (how I would say) clutter by the extra local variable
I don't see much of a difference.

Jan


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