From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: ARC-Seal: i=1; a=rsa-sha256; t=1517164393; cv=none; d=google.com; s=arc-20160816; b=Lt+KFv7HuNfBupkdUXvouU8N2j1/+yrmX0qNdNua+BGEbqTkz1oaIvSQaisC7iLu61 F6YN2/4B2ELuZI8svpceFrWDCDYkA8TCuuCgCsJaFAs2m30cwRuKc2GxXTeD6NcjXelJ BDjjkz/6q3pPk7TgEx3cKG6N/Dswx4vFPWy6KrT5U1mLY3j12eCv4ehoFMc0lDSMN2A5 XhW5ruictUaTTmU22sXTEqyANukCS9yb5+Zgm/XnB0et0iR7ptGVXMbgtKuIrLPIl8h8 lqCjEGRUTXaZG5hwsvjNQbCQA02zl+SmD5k8C5XzlsuQjjpAGkHQqhinNfmDKz8FIw9P X9ww== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:sender:dkim-signature :arc-authentication-results; bh=yNQ5rNefHX8PqarGCrWwdoiiqpV5Nwnkfisx2jR1MTU=; b=rnlXFYvZs6MV+F6STB3o1X6g7M2BZp1tMfaO+eKZ3n2tC7f0TYuu6Zm6RS+px3UUZ4 bnEf0AFb/jazkKRjQFDMstJqW4aK6jKIT9W58p0smQ3OBWQOiZvSNERkUmssVhZCWjjo yY/qsVRxJMUOcR8vCongYZvIRQebilxWJZIFYOyNc35wzem8ejyMqsZ4cKVU9te9PK6C bc8yzeIfa1qH41bnfCFYew3Mk0dKehnUh9PLN6/r6j5tT+Hwxo33BcftQCSSE5xnXfc1 oiCwHHWm7bWy3aDkOqEglxaa/1sCofOkFkzmiTtBQsBORbDo5VgiYIXMcal0lboC+tGW qK1g== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=tKyXR6KQ; spf=pass (google.com: domain of mingo.kernel.org@gmail.com designates 209.85.220.41 as permitted sender) smtp.mailfrom=mingo.kernel.org@gmail.com Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=tKyXR6KQ; spf=pass (google.com: domain of mingo.kernel.org@gmail.com designates 209.85.220.41 as permitted sender) smtp.mailfrom=mingo.kernel.org@gmail.com X-Google-Smtp-Source: AH8x2268DxruZpHxGaeS58RaU+15uvQSgXI/uqJeq9IPHkPxo9dUVVvAsxhyHpSh5i4tn3M491CmZw== Sender: Ingo Molnar Date: Sun, 28 Jan 2018 19:33:09 +0100 From: Ingo Molnar To: Dan Williams Cc: Thomas Gleixner , linux-arch , Cyril Novikov , Kernel Hardening , Peter Zijlstra , Catalin Marinas , X86 ML , Will Deacon , Russell King , Ingo Molnar , Greg KH , "H. Peter Anvin" , Linus Torvalds , Alan Cox , Linux Kernel Mailing List Subject: Re: [PATCH v5 02/12] array_idx: sanitize speculative array de-references Message-ID: <20180128183309.j7zkoyqblich4zhq@gmail.com> References: <151703971300.26578.1185595719337719486.stgit@dwillia2-desk3.amr.corp.intel.com> <151703972396.26578.7326612698912543866.stgit@dwillia2-desk3.amr.corp.intel.com> <20180128085500.djlm5rlbhjlpfj4i@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170609 (1.8.3) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: =?utf-8?q?1590732017653801778?= X-GMAIL-MSGID: =?utf-8?q?1590862170646096600?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: * Dan Williams wrote: > Thomas, Peter, and Alexei wanted s/nospec_barrier/ifence/ and I just checked past discussions, and I cannot find that part, got any links or message-IDs? PeterZ's feedback on Jan 8 was: > On Sun, Jan 07, 2018 at 06:24:11PM -0800, Alexei Starovoitov wrote: > > How about: > > CONFIG_SPECTRE1_WORKAROUND_INDEX_MASK > > CONFIG_SPECTRE1_WORKAROUND_LOAD_FENCE > > INSTRUCTION_FENCE if anything. LFENCE for Intel (and now also for AMD as > per 0592b0bce169) is a misnomer, IFENCE would be a better name for it. Which in that context clearly talked about the config space and how to name the instruction semantics in light of the confusion of LFENCE and MFENCE opcodes on Intel and AMD CPUs... Also, those early reviews were fundamentally low level feedback related to the actual functionality of the barriers and their mapping to the hardware. But the fact is, the current series introduces an inconsistent barrier namespace extension of: barrier() barrier_data() mb() rmb() wmb() store_mb() read_barrier_depends() ... + ifence() + array_idx() + array_idx_mask() This isn't bikeshed painting: _ALL_ existing barrier API names have 'barrier' or its abbreviation 'mb' (memory barrier) somewhere in their names, which makes it pretty easy to recognize them at a glance. I'm giving you high level API naming feedback because we are now growing the size of the barrier API. array_idx() on the other hand is pretty much close to a 'worst possible' name: - it's named in an overly generic, opaque fashion - doesn't indicate it at all that it's a barrier for something ... and since we want all kernel developers to use these facilities correctly, we want the names to be good and suggestive as well. I'd accept pretty much anything else that adds 'barrier' or 'nospec' to the name: array_idx_barrier() or array_idx_nospec(). (I'm slightly leaning towards 'nospec' because it's more indicative of what is being done, and it also is what we do for get uaccess APIs.) ifence() is a similar departure from existing barrier naming nomenclature, and I'd accept pretty much any other variant: barrier_nospec() ifence_nospec() The kernel namespace cleanliness rules are clear and consistent, and there's nothing new about them: - the name of the API should unambiguously refer back to the API category. For barriers this common reference is 'barrier' or 'mb'. - use postfixes or prefixes consistently: pick one and don't mix them. If we go with a _nospec() variant for the uaccess API names then we should use a similar naming for array_idx() and for the new barrier as well - no mixing. > You can always follow on with a patch to fix up the names and placements to your > liking. While they'll pick on my name choices, they won't pick on yours, because > I simply can't be bothered to care about a bikeshed color at this point after > being bounced around for 5 revisions of this patch set. Sorry, this kind of dismissive and condescending attitude won't cut it. Thanks, Ingo From mboxrd@z Thu Jan 1 00:00:00 1970 Sender: Ingo Molnar Date: Sun, 28 Jan 2018 19:33:09 +0100 From: Ingo Molnar Message-ID: <20180128183309.j7zkoyqblich4zhq@gmail.com> References: <151703971300.26578.1185595719337719486.stgit@dwillia2-desk3.amr.corp.intel.com> <151703972396.26578.7326612698912543866.stgit@dwillia2-desk3.amr.corp.intel.com> <20180128085500.djlm5rlbhjlpfj4i@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: [kernel-hardening] Re: [PATCH v5 02/12] array_idx: sanitize speculative array de-references To: Dan Williams Cc: Thomas Gleixner , linux-arch , Cyril Novikov , Kernel Hardening , Peter Zijlstra , Catalin Marinas , X86 ML , Will Deacon , Russell King , Ingo Molnar , Greg KH , "H. Peter Anvin" , Linus Torvalds , Alan Cox , Linux Kernel Mailing List List-ID: * Dan Williams wrote: > Thomas, Peter, and Alexei wanted s/nospec_barrier/ifence/ and I just checked past discussions, and I cannot find that part, got any links or message-IDs? PeterZ's feedback on Jan 8 was: > On Sun, Jan 07, 2018 at 06:24:11PM -0800, Alexei Starovoitov wrote: > > How about: > > CONFIG_SPECTRE1_WORKAROUND_INDEX_MASK > > CONFIG_SPECTRE1_WORKAROUND_LOAD_FENCE > > INSTRUCTION_FENCE if anything. LFENCE for Intel (and now also for AMD as > per 0592b0bce169) is a misnomer, IFENCE would be a better name for it. Which in that context clearly talked about the config space and how to name the instruction semantics in light of the confusion of LFENCE and MFENCE opcodes on Intel and AMD CPUs... Also, those early reviews were fundamentally low level feedback related to the actual functionality of the barriers and their mapping to the hardware. But the fact is, the current series introduces an inconsistent barrier namespace extension of: barrier() barrier_data() mb() rmb() wmb() store_mb() read_barrier_depends() ... + ifence() + array_idx() + array_idx_mask() This isn't bikeshed painting: _ALL_ existing barrier API names have 'barrier' or its abbreviation 'mb' (memory barrier) somewhere in their names, which makes it pretty easy to recognize them at a glance. I'm giving you high level API naming feedback because we are now growing the size of the barrier API. array_idx() on the other hand is pretty much close to a 'worst possible' name: - it's named in an overly generic, opaque fashion - doesn't indicate it at all that it's a barrier for something ... and since we want all kernel developers to use these facilities correctly, we want the names to be good and suggestive as well. I'd accept pretty much anything else that adds 'barrier' or 'nospec' to the name: array_idx_barrier() or array_idx_nospec(). (I'm slightly leaning towards 'nospec' because it's more indicative of what is being done, and it also is what we do for get uaccess APIs.) ifence() is a similar departure from existing barrier naming nomenclature, and I'd accept pretty much any other variant: barrier_nospec() ifence_nospec() The kernel namespace cleanliness rules are clear and consistent, and there's nothing new about them: - the name of the API should unambiguously refer back to the API category. For barriers this common reference is 'barrier' or 'mb'. - use postfixes or prefixes consistently: pick one and don't mix them. If we go with a _nospec() variant for the uaccess API names then we should use a similar naming for array_idx() and for the new barrier as well - no mixing. > You can always follow on with a patch to fix up the names and placements to your > liking. While they'll pick on my name choices, they won't pick on yours, because > I simply can't be bothered to care about a bikeshed color at this point after > being bounced around for 5 revisions of this patch set. Sorry, this kind of dismissive and condescending attitude won't cut it. Thanks, Ingo