RE: [PATCH] x86: allow 32-bit reg to be used with U{RD,WR}MSR

["Hu, Lin1" <lin1.hu@intel.com>]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Monday, December 4, 2023 3:18 PM
> To: Hu, Lin1 <lin1.hu@intel.com>
> Cc: H.J. Lu <hjl.tools@gmail.com>; Binutils <binutils@sourceware.org>
> Subject: Re: [PATCH] x86: allow 32-bit reg to be used with U{RD,WR}MSR
> 
> On 04.12.2023 02:45, Hu, Lin1 wrote:
> > I talked to the people involved. They found the eds description about
> > "ignoring h32 bits of the MSR address for register variants" is a documentation
> bug. The right version is USER_MSR have a GP fault on h32 of the MSR address
> for the register version. So they update the description from #GP(0) If
> MSR_address[63:0] & 0x0000_0000_FFFF_C000 != 0 to #GP(0) If
> MSR_address[63:0] & 0xFFFF_FFFF_FFFF_C000 != 0. And the source register will
> still be r64 in the spec.
> 
> What an unhelpful behavior. I'll need to revert the patch below then, assuming
> they're not willing to re-think.
>

I think they don't what to change it. Because, if they change the spec, it would involve a lot of places, and  r64 make sense at the moment.
 
BRs,
Lin

> 
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@suse.com>
> >> Sent: Friday, November 24, 2023 5:18 PM
> >> To: Binutils <binutils@sourceware.org>
> >> Cc: H.J. Lu <hjl.tools@gmail.com>; Hu, Lin1 <lin1.hu@intel.com>
> >> Subject: [PATCH] x86: allow 32-bit reg to be used with U{RD,WR}MSR
> >>
> >> ... as MSR index specifier: It is unreasonable to demand that people
> >> write less readable / understandable code, just because the present
> >> documentation mentions only Reg64. Whether to also adjust the
> >> disassembler is a separate question, perhaps indeed more tightly tied
> >> to what the spec says.
> >>
> >> --- a/gas/testsuite/gas/i386/x86-64-user_msr.s
> >> +++ b/gas/testsuite/gas/i386/x86-64-user_msr.s
> >> @@ -5,7 +5,7 @@ _start:
> >>  	urdmsr	%r14, %r12
> >>  	urdmsr	%r14, %rax
> >>  	urdmsr	%rdx, %r12
> >> -	urdmsr	%rdx, %rax
> >> +	urdmsr	%edx, %rax
> >>  	urdmsr	$51515151, %r12
> >>  	urdmsr	$51515151, %rax
> >>  	urdmsr	$0x7f, %r12
> >> @@ -14,7 +14,7 @@ _start:
> >>  	uwrmsr	%r12, %r14
> >>  	uwrmsr	%rax, %r14
> >>  	uwrmsr	%r12, %rdx
> >> -	uwrmsr	%rax, %rdx
> >> +	uwrmsr	%rax, %edx
> >>  	uwrmsr	%r12, $51515151
> >>  	uwrmsr	%rax, $51515151
> >>  	uwrmsr	%r12, $0x7f
> >> @@ -24,7 +24,7 @@ _start:
> >>  	.intel_syntax noprefix
> >>  	urdmsr	r12, r14
> >>  	urdmsr	rax, r14
> >> -	urdmsr	r12, rdx
> >> +	urdmsr	r12, edx
> >>  	urdmsr	rax, rdx
> >>  	urdmsr	r12, 51515151
> >>  	urdmsr	rax, 51515151
> >> @@ -33,7 +33,7 @@ _start:
> >>  	urdmsr	r12, 0x80000000
> >>  	uwrmsr	r14, r12
> >>  	uwrmsr	r14, rax
> >> -	uwrmsr	rdx, r12
> >> +	uwrmsr	edx, r12
> >>  	uwrmsr	rdx, rax
> >>  	uwrmsr	51515151, r12
> >>  	uwrmsr	51515151, rax
> >> --- a/opcodes/i386-opc.tbl
> >> +++ b/opcodes/i386-opc.tbl
> >> @@ -3359,9 +3359,9 @@ eretu, 0xf30f01ca, FRED, NoSuf, {}
> >>
> >>  // USER_MSR instructions.
> >>
> >> -urdmsr, 0xf20f38f8, USER_MSR, RegMem|NoSuf|NoRex64, { Reg64, Reg64 }
> >> +urdmsr, 0xf20f38f8, USER_MSR, RegMem|NoSuf|NoRex64, { Reg32|Reg64,
> >> +Reg64 }
> >>  urdmsr, 0xf2f8/0, USER_MSR, Modrm|Vex128|VexMap7|VexW0|NoSuf, {
> >> Imm32, Reg64 } -uwrmsr, 0xf30f38f8, USER_MSR, Modrm|NoSuf|NoRex64, {
> >> Reg64, Reg64 }
> >> +uwrmsr, 0xf30f38f8, USER_MSR, Modrm|NoSuf|NoRex64, { Reg64,
> >> Reg32|Reg64
> >> +}
> >>  // Immediates want to be first; md_assemble() takes care of swapping
> >> operands  // accordingly.
> >>  uwrmsr, 0xf3f8/0, USER_MSR, Modrm|Vex128|VexMap7|VexW0|NoSuf, {
> >> Imm32, Reg64 }