[PATCH 2/2] Support Intel MSR_IMM

[Jan Beulich]

On 06.11.2024 10:09, Hu, Lin1 wrote:
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/msr_imm-inval.l
> @@ -0,0 +1,3 @@
> +.* Assembler messages:
> +.*:5: Error: bad register name `%r12'
> +.*:6: Error: bad register name `%r12'
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/msr_imm-inval.s
> @@ -0,0 +1,6 @@
> +# Check Illegal 32bit MSR_IMM instructions
> +
> +	.text
> +_start:
> +	rdmsr  $51515151, %r12
> +	wrmsrns  %r12, $51515151

If this test is about rejecting the insns outside of 64-bit mode, then you
shouldn't use registers here which are invalid outside of 64-bit mode. The
diagnostics in the .l file should have been telling ...

> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-msr_imm-inval.s
> @@ -0,0 +1,6 @@
> +# Check Illegal 64bit MSR_IMM instructions
> +
> +	.text
> +_start:
> +	rdmsr  $5151515151515151, %r12
> +	wrmsrns  %r12, $5151515151515151

Please also test with negative immediates.

> @@ -8691,6 +8723,8 @@ static const struct dis386 bad_opcode = { "(bad)", { XX }, 0 };
>  /* Fetch error indicator.  */
>  static const struct dis386 err_opcode = { NULL, { XX }, 0 };
>  
> +static const struct dis386 map7_f6_opcode = { VEX_LEN_TABLE (VEX_LEN_MAP7_F6) };
> +
>  static const struct dis386 map7_f8_opcode = { VEX_LEN_TABLE (VEX_LEN_MAP7_F8) };

Minor: I think omitting the new blank line would overall be better.

> --- a/opcodes/i386-gen.c
> +++ b/opcodes/i386-gen.c
> @@ -281,6 +281,8 @@ static const dependency isa_dependencies[] =
>      "64" },
>    { "APX_F",
>      "XSAVE|64" },
> +  { "MSR_IMM",
> +    "64" },
>  };
>  
>  /* This array is populated as process_i386_initializers() walks cpu_flags[].  */
> @@ -406,6 +408,7 @@ static bitfield cpu_flags[] =
>    BITFIELD (USER_MSR),
>    BITFIELD (APX_F),
>    BITFIELD (AVX10_2),
> +  BITFIELD (MSR_IMM),
>    BITFIELD (MWAITX),
>    BITFIELD (CLZERO),
>    BITFIELD (OSPKE),
> --- a/opcodes/i386-opc.h
> +++ b/opcodes/i386-opc.h
> @@ -229,6 +229,8 @@ enum i386_cpu
>    CpuUSER_MSR,
>    /* Intel AVX10.2 Instructions support required.  */
>    CpuAVX10_2,
> +  /* Intel MSR_IMM Instructions support required.  */
> +  CpuMSR_IMM,
>    /* mwaitx instruction required */
>    CpuMWAITX,
>    /* Clzero instruction required */
> @@ -483,6 +485,7 @@ typedef union i386_cpu_flags
>        unsigned int cpulkgs:1;
>        unsigned int cpuuser_msr:1;
>        unsigned int cpuavx10_2:1;
> +      unsigned int cpumsr_imm:1;
>        unsigned int cpumwaitx:1;
>        unsigned int cpuclzero:1;
>        unsigned int cpuospke:1;

For all of the above - imo putting MSR-IMM right next to USER-MSR would
be neater.

> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -964,6 +964,7 @@ cpuid, 0xfa2, i486, NoSuf, {}
>  // Pentium extensions.
>  wrmsr, 0xf30, i586, NoSuf, {}
>  rdtsc, 0xf31, i586, NoSuf, {}
> +rdmsr, 0xf2f6/0, APX_F(MSR_IMM), Modrm|Vex128|SpaceMap7|EVex128|VexW0|NoSuf, { Imm32, Reg64 }
>  rdmsr, 0xf32, i586, NoSuf, {}
>  cmpxchg8b, 0xfc7/1, i586, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|NoRex64|HLEPrefixLock, { Qword|Unspecified|BaseIndex }
>  
> @@ -3373,6 +3374,9 @@ cmp<cc>xadd, 0x66e<cc:opc>, APX_F(CMPCCXADD), Modrm|Vex|EVex128|Space0F38|Src2VV
>  
>  // WRMSRNS instruction.
>  
> +// Immediates want to be first; md_assemble() takes care of swapping operands
> +// accordingly.
> +wrmsrns, 0xf3f6/0, APX_F(MSR_IMM), Modrm|Vex128|SpaceMap7|EVex128|VexW0|NoSuf, { Imm32, Reg64 }
>  wrmsrns, 0x0f01c6, WRMSRNS, NoSuf, {}
>  
>  // WRMSRNS instruction end.

Unless there are specific reasons (which you then would want to state in
the description), please add new forms _after_ pre-existing ones.

Jan