Messages in this thread |  | | Date | Fri, 10 Apr 2015 13:21:32 -0700 | From | Andi Kleen <> | Subject | Re: [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
|  |