From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756100AbbDJUVq (ORCPT ); Fri, 10 Apr 2015 16:21:46 -0400 Received: from mga01.intel.com ([192.55.52.88]:50799 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754365AbbDJUVd (ORCPT ); Fri, 10 Apr 2015 16:21:33 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.11,558,1422950400"; d="scan'208";a="678444546" Date: Fri, 10 Apr 2015 13:21:32 -0700 From: Andi Kleen To: Andy Lutomirski Cc: Andi Kleen , X86 ML , Andrew Lutomirski , "linux-kernel@vger.kernel.org" , Steven Rostedt , Borislav Petkov Subject: Re: [PATCH 4/8] x86: Add support for rd/wr fs/gs base Message-ID: <20150410202132.GQ15335@tassilo.jf.intel.com> References: <1428681033-1549-1-git-send-email-andi@firstfloor.org> <1428681033-1549-5-git-send-email-andi@firstfloor.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > 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