Re: [PATCH v4 08/18] x86/extable: Add support for immediate form MSR instructions

[H. Peter Anvin]

On 2026-06-28 23:55, Juergen Gross wrote:
> Signed-off-by: Xin Li (Intel) <xin@xxxxxxxxx>
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> ---
> V2:
> - new patch, taken from the RFC v2 MSR refactor series by Xin Li
> V3:
> - use instruction decoder (Peter Zijlstra)
> V4:
> - don't assume %rax for immediate form (Andrew Cooper)
> ---
>  arch/x86/mm/extable.c | 46 +++++++++++++++++++++++++++++++++++++------
>  1 file changed, 40 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
> index ceb8d03191ab..9974785c8968 100644
> --- a/arch/x86/mm/extable.c
> +++ b/arch/x86/mm/extable.c
> @@ -166,25 +166,59 @@ 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)
>  {
> +	unsigned long *regptr;
> +	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;
> +	regptr = imm_insn ? insn_get_modrm_reg_ptr(&insn, regs) : &regs->ax;
> +	if (unlikely(!regptr)) {
> +		pr_err("Inconsistent %sMSR access instruction data at rIP: 0x%lx (%pS)!\n",
> +		       wrmsr ? "WR" : "RD", regs->ip, (void *)regs->ip);
> +		show_stack_regs(regs);
> +		goto out;
> +	}
> +
>  	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 = *regptr;

This comment is inconsistent with the use of a *regptr. The code is valid
without this assumption, since you do insn_get_modrm_reg_ptr(), so I think you
can simply omit the comment.

	-hpa