From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753917AbcCNRIQ (ORCPT ); Mon, 14 Mar 2016 13:08:16 -0400 Received: from mail-oi0-f51.google.com ([209.85.218.51]:34761 "EHLO mail-oi0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752373AbcCNRIK (ORCPT ); Mon, 14 Mar 2016 13:08:10 -0400 MIME-Version: 1.0 In-Reply-To: <20160314115729.GC15800@pd.tnic> References: <7e72cd6ce08b946872a462fad13eca1810b8671d.1457805972.git.luto@kernel.org> <20160314115729.GC15800@pd.tnic> From: Andy Lutomirski Date: Mon, 14 Mar 2016 10:07:49 -0700 Message-ID: Subject: Re: [PATCH v4 1/5] x86/paravirt: Add _safe to the read_msr and write_msr PV hooks To: Borislav Petkov Cc: Andy Lutomirski , X86 ML , Paolo Bonzini , Peter Zijlstra , KVM list , Arjan van de Ven , xen-devel , "linux-kernel@vger.kernel.org" , Linus Torvalds , Andrew Morton Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Mar 14, 2016 at 4:57 AM, Borislav Petkov wrote: > On Sat, Mar 12, 2016 at 10:08:48AM -0800, Andy Lutomirski wrote: >> These hooks match the _safe variants, so name them accordingly. >> This will make room for unsafe PV hooks. >> >> Signed-off-by: Andy Lutomirski >> --- >> arch/x86/include/asm/paravirt.h | 33 +++++++++++++++++---------------- >> arch/x86/include/asm/paravirt_types.h | 8 ++++---- >> arch/x86/kernel/paravirt.c | 4 ++-- >> arch/x86/xen/enlighten.c | 4 ++-- >> 4 files changed, 25 insertions(+), 24 deletions(-) >> >> diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h >> index f6192502149e..2e49228ed9a3 100644 >> --- a/arch/x86/include/asm/paravirt.h >> +++ b/arch/x86/include/asm/paravirt.h >> @@ -129,34 +129,35 @@ static inline void wbinvd(void) >> >> #define get_kernel_rpl() (pv_info.kernel_rpl) >> >> -static inline u64 paravirt_read_msr(unsigned msr, int *err) >> +static inline u64 paravirt_read_msr_safe(unsigned msr, int *err) >> { >> - return PVOP_CALL2(u64, pv_cpu_ops.read_msr, msr, err); >> + return PVOP_CALL2(u64, pv_cpu_ops.read_msr_safe, msr, err); >> } >> >> -static inline int paravirt_write_msr(unsigned msr, unsigned low, unsigned high) >> +static inline int paravirt_write_msr_safe(unsigned msr, >> + unsigned low, unsigned high) >> { >> - return PVOP_CALL3(int, pv_cpu_ops.write_msr, msr, low, high); >> + return PVOP_CALL3(int, pv_cpu_ops.write_msr_safe, msr, low, high); >> } >> >> /* These should all do BUG_ON(_err), but our headers are too tangled. */ >> #define rdmsr(msr, val1, val2) \ >> do { \ >> int _err; \ >> - u64 _l = paravirt_read_msr(msr, &_err); \ >> + u64 _l = paravirt_read_msr_safe(msr, &_err); \ >> val1 = (u32)_l; \ >> val2 = _l >> 32; \ >> } while (0) >> >> #define wrmsr(msr, val1, val2) \ >> do { \ >> - paravirt_write_msr(msr, val1, val2); \ >> + paravirt_write_msr_safe(msr, val1, val2); \ >> } while (0) >> >> #define rdmsrl(msr, val) \ >> do { \ >> int _err; \ >> - val = paravirt_read_msr(msr, &_err); \ >> + val = paravirt_read_msr_safe(msr, &_err); \ >> } while (0) >> >> static inline void wrmsrl(unsigned msr, u64 val) >> @@ -164,23 +165,23 @@ static inline void wrmsrl(unsigned msr, u64 val) >> wrmsr(msr, (u32)val, (u32)(val>>32)); >> } >> >> -#define wrmsr_safe(msr, a, b) paravirt_write_msr(msr, a, b) >> +#define wrmsr_safe(msr, a, b) paravirt_write_msr_safe(msr, a, b) >> >> /* rdmsr with exception handling */ >> -#define rdmsr_safe(msr, a, b) \ >> -({ \ >> - int _err; \ >> - u64 _l = paravirt_read_msr(msr, &_err); \ >> - (*a) = (u32)_l; \ >> - (*b) = _l >> 32; \ >> - _err; \ >> +#define rdmsr_safe(msr, a, b) \ >> +({ \ >> + int _err; \ >> + u64 _l = paravirt_read_msr_safe(msr, &_err); \ >> + (*a) = (u32)_l; \ >> + (*b) = _l >> 32; \ >> + _err; \ >> }) >> >> static inline int rdmsrl_safe(unsigned msr, unsigned long long *p) >> { >> int err; >> >> - *p = paravirt_read_msr(msr, &err); >> + *p = paravirt_read_msr_safe(msr, &err); >> return err; >> } >> >> diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h >> index 77db5616a473..5a06cccd36f0 100644 >> --- a/arch/x86/include/asm/paravirt_types.h >> +++ b/arch/x86/include/asm/paravirt_types.h >> @@ -155,10 +155,10 @@ struct pv_cpu_ops { >> void (*cpuid)(unsigned int *eax, unsigned int *ebx, >> unsigned int *ecx, unsigned int *edx); >> >> - /* MSR, PMC and TSR operations. >> - err = 0/-EFAULT. wrmsr returns 0/-EFAULT. */ >> - u64 (*read_msr)(unsigned int msr, int *err); >> - int (*write_msr)(unsigned int msr, unsigned low, unsigned high); >> + /* MSR operations. >> + err = 0/-EIO. wrmsr returns 0/-EIO. */ > > Please fix the comment format while you're touching this : > > /* > * A sentence. > * Another sentence. > */ > > Other than that: > > Acked-by: Borislav Petkov I fixed this in "x86/paravirt: Add paravirt_{read,write}_msr" later in the series. Is that good enough? --Andy