linux-kernel - Re: [PATCH 06/20] x86/msr: Standardize on 'u32' MSR indices in <asm/msr.h>

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <43203e47-7406-44cb-b783-3d71ff4d01ed@zytor.com>
Date: Thu, 10 Apr 2025 10:45:19 -0700
From: Xin Li <xin@...or.com>
To: Peter Zijlstra <peterz@...radead.org>, "H. Peter Anvin" <hpa@...or.com>
Cc: Ingo Molnar <mingo@...nel.org>, linux-kernel@...r.kernel.org,
        Juergen Gross <jgross@...e.com>, Dave Hansen <dave.hansen@...el.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Borislav Petkov <bp@...en8.de>, Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH 06/20] x86/msr: Standardize on 'u32' MSR indices in
 <asm/msr.h>

On 4/10/2025 12:48 AM, Peter Zijlstra wrote:
> I have vague memories of locations where we*have* to use native_{rd,wr}msr()
> because the paravirt indirections cause problems.
> 
> I don't clearly recall the reasons, but it could be instrumentation
> related. The paravirt muck has tracepoints and other crap included.

Hey, I was discussing this with hpa a few days ago ;)

Here is the existing code:

#define native_wrmsr(msr, low, high)                    \
         __wrmsr(msr, low, high)

...

static inline void notrace native_write_msr(unsigned int msr, u32 low, 
u32 high)
{
         __wrmsr(msr, low, high);

         if (tracepoint_enabled(write_msr))
                 do_trace_write_msr(msr, ((u64)high << 32 | low), 0);
}

So native_write_msr() is just native_wrmsr() + do_trace().

native_wrmsr() is used only once in arch/x86/coco/sev/core.c, and Boris
explained that in commit:

https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f980f9c31a923e9040dee0bc679a5f5b09e61f40.

     [ bp: Use the non-tracing rd/wrmsr variants because:
       vmlinux.o: warning: objtool: __sev_es_nmi_complete()+0x11f: \
               call to do_trace_write_msr() leaves .noinstr.text section
       as __sev_es_nmi_complete() is noinstr due to being called from the
       NMI handler exc_nmi(). ]

It's confusing from the function names when to use native_write_msr()
or native_wrmsr() after removing the paravirt MSR APIs.  And I want to 
rename native_wrmsr() to native_wrmsr_no_trace(), and then
native_write_msr() to native_wrmsr() to make their difference explicit.

The same to native_wrmsrl(), which is used in VMX noinstr functions.

There is only native_rdmsr(), but no native_rdmsrl().  And 
native_rdmsr() is used in microcode code only.

BTW, using the native_*() functions on Xen shouldn't be a problem,
because they cause #GP and then Xen could:

1) emulate the MSR instruction, and advance guest IP.

2) inject #GP back into the guest, and the guest MSR instruction is
skipped in guest #GP handler, with MSR access *_safe() APIs an error is
returned to the MSR APIs.

Thanks!
     Xin