[PATCH] x86emul: add memory operand low bits checks for ENQCMD{,S}

Jan Beulich posted 1 patch 1 year, 9 months ago
Failed in applying to current master (apply log)
[PATCH] x86emul: add memory operand low bits checks for ENQCMD{,S}
Posted by Jan Beulich 1 year, 9 months ago
Already ISE rev 044 added text to this effect; rev 045 further dropped
leftover earlier text indicating the contrary:
- ENQCMD requires the low 32 bits of the memory operand to be clear,
- ENDCMDS requires bits 20...30 of the memory operand to be clear.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
I'm a little reluctant to add a Fixes: tag here, because at the time
the code was written the behavior was matching what was documented.

--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -10499,6 +10499,7 @@ x86_emulate(
             goto done;
         if ( vex.pfx == vex_f2 ) /* enqcmd */
         {
+            generate_exception_if(mmvalp->data32[0], EXC_GP, 0);
             fail_if(!ops->read_msr);
             if ( (rc = ops->read_msr(MSR_PASID, &msr_val,
                                      ctxt)) != X86EMUL_OKAY )
@@ -10506,7 +10507,8 @@ x86_emulate(
             generate_exception_if(!(msr_val & PASID_VALID), EXC_GP, 0);
             mmvalp->data32[0] = MASK_EXTR(msr_val, PASID_PASID_MASK);
         }
-        mmvalp->data32[0] &= ~0x7ff00000;
+        else
+            generate_exception_if(mmvalp->data32[0] & 0x7ff00000, EXC_GP, 0);
         state->blk = blk_enqcmd;
         if ( (rc = ops->blk(x86_seg_es, src.val, mmvalp, 64, &_regs.eflags,
                             state, ctxt)) != X86EMUL_OKAY )
Re: [PATCH] x86emul: add memory operand low bits checks for ENQCMD{,S}
Posted by Andrew Cooper 1 year, 9 months ago
On 19/07/2022 13:56, Jan Beulich wrote:
> Already ISE rev 044 added text to this effect; rev 045 further dropped
> leftover earlier text indicating the contrary:
> - ENQCMD requires the low 32 bits of the memory operand to be clear,
> - ENDCMDS requires bits 20...30 of the memory operand to be clear.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> I'm a little reluctant to add a Fixes: tag here, because at the time
> the code was written the behavior was matching what was documented.

It needs a tag, because this is fixing a problem in a previous patch,
and in principle wants backporting to 4.14.

It doesn't matter the cause of the error, and "Intel changed their
documentation" is pretty good as far as excuses go.

As far as the change goes, that does seem to match the latest docs.

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>