Re: [PATCH 18/25] [PATCH] turn priviled operations into macros in entry.S

Thank you for the attention, andi

let's go:

On 8/8/07, Andi Kleen [off-list ref] wrote:

quoted

+#define SYSRETQ                                              \
+                     movq    %gs:pda_oldrsp,%rsp;    \
+                     swapgs;                         \
+                     sysretq;

When the macro does more than sysret it should have a different
name

That's fair. Again, suggestions are welcome. Maybe SYSCALL_RETURN ?

quoted

  */
      .globl int_ret_from_sys_call
 int_ret_from_sys_call:
-     cli
+     DISABLE_INTERRUPTS(CLBR_ANY)

ANY? There are certainly some registers alive at this point like rax

yes, this one is wrong. Thanks for the catch

quoted

 retint_restore_args:
-     cli
+     DISABLE_INTERRUPTS(CLBR_ANY)

Similar.

I don't think so. They are live here, but restore_args follows, so we
can safely clobber anything here. Right?

quoted

      /*
       * The iretq could re-enable interrupts:
       */

@@ -566,10 +587,14 @@ retint_restore_args:
 restore_args:
      RESTORE_ARGS 0,8,0
 iret_label:
-     iretq
+#ifdef CONFIG_PARAVIRT
+     INTERRUPT_RETURN
+ENTRY(native_iret)

ENTRY adds alignment. Why do you need that export anyways?

Just went on the flow. Will change.

quoted

+#endif
+1:   iretq

      .section __ex_table,"a"
-     .quad iret_label,bad_iret
+     .quad 1b, bad_iret

iret_label seems more expressive to me than 1

fair.

quoted

+     ENABLE_INTERRUPTS(CLBR_NONE)

In many of the CLBR_NONEs there are actually some registers free;
but it might be safer to keep it this way. But if some client can get
significantly better code with one or two free registers it might
be worthwhile to investigate.

That's exactly what I had in mind. I'd highly prefer to keep it this
way until it is merged, and we are sure all the rest is stable

quoted

-     swapgs
+     SWAPGS_NOSTACK

There's still stack here

Yes, but it is not safe to use. I think Roasted addressed it later on.

quoted

 paranoid_restore\trace:
      RESTORE_ALL 8
-     iretq
+     INTERRUPT_RETURN

I suspect Xen will need much more changes anyways because of its
ring 3 guest. Are these changes sufficient for lguest?

Yes, they are sufficient for lguest.
Does any xen folks have any comment?

-- 
Glauber de Oliveira Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."