[Bug target/70220] [x86] interrupt attribute optionally needs to provide read, write and control the set of saved registers

["wink at saville dot com" <gcc-bugzilla at gcc dot gnu dot org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70220

--- Comment #9 from Wink Saville <wink at saville dot com> ---
(In reply to H.J. Lu from comment #8)
> (In reply to Wink Saville from comment #7)
> >
> > In my opinion, even if rbp is special, it still needs to be available in the
> > struct full_stack_frame.
> 
> The whole idea of extending interrupter attribute is to avoid
> full_stack_frame.
> ISR shouldn't assume that compiler will save and restore registers in any
> specific way, order or form.  As far as ISR is concerned, it is just black
> magic.

>From my perspective the goal is not to avoid struct full_stack_frame but to
allow it to be created and always consistent.

> 
> > Also, isn't rsp special? Would the compiler or programmer be responsible for
> > initializing it. It seems to me it has to be the compiler, but could be
> > wrong.
> 
> If you change rsp in asm statement, you have to restore it in asm statement.

The rsp is always saved/restored by the hardware, and your struct frame pointer
provides access to it so no problem there. It is special because when there are
local variables the compiler needs to modify it and currently it does that
after the registers are saved. How will we coordinate the compiler initializing
rsp and the programming saving the registers?

One possibility I see is that the programmer adds struct saved_regs as a local
variable (or TLS or ...) and then saves and restores the registers to/from
there. So assuming rbp is special and always points to itself then maybe
something like:

struct saved_regs {
  ac_u64 rax;
  ac_u64 rdx;
  ac_u64 rcx;
  ac_u64 rbx;
  ac_u64 rsi;
  ac_u64 rdi;
  ac_u64 r8;
  ac_u64 r9;
  ac_u64 r10;
  ac_u64 r11;
  ac_u64 r12;
  ac_u64 r13;
  ac_u64 r14;
  ac_u64 r15;
  ac_u64 rbp;
};

__attribute__((__interrupt__(all)))
void test_isr(struct intr_frame* frame) {
  struct saved_regs sr;

  __asm__ volatile("mov %%rax, %0;" : "=m"(sr.rax));
...
  __asm__ volatile("mov %%r15, %0;" : "=m"(sr.r15));
  __asm__ volatile("mov (%rbp), %rax;");
  __asm__ volatile("mov %%rax, %0;" : "=m"(sr.rbp));

  ac_printf("timer_reschedule_isr sr.rax=%x\n", sr.rax);

  tcb_x86 *ptcb = thread_scheduler((ac_u8*)get_sp(), get_ss());
  __asm__ volatile("movq %0, %%rsp;" :: "rm" (ptcb->sp) : "rsp");
  __asm__ volatile("movw %0, %%ss;" :: "rm" (ptcb->ss));
  set_apic_timer_initial_count(ptcb->slice);

  __atomic_add_fetch(&timer_reschedule_isr_counter, 1, __ATOMIC_RELEASE);

  send_apic_eoi();

  __asm__ volatile("mov %0, %%rax;" :: "m"(sr.rbp));
  __asm__ volatile("mov %rax, (%rbp);");
  __asm__ volatile("mov %0, %%r15;" :: "m"(sr.r15));
...
  __asm__ volatile("mov %0, %%rax;" :: "m"(sr.rax));
}

The current compiler generates:

0000000000100490 <test_isr>:
  100490:       55                      push   %rbp
  100491:       48 89 e5                mov    %rsp,%rbp
  100494:       41 53                   push   %r11
  100496:       41 52                   push   %r10
  100498:       41 51                   push   %r9
  10049a:       41 50                   push   %r8
  10049c:       57                      push   %rdi
  10049d:       56                      push   %rsi
  10049e:       51                      push   %rcx
  10049f:       52                      push   %rdx
  1004a0:       50                      push   %rax
  1004a1:       48 83 e4 f0             and    $0xfffffffffffffff0,%rsp
  1004a5:       48 83 c4 80             add    $0xffffffffffffff80,%rsp
  1004a9:       fc                      cld    
  1004aa:       48 89 44 24 08          mov    %rax,0x8(%rsp)
  1004af:       4c 89 7c 24 78          mov    %r15,0x78(%rsp)
  1004b4:       48 8b 45 00             mov    0x0(%rbp),%rax
  1004b8:       48 89 44 24 38          mov    %rax,0x38(%rsp)

...

  10050d:       48 8b 44 24 38          mov    0x38(%rsp),%rax
  100512:       4c 8b 7c 24 78          mov    0x78(%rsp),%r15
  100517:       48 89 45 00             mov    %rax,0x0(%rbp)
  10051b:       48 8b 44 24 08          mov    0x8(%rsp),%rax
  100520:       48 8d 65 b8             lea    -0x48(%rbp),%rsp
  100524:       58                      pop    %rax
  100525:       5a                      pop    %rdx
  100526:       59                      pop    %rcx
  100527:       5e                      pop    %rsi
  100528:       5f                      pop    %rdi
  100529:       41 58                   pop    %r8
  10052b:       41 59                   pop    %r9
  10052d:       41 5a                   pop    %r10
  10052f:       41 5b                   pop    %r11
  100531:       5d                      pop    %rbp
  100532:       48 cf                   iretq  


Obviously the "new" compiler wouldn't be saving/restoring r11..rax, that would
be the programmers responsibility, but it looks like it works.