public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] x86: allow 32-bit reg to be used with U{RD,WR}MSR
@ 2023-11-24  9:18 Jan Beulich
  2023-12-04  1:45 ` Hu, Lin1
  0 siblings, 1 reply; 11+ messages in thread
From: Jan Beulich @ 2023-11-24  9:18 UTC (permalink / raw)
  To: Binutils; +Cc: H.J. Lu, Hu, Lin1

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] x86: allow 32-bit reg to be used with U{RD,WR}MSR
  2023-11-24  9:18 [PATCH] x86: allow 32-bit reg to be used with U{RD,WR}MSR Jan Beulich
@ 2023-12-04  1:45 ` Hu, Lin1
  2023-12-04  7:17   ` Jan Beulich
  0 siblings, 1 reply; 11+ messages in thread
From: Hu, Lin1 @ 2023-12-04  1:45 UTC (permalink / raw)
  To: Beulich, Jan, Binutils; +Cc: H.J. Lu

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.

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 }

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] x86: allow 32-bit reg to be used with U{RD,WR}MSR
  2023-12-04  1:45 ` Hu, Lin1
@ 2023-12-04  7:17   ` Jan Beulich
  2023-12-12  7:13     ` Hu, Lin1
  0 siblings, 1 reply; 11+ messages in thread
From: Jan Beulich @ 2023-12-04  7:17 UTC (permalink / raw)
  To: Hu, Lin1; +Cc: H.J. Lu, Binutils

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 }


^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] x86: allow 32-bit reg to be used with U{RD,WR}MSR
  2023-12-04  7:17   ` Jan Beulich
@ 2023-12-12  7:13     ` Hu, Lin1
  2023-12-12  8:13       ` Jan Beulich
  0 siblings, 1 reply; 11+ messages in thread
From: Hu, Lin1 @ 2023-12-12  7:13 UTC (permalink / raw)
  To: Beulich, Jan; +Cc: H.J. Lu, Binutils

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] x86: allow 32-bit reg to be used with U{RD,WR}MSR
  2023-12-12  7:13     ` Hu, Lin1
@ 2023-12-12  8:13       ` Jan Beulich
  2023-12-12  8:51         ` Hu, Lin1
  0 siblings, 1 reply; 11+ messages in thread
From: Jan Beulich @ 2023-12-12  8:13 UTC (permalink / raw)
  To: Hu, Lin1; +Cc: H.J. Lu, Binutils

On 12.12.2023 08:13, Hu, Lin1 wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Monday, December 4, 2023 3:18 PM
>>
>> 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.

So this last part is what I don't understand: MSR space is 32 bits. Why does
"r64 make sense at the moment"?

Jan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] x86: allow 32-bit reg to be used with U{RD,WR}MSR
  2023-12-12  8:13       ` Jan Beulich
@ 2023-12-12  8:51         ` Hu, Lin1
  2023-12-12  9:20           ` Jan Beulich
  0 siblings, 1 reply; 11+ messages in thread
From: Hu, Lin1 @ 2023-12-12  8:51 UTC (permalink / raw)
  To: Beulich, Jan; +Cc: H.J. Lu, Binutils

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Tuesday, December 12, 2023 4:13 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 12.12.2023 08:13, Hu, Lin1 wrote:
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@suse.com>
> >> Sent: Monday, December 4, 2023 3:18 PM
> >>
> >> 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.
> 
> So this last part is what I don't understand: MSR space is 32 bits. Why does
> "r64 make sense at the moment"?
> 

Without ignoring the high 32 bits, I think r64 and r32 are equivalent. Because the GP fault is "If MSR_address[63:0] &  0xFFFF_FFFF_FFFF_C000", other than "If MSR_address[63:0] & 0xFFFF_FFFF_0000_0000". When they use "urdmsr" or "uwrmsr", Users need to know the MSR name. R32 also can't tell them which name is right or wrong.

BRs,
Lin

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] x86: allow 32-bit reg to be used with U{RD,WR}MSR
  2023-12-12  8:51         ` Hu, Lin1
@ 2023-12-12  9:20           ` Jan Beulich
  2023-12-13  2:23             ` Hu, Lin1
  0 siblings, 1 reply; 11+ messages in thread
From: Jan Beulich @ 2023-12-12  9:20 UTC (permalink / raw)
  To: Hu, Lin1; +Cc: H.J. Lu, Binutils

On 12.12.2023 09:51, Hu, Lin1 wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Tuesday, December 12, 2023 4:13 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 12.12.2023 08:13, Hu, Lin1 wrote:
>>>> -----Original Message-----
>>>> From: Jan Beulich <jbeulich@suse.com>
>>>> Sent: Monday, December 4, 2023 3:18 PM
>>>>
>>>> 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.
>>
>> So this last part is what I don't understand: MSR space is 32 bits. Why does
>> "r64 make sense at the moment"?
>>
> 
> Without ignoring the high 32 bits, I think r64 and r32 are equivalent. Because the GP fault is "If MSR_address[63:0] &  0xFFFF_FFFF_FFFF_C000", other than "If MSR_address[63:0] & 0xFFFF_FFFF_0000_0000". When they use "urdmsr" or "uwrmsr", Users need to know the MSR name. R32 also can't tell them which name is right or wrong.

Feels like you're not getting my point. Imo operand size should reflect
what part of a register is actually used. If the check was as presently
documented, only the low 32 bits of the operand could ever be used (as
MSR space isn't wider), irrespective of the initial form of the insn
limiting things even further (to the low 14 bits). With the wider mask,
you're opening a path to a future where MSR space is 64 bits. That may
indeed be intended, yet I have severe doubts that going beyond 32 bits
is ever going to be necessary. You'd at least need to come close to
the order or a billion MSRs to warrant widening space, I would say.

Furthermore while in the vast majority of cases I expect it won't make
a difference to users simply because whatever operations are needed to
establish the intended MSR index can be carried out with 32-bit
operations (thus zeroing the upper half anyway), someone using 64-bit
operations for such calculations (for whatever reason, perhaps even by
mistake) would then be running into an issue in case such an operation
ended up overflowing into the high 32 bits. Personally I see no reason
to create such a pitfall, even if it is just a minor one.

Jan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] x86: allow 32-bit reg to be used with U{RD,WR}MSR
  2023-12-12  9:20           ` Jan Beulich
@ 2023-12-13  2:23             ` Hu, Lin1
  2023-12-13  6:59               ` Jan Beulich
  0 siblings, 1 reply; 11+ messages in thread
From: Hu, Lin1 @ 2023-12-13  2:23 UTC (permalink / raw)
  To: Beulich, Jan; +Cc: H.J. Lu, Binutils

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Tuesday, December 12, 2023 5:21 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 12.12.2023 09:51, Hu, Lin1 wrote:
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@suse.com>
> >> Sent: Tuesday, December 12, 2023 4:13 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 12.12.2023 08:13, Hu, Lin1 wrote:
> >>>> -----Original Message-----
> >>>> From: Jan Beulich <jbeulich@suse.com>
> >>>> Sent: Monday, December 4, 2023 3:18 PM
> >>>>
> >>>> 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.
> >>
> >> So this last part is what I don't understand: MSR space is 32 bits.
> >> Why does
> >> "r64 make sense at the moment"?
> >>
> >
> > Without ignoring the high 32 bits, I think r64 and r32 are equivalent. Because
> the GP fault is "If MSR_address[63:0] &  0xFFFF_FFFF_FFFF_C000", other than "If
> MSR_address[63:0] & 0xFFFF_FFFF_0000_0000". When they use "urdmsr" or
> "uwrmsr", Users need to know the MSR name. R32 also can't tell them which
> name is right or wrong.
> 
> Feels like you're not getting my point. Imo operand size should reflect what part
> of a register is actually used. If the check was as presently documented, only the
> low 32 bits of the operand could ever be used (as MSR space isn't wider),
> irrespective of the initial form of the insn limiting things even further (to the low
> 14 bits). With the wider mask, you're opening a path to a future where MSR
> space is 64 bits. That may indeed be intended, yet I have severe doubts that
> going beyond 32 bits is ever going to be necessary. You'd at least need to come
> close to the order or a billion MSRs to warrant widening space, I would say.
> 
> Furthermore while in the vast majority of cases I expect it won't make a
> difference to users simply because whatever operations are needed to establish
> the intended MSR index can be carried out with 32-bit operations (thus zeroing
> the upper half anyway), someone using 64-bit operations for such calculations
> (for whatever reason, perhaps even by
> mistake) would then be running into an issue in case such an operation ended up
> overflowing into the high 32 bits. Personally I see no reason to create such a
> pitfall, even if it is just a minor one.

In my opinion, MSR's name is a key. The user doesn't need to add or subtract to it, it's the OS's job to calculate the address. About R64 is telling the user that the MSR space may be wider. I don't think it is a problem. Because, this has no impact on user usage or coding. rdmsr/wrmsr only uses 32-bit registers because they only had 32-bit registers at the time, and the designers stated that if they were designing this instruction now it would use 64-bit registers.

Actually, we're talking about USER_MSR. USER_MSR's actual address is calculated by a_function(base_address, MSR_address[13:0]). Overflow is not a problem, the user just needs to make sure that what is in the lower 14 bits is valid. But I still can't understand why a user would take a key and calculate it.

BRs,
Lin

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] x86: allow 32-bit reg to be used with U{RD,WR}MSR
  2023-12-13  2:23             ` Hu, Lin1
@ 2023-12-13  6:59               ` Jan Beulich
  2023-12-13  8:40                 ` Hu, Lin1
  0 siblings, 1 reply; 11+ messages in thread
From: Jan Beulich @ 2023-12-13  6:59 UTC (permalink / raw)
  To: Hu, Lin1; +Cc: H.J. Lu, Binutils

On 13.12.2023 03:23, Hu, Lin1 wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Tuesday, December 12, 2023 5:21 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 12.12.2023 09:51, Hu, Lin1 wrote:
>>>> -----Original Message-----
>>>> From: Jan Beulich <jbeulich@suse.com>
>>>> Sent: Tuesday, December 12, 2023 4:13 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 12.12.2023 08:13, Hu, Lin1 wrote:
>>>>>> -----Original Message-----
>>>>>> From: Jan Beulich <jbeulich@suse.com>
>>>>>> Sent: Monday, December 4, 2023 3:18 PM
>>>>>>
>>>>>> 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.
>>>>
>>>> So this last part is what I don't understand: MSR space is 32 bits.
>>>> Why does
>>>> "r64 make sense at the moment"?
>>>>
>>>
>>> Without ignoring the high 32 bits, I think r64 and r32 are equivalent. Because
>> the GP fault is "If MSR_address[63:0] &  0xFFFF_FFFF_FFFF_C000", other than "If
>> MSR_address[63:0] & 0xFFFF_FFFF_0000_0000". When they use "urdmsr" or
>> "uwrmsr", Users need to know the MSR name. R32 also can't tell them which
>> name is right or wrong.
>>
>> Feels like you're not getting my point. Imo operand size should reflect what part
>> of a register is actually used. If the check was as presently documented, only the
>> low 32 bits of the operand could ever be used (as MSR space isn't wider),
>> irrespective of the initial form of the insn limiting things even further (to the low
>> 14 bits). With the wider mask, you're opening a path to a future where MSR
>> space is 64 bits. That may indeed be intended, yet I have severe doubts that
>> going beyond 32 bits is ever going to be necessary. You'd at least need to come
>> close to the order or a billion MSRs to warrant widening space, I would say.
>>
>> Furthermore while in the vast majority of cases I expect it won't make a
>> difference to users simply because whatever operations are needed to establish
>> the intended MSR index can be carried out with 32-bit operations (thus zeroing
>> the upper half anyway), someone using 64-bit operations for such calculations
>> (for whatever reason, perhaps even by
>> mistake) would then be running into an issue in case such an operation ended up
>> overflowing into the high 32 bits. Personally I see no reason to create such a
>> pitfall, even if it is just a minor one.
> 
> In my opinion, MSR's name is a key. The user doesn't need to add or subtract to it, it's the OS's job to calculate the address. About R64 is telling the user that the MSR space may be wider. I don't think it is a problem. Because, this has no impact on user usage or coding. rdmsr/wrmsr only uses 32-bit registers because they only had 32-bit registers at the time, and the designers stated that if they were designing this instruction now it would use 64-bit registers.
> 
> Actually, we're talking about USER_MSR. USER_MSR's actual address is calculated by a_function(base_address, MSR_address[13:0]). Overflow is not a problem, the user just needs to make sure that what is in the lower 14 bits is valid. But I still can't understand why a user would take a key and calculate it.

Even now there are already ranges of similar MSRs (they aren't user mode
accessible, but such ranges could show up). Calculating the actual MSR
address then may indeed involve arithmetic, and how exactly one chooses
to carry out the arithmetic would imo better be left entirely to the
programmer, without the architecture dictating anything more than the
bare minimum of constraints.

Anyway, I see this doesn't lead anywhere. I'll revert said patch, perhaps
not today but later this week.

Jan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] x86: allow 32-bit reg to be used with U{RD,WR}MSR
  2023-12-13  6:59               ` Jan Beulich
@ 2023-12-13  8:40                 ` Hu, Lin1
  2023-12-13  8:50                   ` Jan Beulich
  0 siblings, 1 reply; 11+ messages in thread
From: Hu, Lin1 @ 2023-12-13  8:40 UTC (permalink / raw)
  To: Beulich, Jan; +Cc: H.J. Lu, Binutils

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Wednesday, December 13, 2023 2:59 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 13.12.2023 03:23, Hu, Lin1 wrote:
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@suse.com>
> >> Sent: Tuesday, December 12, 2023 5:21 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 12.12.2023 09:51, Hu, Lin1 wrote:
> >>>> -----Original Message-----
> >>>> From: Jan Beulich <jbeulich@suse.com>
> >>>> Sent: Tuesday, December 12, 2023 4:13 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 12.12.2023 08:13, Hu, Lin1 wrote:
> >>>>>> -----Original Message-----
> >>>>>> From: Jan Beulich <jbeulich@suse.com>
> >>>>>> Sent: Monday, December 4, 2023 3:18 PM
> >>>>>>
> >>>>>> 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.
> >>>>
> >>>> So this last part is what I don't understand: MSR space is 32 bits.
> >>>> Why does
> >>>> "r64 make sense at the moment"?
> >>>>
> >>>
> >>> Without ignoring the high 32 bits, I think r64 and r32 are
> >>> equivalent. Because
> >> the GP fault is "If MSR_address[63:0] &  0xFFFF_FFFF_FFFF_C000",
> >> other than "If MSR_address[63:0] & 0xFFFF_FFFF_0000_0000". When they
> >> use "urdmsr" or "uwrmsr", Users need to know the MSR name. R32 also
> >> can't tell them which name is right or wrong.
> >>
> >> Feels like you're not getting my point. Imo operand size should
> >> reflect what part of a register is actually used. If the check was as
> >> presently documented, only the low 32 bits of the operand could ever
> >> be used (as MSR space isn't wider), irrespective of the initial form
> >> of the insn limiting things even further (to the low
> >> 14 bits). With the wider mask, you're opening a path to a future
> >> where MSR space is 64 bits. That may indeed be intended, yet I have
> >> severe doubts that going beyond 32 bits is ever going to be
> >> necessary. You'd at least need to come close to the order or a billion MSRs to
> warrant widening space, I would say.
> >>
> >> Furthermore while in the vast majority of cases I expect it won't
> >> make a difference to users simply because whatever operations are
> >> needed to establish the intended MSR index can be carried out with
> >> 32-bit operations (thus zeroing the upper half anyway), someone using
> >> 64-bit operations for such calculations (for whatever reason, perhaps
> >> even by
> >> mistake) would then be running into an issue in case such an
> >> operation ended up overflowing into the high 32 bits. Personally I
> >> see no reason to create such a pitfall, even if it is just a minor one.
> >
> > In my opinion, MSR's name is a key. The user doesn't need to add or subtract
> to it, it's the OS's job to calculate the address. About R64 is telling the user that
> the MSR space may be wider. I don't think it is a problem. Because, this has no
> impact on user usage or coding. rdmsr/wrmsr only uses 32-bit registers because
> they only had 32-bit registers at the time, and the designers stated that if they
> were designing this instruction now it would use 64-bit registers.
> >
> > Actually, we're talking about USER_MSR. USER_MSR's actual address is
> calculated by a_function(base_address, MSR_address[13:0]). Overflow is not a
> problem, the user just needs to make sure that what is in the lower 14 bits is
> valid. But I still can't understand why a user would take a key and calculate it.
> 
> Even now there are already ranges of similar MSRs (they aren't user mode
> accessible, but such ranges could show up). Calculating the actual MSR address
> then may indeed involve arithmetic, and how exactly one chooses to carry out
> the arithmetic would imo better be left entirely to the programmer, without the
> architecture dictating anything more than the bare minimum of constraints.
> 
> Anyway, I see this doesn't lead anywhere. I'll revert said patch, perhaps not
> today but later this week.
> 

OK. Thanks.

But, I still have a question, I understand that calculating the actual MSR address then involve arithmetic, but I don't understand when you mention programmer, which field do they work and how do they use the MSR instructions, could you give me an example?

BRs,
Lin

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] x86: allow 32-bit reg to be used with U{RD,WR}MSR
  2023-12-13  8:40                 ` Hu, Lin1
@ 2023-12-13  8:50                   ` Jan Beulich
  0 siblings, 0 replies; 11+ messages in thread
From: Jan Beulich @ 2023-12-13  8:50 UTC (permalink / raw)
  To: Hu, Lin1; +Cc: H.J. Lu, Binutils

On 13.12.2023 09:40, Hu, Lin1 wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Wednesday, December 13, 2023 2:59 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 13.12.2023 03:23, Hu, Lin1 wrote:
>>>> -----Original Message-----
>>>> From: Jan Beulich <jbeulich@suse.com>
>>>> Sent: Tuesday, December 12, 2023 5:21 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 12.12.2023 09:51, Hu, Lin1 wrote:
>>>>>> -----Original Message-----
>>>>>> From: Jan Beulich <jbeulich@suse.com>
>>>>>> Sent: Tuesday, December 12, 2023 4:13 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 12.12.2023 08:13, Hu, Lin1 wrote:
>>>>>>>> -----Original Message-----
>>>>>>>> From: Jan Beulich <jbeulich@suse.com>
>>>>>>>> Sent: Monday, December 4, 2023 3:18 PM
>>>>>>>>
>>>>>>>> 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.
>>>>>>
>>>>>> So this last part is what I don't understand: MSR space is 32 bits.
>>>>>> Why does
>>>>>> "r64 make sense at the moment"?
>>>>>>
>>>>>
>>>>> Without ignoring the high 32 bits, I think r64 and r32 are
>>>>> equivalent. Because
>>>> the GP fault is "If MSR_address[63:0] &  0xFFFF_FFFF_FFFF_C000",
>>>> other than "If MSR_address[63:0] & 0xFFFF_FFFF_0000_0000". When they
>>>> use "urdmsr" or "uwrmsr", Users need to know the MSR name. R32 also
>>>> can't tell them which name is right or wrong.
>>>>
>>>> Feels like you're not getting my point. Imo operand size should
>>>> reflect what part of a register is actually used. If the check was as
>>>> presently documented, only the low 32 bits of the operand could ever
>>>> be used (as MSR space isn't wider), irrespective of the initial form
>>>> of the insn limiting things even further (to the low
>>>> 14 bits). With the wider mask, you're opening a path to a future
>>>> where MSR space is 64 bits. That may indeed be intended, yet I have
>>>> severe doubts that going beyond 32 bits is ever going to be
>>>> necessary. You'd at least need to come close to the order or a billion MSRs to
>> warrant widening space, I would say.
>>>>
>>>> Furthermore while in the vast majority of cases I expect it won't
>>>> make a difference to users simply because whatever operations are
>>>> needed to establish the intended MSR index can be carried out with
>>>> 32-bit operations (thus zeroing the upper half anyway), someone using
>>>> 64-bit operations for such calculations (for whatever reason, perhaps
>>>> even by
>>>> mistake) would then be running into an issue in case such an
>>>> operation ended up overflowing into the high 32 bits. Personally I
>>>> see no reason to create such a pitfall, even if it is just a minor one.
>>>
>>> In my opinion, MSR's name is a key. The user doesn't need to add or subtract
>> to it, it's the OS's job to calculate the address. About R64 is telling the user that
>> the MSR space may be wider. I don't think it is a problem. Because, this has no
>> impact on user usage or coding. rdmsr/wrmsr only uses 32-bit registers because
>> they only had 32-bit registers at the time, and the designers stated that if they
>> were designing this instruction now it would use 64-bit registers.
>>>
>>> Actually, we're talking about USER_MSR. USER_MSR's actual address is
>> calculated by a_function(base_address, MSR_address[13:0]). Overflow is not a
>> problem, the user just needs to make sure that what is in the lower 14 bits is
>> valid. But I still can't understand why a user would take a key and calculate it.
>>
>> Even now there are already ranges of similar MSRs (they aren't user mode
>> accessible, but such ranges could show up). Calculating the actual MSR address
>> then may indeed involve arithmetic, and how exactly one chooses to carry out
>> the arithmetic would imo better be left entirely to the programmer, without the
>> architecture dictating anything more than the bare minimum of constraints.
>>
>> Anyway, I see this doesn't lead anywhere. I'll revert said patch, perhaps not
>> today but later this week.
> 
> OK. Thanks.
> 
> But, I still have a question, I understand that calculating the actual MSR address then involve arithmetic, but I don't understand when you mention programmer, which field do they work and how do they use the MSR instructions, could you give me an example?

I don't have a concrete example, and I'm not sure contriving one would help.
But consider a calculation involving a negative offset, done in 64 bits, but
with the (negative) rhs value passed in which was truncated to 32 bits.
That'll overflow (or underflow, depending on the direction you look at it)
in 64 bits (thus yielding an invalid MSR when taking into account all 64
bits), but would be quite fine when only the low 32 bits were consumed.

My earlier point simply was: You never know how people are going to use any
of what you provide. Restrict them as little as possible.

Jan

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-12-13  8:50 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-24  9:18 [PATCH] x86: allow 32-bit reg to be used with U{RD,WR}MSR Jan Beulich
2023-12-04  1:45 ` Hu, Lin1
2023-12-04  7:17   ` Jan Beulich
2023-12-12  7:13     ` Hu, Lin1
2023-12-12  8:13       ` Jan Beulich
2023-12-12  8:51         ` Hu, Lin1
2023-12-12  9:20           ` Jan Beulich
2023-12-13  2:23             ` Hu, Lin1
2023-12-13  6:59               ` Jan Beulich
2023-12-13  8:40                 ` Hu, Lin1
2023-12-13  8:50                   ` Jan Beulich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).