Re: [Xen-devel] [PATCH v3 4/6] x86/XPTI: use %r12 to write zero into xen_cr3

[Andrew Cooper]

On 15/03/18 16:39, Jan Beulich wrote:
 On 15.03.18 at 17:02,  wrote:
>> On 13/03/18 13:49, Jan Beulich wrote:
>>> Now that we zero all registers early on all entry paths, use that to
>>> avoid a couple of immediates here.
>>>
>>> Signed-off-by: Jan Beulich 
>>> ---
>>> We may want to consider eliminating a few more $0 this way. But
>>> especially for byte ones I'm not sure it's worth it, due to the REX
>>> prefix the use of %r12 would incur.
>>>
>>> --- a/xen/arch/x86/x86_64/compat/entry.S
>>> +++ b/xen/arch/x86/x86_64/compat/entry.S
>>> @@ -216,7 +216,7 @@ ENTRY(cstar_enter)
>>>  mov   %rcx, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
>>>  neg   %rcx
>>>  mov   %rcx, %cr3
>>> -movq  $0, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
>>> +mov   %r12, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
>> It is unreasonable to expect people to realise that this use of %r12 is
>> for a zero, because there is no write to %r12 visible.  These need some
>> kind of comment.
> Well, okay, I'll add the same comment in all 7 places.

With that change, Acked-by: Andrew Cooper 

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

Re: [Xen-devel] [PATCH v3 4/6] x86/XPTI: use %r12 to write zero into xen_cr3

[Jan Beulich]

>>> On 15.03.18 at 17:02,  wrote:
> On 13/03/18 13:49, Jan Beulich wrote:
>> Now that we zero all registers early on all entry paths, use that to
>> avoid a couple of immediates here.
>>
>> Signed-off-by: Jan Beulich 
>> ---
>> We may want to consider eliminating a few more $0 this way. But
>> especially for byte ones I'm not sure it's worth it, due to the REX
>> prefix the use of %r12 would incur.
>>
>> --- a/xen/arch/x86/x86_64/compat/entry.S
>> +++ b/xen/arch/x86/x86_64/compat/entry.S
>> @@ -216,7 +216,7 @@ ENTRY(cstar_enter)
>>  mov   %rcx, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
>>  neg   %rcx
>>  mov   %rcx, %cr3
>> -movq  $0, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
>> +mov   %r12, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
> 
> It is unreasonable to expect people to realise that this use of %r12 is
> for a zero, because there is no write to %r12 visible.  These need some
> kind of comment.

Well, okay, I'll add the same comment in all 7 places.

Jan


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

Re: [Xen-devel] [PATCH v3 4/6] x86/XPTI: use %r12 to write zero into xen_cr3

[Andrew Cooper]

On 13/03/18 13:49, Jan Beulich wrote:
> Now that we zero all registers early on all entry paths, use that to
> avoid a couple of immediates here.
>
> Signed-off-by: Jan Beulich 
> ---
> We may want to consider eliminating a few more $0 this way. But
> especially for byte ones I'm not sure it's worth it, due to the REX
> prefix the use of %r12 would incur.
>
> --- a/xen/arch/x86/x86_64/compat/entry.S
> +++ b/xen/arch/x86/x86_64/compat/entry.S
> @@ -216,7 +216,7 @@ ENTRY(cstar_enter)
>  mov   %rcx, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
>  neg   %rcx
>  mov   %rcx, %cr3
> -movq  $0, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
> +mov   %r12, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)

It is unreasonable to expect people to realise that this use of %r12 is
for a zero, because there is no write to %r12 visible.  These need some
kind of comment.  Perhaps:

diff --git a/xen/arch/x86/x86_64/compat/entry.S 
b/xen/arch/x86/x86_64/compat/entry.S

index f52bffc..14c87a0 100644

--- a/xen/arch/x86/x86_64/compat/entry.S

+++ b/xen/arch/x86/x86_64/compat/entry.S

@@ -215,7 +215,7 @@ ENTRY(cstar_enter)

 mov   %rcx, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)

 neg   %rcx

 mov   %rcx, %cr3

-    movq  $0, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)

+    movq  %r12, STACK_CPUINFO_FIELD(xen_cr3)(%rbx) /* Arbitrary zero reg. 
*/

 .Lcstar_cr3_okay:

 sti

 
~Andrew

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