From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.5 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 46935ECDE44 for ; Wed, 24 Oct 2018 19:43:26 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id EE10B20831 for ; Wed, 24 Oct 2018 19:43:25 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="aWfX7h2B" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org EE10B20831 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727199AbeJYEMr (ORCPT ); Thu, 25 Oct 2018 00:12:47 -0400 Received: from mail.kernel.org ([198.145.29.99]:60234 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725994AbeJYEMq (ORCPT ); Thu, 25 Oct 2018 00:12:46 -0400 Received: from mail-wm1-f42.google.com (mail-wm1-f42.google.com [209.85.128.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id E984820838 for ; Wed, 24 Oct 2018 19:43:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1540410203; bh=/7S5W1p4mQlQiMVukMxG2Hl5Z1udO4DYAFseVZHZJTc=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=aWfX7h2B7Fpf7kjIiQk3vb2MCq57vk6JKrAwcB3dB/Eaqp0wemJFhU0CYp18g9Kk1 +KvC7gmJNQOtu/MsPE7o0zqPluXA7pgv50FmRA/Hi/vJIDTqvnyKPM7L3wvV3E4iVd 1dmp1mgflM7ord7BJfx3dFItJtf0yLCYIrsN/1xc= Received: by mail-wm1-f42.google.com with SMTP id b14-v6so3997997wmj.3 for ; Wed, 24 Oct 2018 12:43:22 -0700 (PDT) X-Gm-Message-State: AGRZ1gLVZ+bBBpcTxLZpbWzDH98O3pX5sEKz1NKKoUMEknSullo50/jE o8HIlp/ZeUoHWJ5lLqTI+UPT7B+FVAcpZtVq3WAVyA== X-Google-Smtp-Source: AJdET5d1NuGc2p0AFI2jg0S9BOPZ9tUl2EWLGCAVf93p6Niz3dSA1/2va9CoBsVf77sN8/GnCPJl+robUlKAumeHFrA= X-Received: by 2002:a1c:f312:: with SMTP id q18-v6mr4007453wmq.14.1540410201338; Wed, 24 Oct 2018 12:43:21 -0700 (PDT) MIME-Version: 1.0 References: <1537312139-5580-1-git-send-email-chang.seok.bae@intel.com> <1537312139-5580-3-git-send-email-chang.seok.bae@intel.com> In-Reply-To: From: Andy Lutomirski Date: Wed, 24 Oct 2018 12:43:09 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [regression in -rc1] Re: [PATCH v6 2/8] x86/fsgsbase/64: Introduce FS/GS base helper functions To: "Bae, Chang Seok" Cc: Andrew Lutomirski , Ingo Molnar , Thomas Gleixner , "H. Peter Anvin" , Andi Kleen , Dave Hansen , "Metzger, Markus T" , "Ravi V. Shankar" , LKML Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Oct 24, 2018 at 12:29 PM Bae, Chang Seok wrote: > > On Wed, Oct 24, 2018 at 12:22 PM Andy Lutomirski > > On Wed, Oct 24, 2018 at 12:13 PM Bae, Chang Seok > > wrote: > > > > > > On Tue, Sep 18, 2018 at 12:02 PM Andy Lutomirski > > > > On Tue, Sep 18, 2018 at 4:09 PM Chang S. Bae > > > > > > > > wrote: > > > > > > > > > > With new helpers, FS/GS base access is centralized. > > > > > Eventually, when FSGSBASE instruction enabled, it will be faster. > > > > > > > > Sorry for not catching this during review, but: > > > > > > > > > +void x86_fsbase_write_cpu(unsigned long fsbase) { > > > > > + /* > > > > > + * Set the selector to 0 as a notion, that the segment base is > > > > > + * overwritten, which will be checked for skipping the segment load > > > > > + * during context switch. > > > > > + */ > > > > > + loadseg(FS, 0); > > > > > > > > ^^^ > > > > > > > > what? > > > > > > > > > + wrmsrl(MSR_FS_BASE, fsbase); } > > > > > > > > I don't understand what the comment is trying to say, but the sole > > > > caller so far of this function is x86_gsbase_write_task(), and the code looks > > incorrect. > > > > > > > > Ingo, I think we need to address this during this merge window, > > > > probably by removing the comment and the loadseg() call (and the > > > > same for gsbase...inactive). But first, Chang, can you explain what > > > > exactly your intent is here? > > > > > > It's coming from do_arch_prctl_64(). If you think it really makes > > > confusion in x86_fsbase_write_cpu(), how about moving it to > > x86_fsbase_write_task()? > > > > Why should ..write_task() magically change the index but only if it's writing > > current? > > > > I think you should move it all the way out to the caller > > (do_arch_prctl_64()?) and we can see if it makes sense there. > > > > Okay. x86_fsbase_write_task() doesn't make sense. > Then, it should rollback that helper and call x86_fsbase_write_cpu() only > from ptrace. Same for gsbase. Sounds okay? > I think x86_fsbase_write_task() makes plenty of sense, but I think that callers need to be aware that the effect of writing a nonzero fsbase *and* a nonzero fsindex is bizarre on non-FSGSBASE systems. So that code should go in the callers. The oddities involved have little to do with whether the caller is writing to current or to something else. Arguably the code should be entirely split out into the code that writes current (arch_prctl()) and the code that writes a stopped task (ptrace). I don't think there are any code paths that genuinely can write either. --Andy --Andy