diff mbox series

[3/5] xen/wait: Minor asm improvements

Message ID 20220718071825.22113-4-andrew.cooper3@citrix.com (mailing list archive)
State New, archived
Headers show
Series xen/wait: Improvements | expand

Commit Message

Andrew Cooper July 18, 2022, 7:18 a.m. UTC 
There is no point preserving all registers.  Instead, preserve an arbitrary 6
registers, and list the rest as clobbered.  This does not alter the register
scheduling at all, but does reduce the amount of state needing saving.

Use a named parameter for page size, instead of needing to parse which is
parameter 3.  Adjust the formatting of the parameters slightly to simply the

Comments

Jan Beulich July 18, 2022, 10:04 a.m. UTC | #1 
On 18.07.2022 09:18, Andrew Cooper wrote:
> --- a/xen/common/wait.c
> +++ b/xen/common/wait.c
> @@ -151,13 +151,12 @@ static void __prepare_to_wait(struct waitqueue_vcpu *wqv)
>       * copies in from wqv->stack over the active stack.
>       */
>      asm volatile (
> -        "push %%rax; push %%rbx; push %%rdx; push %%rbp;"
> -        "push %%r8;  push %%r9;  push %%r10; push %%r11;"
> -        "push %%r12; push %%r13; push %%r14; push %%r15;"
> +        "push %%rbx; push %%rbp; push %%r12;"
> +        "push %%r13; push %%r14; push %%r15;"
>  
>          "sub %%esp,%%ecx;"
> -        "cmp %3,%%ecx;"
> -        "ja .L_skip;"
> +        "cmp %[sz], %%ecx;"
> +        "ja .L_skip;"       /* Bail if >4k */
>          "mov %%rsp,%%rsi;"
>  
>          /* check_wakeup_from_wait() longjmp()'s to this point. */
> @@ -165,12 +164,12 @@ static void __prepare_to_wait(struct waitqueue_vcpu *wqv)
>          "mov %%rsp,%%rsi;"
>  
>          ".L_skip:"
> -        "pop %%r15; pop %%r14; pop %%r13; pop %%r12;"
> -        "pop %%r11; pop %%r10; pop %%r9;  pop %%r8;"
> -        "pop %%rbp; pop %%rdx; pop %%rbx; pop %%rax"
> +        "pop %%r15; pop %%r14; pop %%r13;"
> +        "pop %%r12; pop %%rbp; pop %%rbx;"
>          : "=&S" (wqv->esp), "=&c" (dummy), "=&D" (dummy)
> -        : "i" (PAGE_SIZE), "0" (0), "1" (cpu_info), "2" (wqv->stack)
> -        : "memory" );
> +        : "0" (0), "1" (cpu_info), "2" (wqv->stack),
> +          [sz] "i" (PAGE_SIZE)
> +        : "memory", "rax", "rdx", "r8", "r9", "r10", "r11" );
>  
>      if ( unlikely(wqv->esp == 0) )
>      {
> @@ -224,11 +223,12 @@ void check_wakeup_from_wait(void)
>       * All other GPRs are available for use; they're either restored from
>       * wqv->stack or explicitly clobbered.
>       */

Ah, this is where the comment stops being misleading. I think it would
be better if you had it correct there and adjusted it here.

> -    asm volatile (
> -        "mov %1,%%"__OP"sp; jmp .L_wq_resume;"
> -        : : "S" (wqv->stack), "D" (wqv->esp),
> -          "c" ((char *)get_cpu_info() - (char *)wqv->esp)
> -        : "memory" );
> +    asm volatile ( "mov %%rdi, %%rsp;"
> +                   "jmp .L_wq_resume;"

Minor remark: No need for trailing semicolons like this one. Anyway:
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan
diff mbox series

Patch

diff of the subsequent patch.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
---
 xen/common/wait.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/xen/common/wait.c b/xen/common/wait.c
index 4dcfa17a8a3f..4bc030d1a09d 100644
--- a/xen/common/wait.c
+++ b/xen/common/wait.c
@@ -151,13 +151,12 @@  static void __prepare_to_wait(struct waitqueue_vcpu *wqv)
      * copies in from wqv->stack over the active stack.
      */
     asm volatile (
-        "push %%rax; push %%rbx; push %%rdx; push %%rbp;"
-        "push %%r8;  push %%r9;  push %%r10; push %%r11;"
-        "push %%r12; push %%r13; push %%r14; push %%r15;"
+        "push %%rbx; push %%rbp; push %%r12;"
+        "push %%r13; push %%r14; push %%r15;"
 
         "sub %%esp,%%ecx;"
-        "cmp %3,%%ecx;"
-        "ja .L_skip;"
+        "cmp %[sz], %%ecx;"
+        "ja .L_skip;"       /* Bail if >4k */
         "mov %%rsp,%%rsi;"
 
         /* check_wakeup_from_wait() longjmp()'s to this point. */
@@ -165,12 +164,12 @@  static void __prepare_to_wait(struct waitqueue_vcpu *wqv)
         "mov %%rsp,%%rsi;"
 
         ".L_skip:"
-        "pop %%r15; pop %%r14; pop %%r13; pop %%r12;"
-        "pop %%r11; pop %%r10; pop %%r9;  pop %%r8;"
-        "pop %%rbp; pop %%rdx; pop %%rbx; pop %%rax"
+        "pop %%r15; pop %%r14; pop %%r13;"
+        "pop %%r12; pop %%rbp; pop %%rbx;"
         : "=&S" (wqv->esp), "=&c" (dummy), "=&D" (dummy)
-        : "i" (PAGE_SIZE), "0" (0), "1" (cpu_info), "2" (wqv->stack)
-        : "memory" );
+        : "0" (0), "1" (cpu_info), "2" (wqv->stack),
+          [sz] "i" (PAGE_SIZE)
+        : "memory", "rax", "rdx", "r8", "r9", "r10", "r11" );
 
     if ( unlikely(wqv->esp == 0) )
     {
@@ -224,11 +223,12 @@  void check_wakeup_from_wait(void)
      * All other GPRs are available for use; they're either restored from
      * wqv->stack or explicitly clobbered.
      */
-    asm volatile (
-        "mov %1,%%"__OP"sp; jmp .L_wq_resume;"
-        : : "S" (wqv->stack), "D" (wqv->esp),
-          "c" ((char *)get_cpu_info() - (char *)wqv->esp)
-        : "memory" );
+    asm volatile ( "mov %%rdi, %%rsp;"
+                   "jmp .L_wq_resume;"
+                   :
+                   : "S" (wqv->stack), "D" (wqv->esp),
+                     "c" ((char *)get_cpu_info() - (char *)wqv->esp)
+                   : "memory" );
     unreachable();
 }