From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1758852AbbJ3TXo (ORCPT <rfc822;w@1wt.eu>);
	Fri, 30 Oct 2015 15:23:44 -0400
Received: from mail.skyhub.de ([78.46.96.112]:55592 "EHLO mail.skyhub.de"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751024AbbJ3TXn (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Fri, 30 Oct 2015 15:23:43 -0400
Date: Fri, 30 Oct 2015 20:23:29 +0100
From: Borislav Petkov <bp@alien8.de>
To: Andy Lutomirski <luto@amacapital.net>
Cc: LKML <linux-kernel@vger.kernel.org>, Andy Lutomirski <luto@kernel.org>,
        "H. Peter Anvin" <hpa@zytor.com>, Ingo Molnar <mingo@kernel.org>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [RFC PATCH] x86/cpu: Fix MSR value truncation issue
Message-ID: <20151030192329.GJ20952@pd.tnic>
References: <1446226105-13384-1-git-send-email-bp@alien8.de>
 <CALCETrXpjxHFszxDYdT=iZo8O-x7eNy_FBePSkqTjVzZfVT-fg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <CALCETrXpjxHFszxDYdT=iZo8O-x7eNy_FBePSkqTjVzZfVT-fg@mail.gmail.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, Oct 30, 2015 at 11:59:39AM -0700, Andy Lutomirski wrote:
> Is this with or without:
> 
> commit 47edb65178cb7056c2eea0b6c41a7d8c84547192
> Author: Andy Lutomirski <luto@kernel.org>
> Date:   Thu Jul 23 12:14:40 2015 -0700
> 
>     x86/asm/msr: Make wrmsrl() a function

with.

I'm playing ontop of 4.3-rc7.

> If that patch is applied, then I think that gcc is just being dumb and

Huh, why?

gcc is actually being smart by completely avoiding the shift to the
upper bits and puts those directly into %edx and sparse simply says that
we're truncating some bits.

I actually think that sparse is correct in saying that some bits are
going to be zeroed out even though we want that here.

> that we should consider tweaking wrmsrl to avoid generating the
> warning.  Maybe change (u32)val to (u32)(val & 0xffffffffull)?

That works.

> I don't see why we should uglify the caller when the problem is some
> combination of gcc and the wrmsrl implementation.

Why uglify?

We're basically making explicit that we write the high 32-bits with the
SYSCALL and SYSRET CS and SS and we set the low 32-bit explicitly to 0:

	wrmsr(MSR_STAR, 0, (__USER32_CS << 16) | __KERNEL_CS);

versus setting the low 32-bits to zero *implicitly*:

	wrmsrl(MSR_STAR,  ((u64)__USER32_CS)<<48  | ((u64)__KERNEL_CS)<<32);

due to that shifting to the left filling up the 32-bit with zeroes.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.