[2/3] x86: RMPUPDATE wants operands in different form

Message ID 367987fe-c0b8-412a-a5ad-84ff6ab7c059@suse.com
State New
Headers
Series x86: SNP improvements |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Jan Beulich Jan. 23, 2025, 7:59 a.m. UTC
  AMD are about to update their doc, to help clarify that what we
currently do isn't quite right: In particular it is not %rax but %rcx
which is affected by address size. In fact, that's a normal memory
operand, just not expressed via ModR/M byte, but fixed to (%rcx) (or
(%ecx) with 32-bit addressing).

To support this in the assembler, generalize memory operand handling so
far specific to XLAT (which isn't really a string insn, but requires its
memory operand to be (%bx) / (%ebx) / (%rbx)).

In the disassembler mimic handling after XLAT's, too.
---
As said above, having the rAX operand affect address size is wrong. I'm
uncertain though whether in the two affected templates we should
actually correct this, as people may in principle already be using
either of those forms to control the address size prefix (short of a
correct way for doing so). Then again it seems pretty unlikely that
address size overrides might be used in practice with this insn, for
other than testcase purposes.
  

Patch

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -6999,10 +6999,10 @@  i386_assemble (char *line)
 
   /* All Intel opcodes have reversed operands except for "bound", "enter",
      "invlpg*", "monitor*", "mwait*", "tpause", "umwait", "pvalidate",
-     "rmpadjust", "rmpupdate", and "rmpquery".  We also don't reverse
-     intersegment "jmp" and "call" instructions with 2 immediate operands so
-     that the immediate segment precedes the offset consistently in Intel and
-     AT&T modes.  */
+     "rmpadjust", "rmpquery", and deprecated forms of "rmpupdate".
+     We also don't reverse intersegment "jmp" and "call" instructions with
+     2 immediate operands so that the immediate segment precedes the offset
+     consistently in Intel and AT&T modes.  */
   if (intel_syntax
       && i.operands > 1
       && (t->mnem_off != MN_bound)
@@ -7010,7 +7010,7 @@  i386_assemble (char *line)
       && !startswith (mnemonic, "monitor")
       && !startswith (mnemonic, "mwait")
       && (t->mnem_off != MN_pvalidate)
-      && !startswith (mnemonic, "rmp")
+      && (!startswith (mnemonic, "rmp") || i.mem_operands)
       && (t->mnem_off != MN_tpause)
       && (t->mnem_off != MN_umwait)
       && !(i.operands == 2
@@ -14878,7 +14878,7 @@  i386_index_check (const char *operand_st
   if (t->opcode_modifier.isstring)
     {
       /* Memory operands of string insns are special in that they only allow
-	 a single register (rDI, rSI, or rBX) as their memory address.  */
+	 a single register (rDI or rSI) as their memory address.  */
       const reg_entry *expected_reg;
       static const char di_si[][2][4] =
 	{
@@ -14886,7 +14886,14 @@  i386_index_check (const char *operand_st
 	  { "si", "di" },
 	  { "rsi", "rdi" }
 	};
-      static const char bx[][4] = { "ebx", "bx", "rbx" };
+      /* For a few other insns with fixed register addressing we (ab)use the
+	 IsString attribute as well.  */
+      static const char loregs[][4][4] =
+	{
+	  { "eax", "ecx", "edx", "ebx" },
+	  {  "ax",  "cx",  "dx",  "bx" },
+	  { "rax", "rcx", "rdx", "rbx" }
+	};
 
       kind = "string address";
 
@@ -14904,8 +14911,16 @@  i386_index_check (const char *operand_st
 						 di_si[addr_mode][op == es_op]);
 	}
       else
-	expected_reg
-	  = (const reg_entry *)str_hash_find (reg_hash, bx[addr_mode]);
+	{
+	  unsigned int op = t->operand_types[0].bitfield.baseindex ? 0 : 1;
+
+	  if (!t->operand_types[op].bitfield.instance)
+	    return 1; /* Operand mismatch will be detected elsewhere.  */
+	  expected_reg
+	    = str_hash_find (reg_hash,
+			     loregs[addr_mode][t->operand_types[op]
+					       .bitfield.instance - 1]);
+	}
 
       if (i.base_reg != expected_reg
 	  || i.index_reg
--- a/gas/testsuite/gas/i386/snp.s
+++ b/gas/testsuite/gas/i386/snp.s
@@ -11,7 +11,8 @@  att:
         psmash	%eax
         rmpupdate
         rmpupdate %rax, %rcx
-        rmpupdate %eax, %rcx
+        rmpupdate (%rcx), %rax
+        rmpupdate (%ecx), %rax
         rmpadjust
         rmpadjust %rax, %rcx, %rdx
         rmpadjust %eax, %rcx, %rdx
@@ -30,7 +31,8 @@  intel:
         psmash	eax
         rmpupdate
         rmpupdate rax, rcx
-        rmpupdate eax, rcx
+        rmpupdate rax, [rcx]
+        rmpupdate rax, [ecx]
         rmpadjust
         rmpadjust rax, rcx, rdx
         rmpadjust eax, rcx, rdx
--- a/gas/testsuite/gas/i386/snp64.d
+++ b/gas/testsuite/gas/i386/snp64.d
@@ -14,9 +14,10 @@  Disassembly of section \.text:
 [ 	]*[a-f0-9]+:[ 	]+f3 0f 01 ff[ 	]+psmash
 [ 	]*[a-f0-9]+:[ 	]+f3 0f 01 ff[ 	]+psmash
 [ 	]*[a-f0-9]+:[ 	]+67 f3 0f 01 ff[ 	]+addr32 psmash
-[ 	]*[a-f0-9]+:[ 	]+f2 0f 01 fe[ 	]+rmpupdate
-[ 	]*[a-f0-9]+:[ 	]+f2 0f 01 fe[ 	]+rmpupdate
-[ 	]*[a-f0-9]+:[ 	]+67 f2 0f 01 fe[ 	]+addr32 rmpupdate
+[ 	]*[a-f0-9]+:[ 	]+f2 0f 01 fe[ 	]+rmpupdate \(%rcx\),%rax
+[ 	]*[a-f0-9]+:[ 	]+f2 0f 01 fe[ 	]+rmpupdate \(%rcx\),%rax
+[ 	]*[a-f0-9]+:[ 	]+f2 0f 01 fe[ 	]+rmpupdate \(%rcx\),%rax
+[ 	]*[a-f0-9]+:[ 	]+67 f2 0f 01 fe[ 	]+rmpupdate \(%ecx\),%rax
 [ 	]*[a-f0-9]+:[ 	]+f3 0f 01 fe[ 	]+rmpadjust
 [ 	]*[a-f0-9]+:[ 	]+f3 0f 01 fe[ 	]+rmpadjust
 [ 	]*[a-f0-9]+:[ 	]+67 f3 0f 01 fe[ 	]+addr32 rmpadjust
@@ -28,9 +29,10 @@  Disassembly of section \.text:
 [ 	]*[a-f0-9]+:[ 	]+f3 0f 01 ff[ 	]+psmash
 [ 	]*[a-f0-9]+:[ 	]+f3 0f 01 ff[ 	]+psmash
 [ 	]*[a-f0-9]+:[ 	]+67 f3 0f 01 ff[ 	]+addr32 psmash
-[ 	]*[a-f0-9]+:[ 	]+f2 0f 01 fe[ 	]+rmpupdate
-[ 	]*[a-f0-9]+:[ 	]+f2 0f 01 fe[ 	]+rmpupdate
-[ 	]*[a-f0-9]+:[ 	]+67 f2 0f 01 fe[ 	]+addr32 rmpupdate
+[ 	]*[a-f0-9]+:[ 	]+f2 0f 01 fe[ 	]+rmpupdate \(%rcx\),%rax
+[ 	]*[a-f0-9]+:[ 	]+f2 0f 01 fe[ 	]+rmpupdate \(%rcx\),%rax
+[ 	]*[a-f0-9]+:[ 	]+f2 0f 01 fe[ 	]+rmpupdate \(%rcx\),%rax
+[ 	]*[a-f0-9]+:[ 	]+67 f2 0f 01 fe[ 	]+rmpupdate \(%ecx\),%rax
 [ 	]*[a-f0-9]+:[ 	]+f3 0f 01 fe[ 	]+rmpadjust
 [ 	]*[a-f0-9]+:[ 	]+f3 0f 01 fe[ 	]+rmpadjust
 [ 	]*[a-f0-9]+:[ 	]+67 f3 0f 01 fe[ 	]+addr32 rmpadjust
--- a/gas/testsuite/gas/i386/x86-64-arch-4.d
+++ b/gas/testsuite/gas/i386/x86-64-arch-4.d
@@ -25,7 +25,7 @@  Disassembly of section .text:
 [ 	]*[a-f0-9]+:	c4 e2 4d df 39[ 	]+vaesdeclast \(%rcx\),%ymm6,%ymm7
 [ 	]*[a-f0-9]+:	f3 0f 01 ff[ 	]+psmash
 [ 	]*[a-f0-9]+:	f2 0f 01 ff[ 	]+pvalidate
-[ 	]*[a-f0-9]+:	f2 0f 01 fe[ 	]+rmpupdate
+[ 	]*[a-f0-9]+:	f2 0f 01 fe[ 	]+rmpupdate \(%rcx\),%rax
 [ 	]*[a-f0-9]+:	f3 0f 01 fe[ 	]+rmpadjust
 [ 	]*[a-f0-9]+:	66 0f 38 82 10[ 	]+invpcid \(%rax\),%rdx
 [ 	]*[a-f0-9]+:	0f 01 ee[ 	]+rdpkru
--- a/opcodes/i386-dis.c
+++ b/opcodes/i386-dis.c
@@ -524,6 +524,7 @@  fetch_error (const instr_info *ins)
 #define Xz { OP_DSreg, eSI_reg }
 #define Yb { OP_ESreg, eDI_reg }
 #define Yv { OP_ESreg, eDI_reg }
+#define DSCX { OP_DSreg, eCX_reg }
 #define DSBX { OP_DSreg, eBX_reg }
 
 #define es { OP_REG, es_reg }
@@ -4640,7 +4641,7 @@  static const struct dis386 x86_64_table[
   /* X86_64_0F01_REG_7_MOD_3_RM_6_PREFIX_3 */
   {
     { Bad_Opcode },
-    { "rmpupdate",	{ Skip_MODRM }, 0 },
+    { "rmpupdate",	{ RMrAX, DSCX, Skip_MODRM }, 0 },
   },
 
   /* X86_64_0F01_REG_7_MOD_3_RM_7_PREFIX_1 */
@@ -13142,6 +13143,8 @@  OP_DSreg (instr_info *ins, int code, int
     {
       switch (ins->codep[-1])
 	{
+	case 0x01:	/* rmpupdate */
+	  break;
 	case 0x6f:	/* outsw/outsl */
 	  intel_operand_size (ins, z_mode, sizeflag);
 	  break;
--- a/opcodes/i386-opc.tbl
+++ b/opcodes/i386-opc.tbl
@@ -596,7 +596,7 @@  ssto, 0xaa, 0, W|No_sSuf|RepPrefixOk, {}
 ssto, 0xaa, 0, W|No_sSuf|IsStringEsOp0|RepPrefixOk, { Byte|Word|Dword|Qword|Unspecified|BaseIndex }
 ssto, 0xaa, 0, W|No_sSuf|IsStringEsOp1|RepPrefixOk, { Acc|Byte|Word|Dword|Qword, Byte|Word|Dword|Qword|Unspecified|BaseIndex }
 xlat, 0xd7, 0, No_wSuf|No_lSuf|No_sSuf|No_qSuf|IntelSuffix, {}
-xlat, 0xd7, 0, No_wSuf|No_lSuf|No_sSuf|No_qSuf|IsString|IntelSuffix, { Byte|Unspecified|BaseIndex }
+xlat, 0xd7, 0, No_wSuf|No_lSuf|No_sSuf|No_qSuf|IsString|IntelSuffix, { RegB|Byte|Unspecified|BaseIndex }
 
 // Bit manipulation.
 bsf, 0xfbc, i386, Modrm|CheckOperandSize|No_bSuf|No_sSuf|RepPrefixOk, { Reg16|Reg32|Reg64|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
@@ -3159,6 +3159,7 @@  psmash, 0xf30f01ff, SNP&x64, NoSuf, {}
 psmash, 0xf30f01ff, SNP&x64, AddrPrefixOpReg|NoSuf, { Acc|Dword|Qword }
 pvalidate, 0xf20f01ff, SNP, NoSuf, {}
 pvalidate, 0xf20f01ff, SNP, AddrPrefixOpReg|NoSuf, { Acc|Word|Dword|Qword, RegC|Dword, RegD|Dword }
+// These two forms exist only for compatibility with older gas.
 rmpupdate, 0xf20f01fe, SNP&x64, NoSuf, {}
 rmpupdate, 0xf20f01fe, SNP&x64, AddrPrefixOpReg|NoSuf, { Acc|Dword|Qword, RegC|Qword }
 rmpadjust, 0xf30f01fe, SNP&x64, NoSuf, {}
@@ -3168,6 +3169,9 @@  pvalidate, 0xf20f01ff, SNP, AddrPrefixOp
 rmpupdate, 0xf20f01fe, SNP&x64, AddrPrefixOpReg|NoSuf, { Acc|Dword|Qword }
 rmpadjust, 0xf30f01fe, SNP&x64, AddrPrefixOpReg|NoSuf, { Acc|Dword|Qword }
 
+// This must come last, for its IsString attribute to take effect.
+rmpupdate, 0xf20f01fe, SNP&x64, IsString|NoSuf|NoRex64, { RegC|Unspecified|BaseIndex, Acc|Qword }
+
 // SNP instructions end
 
 // RMPQUERY instruction