lkml.org 
[lkml]   [2015]   [Apr]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 4/8] x86: Add support for rd/wr fs/gs base
> We never run paranoid_exit if we interrupted userspace, and we can't
> context switch on the IST stack, so I don't see how this is possible.
>
> > - Restore from R15 (with FSGSBASE), if the gs base was saved
> > in R15
>
> What about case 4: we interrupted the kernel with usergs? (The code
> seems more correct in this regard, but this description above is
> confusing to me.)

I'll fix the description.

> > estacks = per_cpu(debug_stack, cpu);
> > + /* Store GS at bottom of stack for bootstrap access */
> > + *(void **)estacks = gs;
> > estacks += exception_stack_sizes[v];
> > oist->ist[v] = t->x86_tss.ist[v] =
> > (unsigned long)estacks;
>
> Seems reasonable to me.
>
> You could possibly simplify some things if you wrote the address to
> the bottom of *each* debug stack. Then you wouldn't need the extra
> alignment stuff.

It would waste 16K or so per CPU. I don't think the if is a problem.


> > +/*
> > + * Stack layout:
> > + * +16 pt_regs
> > + * +8 stack mask for ist or 0
>
> What does that mean?
>
> Oh, I get it. It's the size of the IST stack we're on. Let's please
> make all IST stacks the same size as suggested above and get rid of
> this. After all, they really are all the same size -- there's just
> more than one debug stack.

I don't want to waste the memory here. A few instructions more is much
preferable.

> > + movq_cfi rdx, RDX+OFF
> > + movq_cfi rcx, RCX+OFF
> > + movq_cfi rax, RAX+OFF
> > + movq %r8, R8+OFF(%rsp)
> > + movq %r9, R9+OFF(%rsp)
> > + movq %r10, R10+OFF(%rsp)
> > + movq %r11, R11+OFF(%rsp)
> > + movq_cfi rbx, RBX+OFF
> > + movq %rbp, RBP+OFF(%rsp)
> > + movq %r12, R12+OFF(%rsp)
> > + movq %r13, R13+OFF(%rsp)
> > + movq %r14, R14+OFF(%rsp)
> > + movq %r15, R15+OFF(%rsp)
> > + movq $-1,ORIG_RAX+OFF(%rsp) /* no syscall to restart */
> > +33:
> > + ASM_NOP5 /* May be replaced with jump to paranoid_save_gs */
>
> Is there some reason that the normal ALTERNATIVE macro can't be used here?

This is assembler, not C.

Since you asked for such extensive use I added a new macro for it now.

> > +.macro idtentry sym do_sym has_error_code:req paranoid=0 shift_ist=-1 \
> > + stack_mask=-EXCEPTION_STKSZ
>
> This can be removed as well, I think.

No with the different sized stacks.


-Andi
--
ak@linux.intel.com -- Speaking for myself only


\
 
 \ /
  Last update: 2015-04-10 23:01    [W:0.083 / U:8.720 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site