[PATCH] x86emul: correct AVX512BW write masking checks

[PATCH] x86emul: correct AVX512BW write masking checks

[Jan Beulich]

For VPSADBW this likely was a result of bad copy-and-paste.

For VPS{L,R}LDQ comment and code were not in line, but then again the
comment also wasn't fully updated from the AVX2 original it got cloned
from.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -6633,11 +6633,13 @@ x86_emulate(
         get_fpu(X86EMUL_FPU_mmx);
         goto simd_0f_common;
 
+    case X86EMUL_OPC_EVEX_66(0x0f, 0xf6): /* vpsadbw [xyz]mm/mem,[xyz]mm,[xyz]mm */
+        generate_exception_if(evex.opmsk, EXC_UD);
+        /* fall through */
     case X86EMUL_OPC_EVEX_66(0x0f, 0xd1): /* vpsrlw xmm/m128,[xyz]mm,[xyz]mm{k} */
     case X86EMUL_OPC_EVEX_66(0x0f, 0xe1): /* vpsraw xmm/m128,[xyz]mm,[xyz]mm{k} */
     case X86EMUL_OPC_EVEX_66(0x0f, 0xf1): /* vpsllw xmm/m128,[xyz]mm,[xyz]mm{k} */
     case X86EMUL_OPC_EVEX_66(0x0f, 0xf5): /* vpmaddwd [xyz]mm/mem,[xyz]mm,[xyz]mm{k} */
-    case X86EMUL_OPC_EVEX_66(0x0f, 0xf6): /* vpsadbw [xyz]mm/mem,[xyz]mm,[xyz]mm{k} */
         fault_suppression = false;
         /* fall through */
     case X86EMUL_OPC_EVEX_66(0x0f, 0xd5): /* vpmullw [xyz]mm/mem,[xyz]mm,[xyz]mm{k} */
@@ -6998,8 +7000,9 @@ x86_emulate(
         case 6: /* vpsllq $imm8,[xyz]mm/mem,[xyz]mm{k} */
             generate_exception_if(!evex.w, EXC_UD);
             goto avx512f_shift_imm;
-        case 3: /* vpsrldq $imm8,{x,y}mm,{x,y}mm */
-        case 7: /* vpslldq $imm8,{x,y}mm,{x,y}mm */
+        case 3: /* vpsrldq $imm8,[xyz]mm/mem,[xyz]mm */
+        case 7: /* vpslldq $imm8,[xyz]mm/mem,[xyz]mm */
+            generate_exception_if(evex.opmsk, EXC_UD);
             goto avx512bw_shift_imm;
         }
         goto unrecognized_insn;





_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] x86emul: correct AVX512BW write masking checks

[Andrew Cooper]

On 14/01/2019 11:40, Jan Beulich wrote:
> For VPSADBW this likely was a result of bad copy-and-paste.
>
> For VPS{L,R}LDQ comment and code were not in line, but then again the
> comment also wasn't fully updated from the AVX2 original it got cloned
> from.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

I'm guessing that these are covered by the first row of table 2-40, and
the absense of {k1} in the instruction specifications?

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] x86emul: correct AVX512BW write masking checks

[Jan Beulich]

>>> On 30.01.19 at 20:04, <andrew.cooper3@citrix.com> wrote:
> On 14/01/2019 11:40, Jan Beulich wrote:
>> For VPSADBW this likely was a result of bad copy-and-paste.
>>
>> For VPS{L,R}LDQ comment and code were not in line, but then again the
>> comment also wasn't fully updated from the AVX2 original it got cloned
>> from.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> I'm guessing that these are covered by the first row of table 2-40, and
> the absense of {k1} in the instruction specifications?

Yes, that's how I understand it. I'm also generally taking binutils'
opcodes table as secondary reference, so see whether the Intel
folks having implemented that had any information differing from
what the SDM or the ISA extensions doc say. XED may also be
useful as a secondary source, but I think I didn't check it for the
cases here.

Besides the absence of {k1} it's also the absence of "under write
mask k1" in the description column, and the operation section not
describing conditional write back. I usually only take all three
matching up as sufficient proof for there not simply being an
omission somewhere.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] x86emul: correct AVX512BW write masking checks

[Andrew Cooper]

On 31/01/2019 09:30, Jan Beulich wrote:
>>>> On 30.01.19 at 20:04, <andrew.cooper3@citrix.com> wrote:
>> On 14/01/2019 11:40, Jan Beulich wrote:
>>> For VPSADBW this likely was a result of bad copy-and-paste.
>>>
>>> For VPS{L,R}LDQ comment and code were not in line, but then again the
>>> comment also wasn't fully updated from the AVX2 original it got cloned
>>> from.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> I'm guessing that these are covered by the first row of table 2-40, and
>> the absense of {k1} in the instruction specifications?
> Yes, that's how I understand it. I'm also generally taking binutils'
> opcodes table as secondary reference, so see whether the Intel
> folks having implemented that had any information differing from
> what the SDM or the ISA extensions doc say. XED may also be
> useful as a secondary source, but I think I didn't check it for the
> cases here.
>
> Besides the absence of {k1} it's also the absence of "under write
> mask k1" in the description column, and the operation section not
> describing conditional write back. I usually only take all three
> matching up as sufficient proof for there not simply being an
> omission somewhere.

In some copious free time, I think it would be very interesting to try
and wire sandsifter up, avoid it skipping 0x62 as a prefix, and check
that x86_emulate() gives the same behaviour as the instruction executed
by sandsifter.  This would be excellent for executing all the corner
cases in the EVEX encoding, and it should be easy to cause sandsifter to
short circuit the disp8/disp32 scanning which accounts for most of its
time for a single instruction.

Either way, this change looks reasonable.  Reviewed-by: Andrew Cooper
<andrew.cooper3@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] x86emul: correct AVX512BW write masking checks

[Juergen Gross]

On 14/01/2019 12:40, Jan Beulich wrote:
> For VPSADBW this likely was a result of bad copy-and-paste.
> 
> For VPS{L,R}LDQ comment and code were not in line, but then again the
> comment also wasn't fully updated from the AVX2 original it got cloned
> from.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Release-acked-by: Juergen Gross <jgross@suse.com>


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel