Signed-off-by: Xin Li (Intel) <xin@zytor.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- new patch, taken from the RFC v2 MSR refactor series by Xin Li
V3:
- use instruction decoder (Peter Zijlstra)
---
 arch/x86/mm/extable.c | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index 2fdc1f1f5adb..22e14ff2d3e9 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -166,23 +166,48 @@ static bool ex_handler_uaccess(const struct exception_table_entry *fixup,
 static bool ex_handler_msr(const struct exception_table_entry *fixup,
 			   struct pt_regs *regs, bool wrmsr, bool safe, int reg)
 {
+	struct insn insn;
+	bool imm_insn;
+	u32 msr;
+
+	imm_insn = insn_decode_kernel(&insn, (void *)regs->ip) &&
+		   insn.vex_prefix.nbytes;
+	msr = imm_insn ? insn.immediate.value : (u32)regs->cx;
+
 	if (__ONCE_LITE_IF(!safe && wrmsr)) {
-		pr_warn("unchecked MSR access error: WRMSR to 0x%x (tried to write 0x%08x%08x) at rIP: 0x%lx (%pS)\n",
-			(unsigned int)regs->cx, (unsigned int)regs->dx,
-			(unsigned int)regs->ax,  regs->ip, (void *)regs->ip);
+		/*
+		 * To maintain consistency with existing RDMSR and WRMSR(NS) instructions,
+		 * the register operand for immediate form MSR instructions is ALWAYS
+		 * encoded as RAX in <asm/msr.h> for reading or writing the MSR value.
+		 */
+		u64 msr_val = regs->ax;
+
+		if (!imm_insn) {
+			/*
+			 * On processors that support the Intel 64 architecture, the
+			 * high-order 32 bits of each of RAX and RDX are ignored.
+			 */
+			msr_val &= 0xffffffff;
+			msr_val |= (u64)regs->dx << 32;
+		}
+
+		pr_warn("unchecked MSR access error: WRMSR to 0x%x (tried to write 0x%016llx) at rIP: 0x%lx (%pS)\n",
+			msr, msr_val, regs->ip, (void *)regs->ip);
 		show_stack_regs(regs);
 	}
 
 	if (__ONCE_LITE_IF(!safe && !wrmsr)) {
 		pr_warn("unchecked MSR access error: RDMSR from 0x%x at rIP: 0x%lx (%pS)\n",
-			(unsigned int)regs->cx, regs->ip, (void *)regs->ip);
+			msr, regs->ip, (void *)regs->ip);
 		show_stack_regs(regs);
 	}
 
 	if (!wrmsr) {
 		/* Pretend that the read succeeded and returned 0. */
 		regs->ax = 0;
-		regs->dx = 0;
+
+		if (!imm_insn)
+			regs->dx = 0;
 	}
 
 	if (safe)
-- 
2.53.0

Ideally please CC xen-devel on all patches if you've CC'd some, or at
least CC me on all patches that you partially send to xen-devel.  Having
only 4 patches of the series is very awkward to reply to.

> + /* + * To maintain consistency with existing RDMSR and WRMSR(NS)
> instructions, + * the register operand for immediate form MSR
> instructions is ALWAYS + * encoded as RAX in <asm/msr.h> for reading
> or writing the MSR value. + */ + u64 msr_val = regs->ax;

This is unsafe.  It assumes that the only source of MSR $IMM
instructions anywhere are the wrappers.

While this might be the wish of the developers, that doesn't make it true.


You've already decoded the instruction and got the ModRM byte, so either
check that it really is encoding %rax, or select the proper GPR based on
ModRM.

~Andrew

On 18.02.26 16:48, Andrew Cooper wrote:
> Ideally please CC xen-devel on all patches if you've CC'd some, or at
> least CC me on all patches that you partially send to xen-devel.  Having
> only 4 patches of the series is very awkward to reply to.
> 
>> + /* + * To maintain consistency with existing RDMSR and WRMSR(NS)
>> instructions, + * the register operand for immediate form MSR
>> instructions is ALWAYS + * encoded as RAX in <asm/msr.h> for reading
>> or writing the MSR value. + */ + u64 msr_val = regs->ax;
> 
> This is unsafe.  It assumes that the only source of MSR $IMM
> instructions anywhere are the wrappers.
> 
> While this might be the wish of the developers, that doesn't make it true.
> 
> 
> You've already decoded the instruction and got the ModRM byte, so either
> check that it really is encoding %rax, or select the proper GPR based on
> ModRM.

I'll use the proper GPR.

Thanks,


Juergen