|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3] x86/emul: Fix the handling of unimplemented Grp7 instructions
Reviewed-by: Petre Pircalabu <ppircalabu@xxxxxxxxxxxxxxx>
On Ma, 2017-09-05 at 09:41 +0100, Andrew Cooper wrote:
> Grp7 is abnormally complicated to decode, even by x86's standards,
> with
> {s,l}msw being the problematic cases.
>
> Previously, any value which fell through the first switch statement
> (looking
> for instructions with entirely implicit operands) would be
> interpreted by the
> second switch statement (handling instructions with memory operands).
>
> Unimplemented instructions would then hit the #UD case for having a
> non-memory
> operand, rather than taking the cannot_emulate path.
>
> Consolidate the two switch statements into a single one, using ranges
> to cover
> the instructions with memory operands.
>
> This also fixes the emulation of xend, which would hit the #UD path
> when it
> should complete with no side effects.
>
> Reported-by: Petre Pircalabu <ppircalabu@xxxxxxxxxxxxxxx>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Petre Pircalabu <ppircalabu@xxxxxxxxxxxxxxx>
>
> v3:
> * Use a single switch statement.
>
> v2:
> * Use break rather than goto complete_insn for implicit
> instructions.
> * Note that we actually fix the behaviour of xend.
>
> RFC as I've only done light testing so far.
> ---
> xen/arch/x86/x86_emulate/x86_emulate.c | 49 +++++++++++++++++++++---
> ----------
> 1 file changed, 31 insertions(+), 18 deletions(-)
>
> diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c
> b/xen/arch/x86/x86_emulate/x86_emulate.c
> index 2201852..43e0e00 100644
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -4987,9 +4987,12 @@ x86_emulate(
> }
> break;
>
> - case X86EMUL_OPC(0x0f, 0x01): /* Grp7 */ {
> + case X86EMUL_OPC(0x0f, 0x01): /* Grp7 */
> + {
> unsigned long base, limit, cr0, cr0w;
>
> + seg = (modrm_reg & 1) ? x86_seg_idtr : x86_seg_gdtr;
> +
> switch( modrm )
> {
> case 0xca: /* clac */
> @@ -5000,7 +5003,7 @@ x86_emulate(
> _regs.eflags &= ~X86_EFLAGS_AC;
> if ( modrm == 0xcb )
> _regs.eflags |= X86_EFLAGS_AC;
> - goto complete_insn;
> + break;
>
> #ifdef __XEN__
> case 0xd1: /* xsetbv */
> @@ -5012,7 +5015,7 @@ x86_emulate(
> handle_xsetbv(_regs.ecx,
> _regs.eax |
> (_regs.rdx << 32)),
> EXC_GP, 0);
> - goto complete_insn;
> + break;
> #endif
>
> case 0xd4: /* vmfunc */
> @@ -5020,7 +5023,7 @@ x86_emulate(
> fail_if(!ops->vmfunc);
> if ( (rc = ops->vmfunc(ctxt)) != X86EMUL_OKAY )
> goto done;
> - goto complete_insn;
> + break;
>
> case 0xd5: /* xend */
> generate_exception_if(vex.pfx, EXC_UD);
> @@ -5034,7 +5037,7 @@ x86_emulate(
> EXC_UD);
> /* Neither HLE nor RTM can be active when we get here.
> */
> _regs.eflags |= X86_EFLAGS_ZF;
> - goto complete_insn;
> + break;
>
> case 0xdf: /* invlpga */
> generate_exception_if(!in_protmode(ctxt, ops), EXC_UD);
> @@ -5043,7 +5046,7 @@ x86_emulate(
> if ( (rc = ops->invlpg(x86_seg_none,
> truncate_ea(_regs.r(ax)),
> ctxt)) )
> goto done;
> - goto complete_insn;
> + break;
>
> case 0xf9: /* rdtscp */
> fail_if(ops->read_msr == NULL);
> @@ -5091,16 +5094,16 @@ x86_emulate(
> base += sizeof(zero);
> limit -= sizeof(zero);
> }
> - goto complete_insn;
> - }
> + break;
> }
>
> - seg = (modrm_reg & 1) ? x86_seg_idtr : x86_seg_gdtr;
> +#define _GRP7(mod, reg) \
> + (((mod) << 6) | ((reg) << 3)) ... (((mod) << 6) | ((reg)
> << 3) | 7)
> +#define GRP7_MEM(reg) _GRP7(0, reg): case _GRP7(1, reg): case
> _GRP7(2, reg)
> +#define GRP7_ALL(reg) GRP7_MEM(reg): case _GRP7(3, reg)
>
> - switch ( modrm_reg & 7 )
> - {
> - case 0: /* sgdt */
> - case 1: /* sidt */
> + case GRP7_MEM(0): /* sgdt */
> + case GRP7_MEM(1): /* sidt */
> generate_exception_if(ea.type != OP_MEM, EXC_UD);
> generate_exception_if(umip_active(ctxt, ops), EXC_GP,
> 0);
> fail_if(!ops->read_segment || !ops->write);
> @@ -5119,8 +5122,9 @@ x86_emulate(
> op_bytes, ctxt)) != X86EMUL_OKAY )
> goto done;
> break;
> - case 2: /* lgdt */
> - case 3: /* lidt */
> +
> + case GRP7_MEM(2): /* lgdt */
> + case GRP7_MEM(3): /* lidt */
> generate_exception_if(!mode_ring0(), EXC_GP, 0);
> generate_exception_if(ea.type != OP_MEM, EXC_UD);
> fail_if(ops->write_segment == NULL);
> @@ -5138,7 +5142,8 @@ x86_emulate(
> if ( (rc = ops->write_segment(seg, &sreg, ctxt)) )
> goto done;
> break;
> - case 4: /* smsw */
> +
> + case GRP7_ALL(4): /* smsw */
> generate_exception_if(umip_active(ctxt, ops), EXC_GP,
> 0);
> if ( ea.type == OP_MEM )
> {
> @@ -5153,7 +5158,8 @@ x86_emulate(
> if ( (rc = ops->read_cr(0, &dst.val, ctxt)) )
> goto done;
> break;
> - case 6: /* lmsw */
> +
> + case GRP7_ALL(6): /* lmsw */
> fail_if(ops->read_cr == NULL);
> fail_if(ops->write_cr == NULL);
> generate_exception_if(!mode_ring0(), EXC_GP, 0);
> @@ -5169,16 +5175,23 @@ x86_emulate(
> if ( (rc = ops->write_cr(0, cr0, ctxt)) )
> goto done;
> break;
> - case 7: /* invlpg */
> +
> + case GRP7_MEM(7): /* invlpg */
> generate_exception_if(!mode_ring0(), EXC_GP, 0);
> generate_exception_if(ea.type != OP_MEM, EXC_UD);
> fail_if(ops->invlpg == NULL);
> if ( (rc = ops->invlpg(ea.mem.seg, ea.mem.off, ctxt)) )
> goto done;
> break;
> +
> default:
> goto cannot_emulate;
> }
> +
> +#undef GRP7_ALL
> +#undef GRP7_MEM
> +#undef _GRP7
> +
> break;
> }
>
________________________
This email was scanned by Bitdefender
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |