Re: [PATCH 17/24] x86/entry: Introduce struct ist_regs

From: Lai Jiangshan
Date: Fri Sep 10 2021 - 03:13:14 EST




On 2021/9/10 12:31, H. Peter Anvin wrote:
Note: the examples in this email all compiled with:

gcc -O2 -mpreferred-stack-boundary=3 -mcmodel=kernel

The disassembly has been slightly simplified.

Saving results in registers is non-flexible no matter in ASM or hack in C like this.

Saving CR3 in ist_regs is not different than saving rax in pt_regs, and both of
ist_regs and embedded pt_regs can be moved when stack is required to switch.

I prefer to my original design.



On 9/9/21 5:18 PM, Lai Jiangshan wrote:

This patch was over designed.

In ASM code, we can easily save results in the callee-saved registers.
For example, rc3 is saved in %r14, gsbase info is saved in %rbx.

And in C code, we can't save results in registers.  And I thought there was
no place to save the results because the CR3 and gsbase are not kernel's.
So I extended the pt_regs to ist_regs to save the results.

But it was incorrect.  The results can be saved in percpu data at the end of
paranoid_entry() after the CR3/gsbase are settled down.  And the results
can be read at the beginning of paranoid_exit() before the CR3/gsbase are
switched to the interrupted context's.


OK, count me confused. Of course you can save results in caller-saved registers in C; it is kind of what they do:


extern void bar(void);

unsigned long foo(unsigned long v)
{
    bar();
    return v;
}

0000000000000000 <foo>:
   0:   41 54                   push   %r12
   2:   49 89 fc                mov    %rdi,%r12
   5:   e8 00 00 00 00          callq  bar
   a:   4c 89 e0                mov    %r12,%rax
   d:   41 5c                   pop    %r12
   f:   c3                      retq

Now, if you need to specify *which* registers, you have to declare them as global register variables - NOT local (which have completely different semantics). This also means that you (probably) want to isolate this code into its own compilation unit, because it will prevent any other code in the same .c file from using that register as well.

For example:

register unsigned long r14 asm("%r14");
unsigned long foo(unsigned long v)
{
    r14 = v;
    bar();
    v = r14;
    return v;
}

0000000000000000 <foo>:
   0:   49 89 fe                mov    %rdi,%r14
   3:   e8 00 00 00 00          callq  bar
   8:   4c 89 f0                mov    %r14,%rax
   b:   c3                      retq

WARNING: This also means that gcc will happily discard the old value in %r14, so if you need it preserved you have to do so explicitly; if you are called direct from assembly and are happy to lose the value then the above code is fine -- and it is even slightly more efficient!

For preserving the old r14, in this case:

register unsigned long r14 asm("%r14");
unsigned long foo(unsigned long v)
{
    unsigned long saved_r14 = r14;
    r14 = v;
    bar();
    v = r14;
    r14 = saved_r14;
    return v;
}

0000000000000000 <foo>:
   0:   53                      push   %rbx
   1:   4c 89 f3                mov    %r14,%rbx
   4:   49 89 fe                mov    %rdi,%r14
   7:   e8 00 00 00 00          callq  bar
   c:   4c 89 f0                mov    %r14,%rax
   f:   49 89 de                mov    %rbx,%r14
  12:   5b                      pop    %rbx
  13:   c3                      retq


HOWEVER, if you are relying on not using the stack, then using C code is probably very much not a good idea. It is very hard to guarantee that just because the C compiler is *currently* not using a stack, that it won't do so *in the future*.

Again, finally, local register variables DO NOT WORK, this does NOT do what you expect:

unsigned long foo(unsigned long v)
{
    register unsigned long r14 asm("%r14") = v;
    bar();
    return r14;
}

0000000000000000 <foo>:
   0:   41 54                   push   %r12
   2:   49 89 fc                mov    %rdi,%r12
   5:   e8 00 00 00 00          callq  bar
   a:   4c 89 e0                mov    %r12,%rax
   d:   41 5c                   pop    %r12
   f:   c3                      retq