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

2018-03-15 Thread 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

2018-03-15 Thread 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

2018-03-15 Thread 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

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

2018-03-13 Thread Jan Beulich
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)
 .Lcstar_cr3_okay:
 sti
 .Lcstar_cr3_end:
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -185,7 +185,7 @@ ENTRY(lstar_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)
 .Llstar_cr3_okay:
 sti
 .Llstar_cr3_end:
@@ -295,7 +295,7 @@ GLOBAL(sysenter_eflags_saved)
 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)
 .Lsyse_cr3_okay:
 sti
 .Lsyse_cr3_end:
@@ -348,7 +348,7 @@ ENTRY(int80_direct_trap)
 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)
 .Lint80_cr3_okay:
 sti
 .Lint80_cr3_end:
@@ -562,10 +562,9 @@ ENTRY(common_interrupt)
 neg   %rcx
 .Lintr_cr3_load:
 mov   %rcx, %cr3
-xor   %ecx, %ecx
-mov   %rcx, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
+mov   %r12, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
 testb $3, UREGS_cs(%rsp)
-cmovnz %rcx, %r15
+cmovnz %r12, %r15
 .Lintr_cr3_okay:
 
 CR4_PV32_RESTORE
@@ -610,10 +609,9 @@ GLOBAL(handle_exception)
 neg   %rcx
 .Lxcpt_cr3_load:
 mov   %rcx, %cr3
-xor   %ecx, %ecx
-mov   %rcx, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
+mov   %r12, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
 testb $3, UREGS_cs(%rsp)
-cmovnz %rcx, %r15
+cmovnz %r12, %r15
 .Lxcpt_cr3_okay:
 
 handle_exception_saved:
@@ -838,7 +836,7 @@ handle_ist_exception:
 neg   %rcx
 .List_cr3_load:
 mov   %rcx, %cr3
-movq  $0, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
+mov   %r12, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
 .List_cr3_okay:
 
 CR4_PV32_RESTORE




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