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

[Jan Beulich <jbeulich@suse.com>]

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.

Jan

>> -----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 }