Re: [PATCH for 4.9 5/6] x86/emul: Drop swint_emulate infrastructure

["Jan Beulich" <JBeulich@suse.com>]

>>> On 31.03.17 at 21:50, <andrew.cooper3@citrix.com> wrote:
> With the SVM injection logic capable of doing its own emulation, there is no
> need for this hardware-specific assistance in the common emulator.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with one remark below.

> ---
>  tools/fuzz/x86_instruction_emulator/fuzz-emul.c |  18 +--
>  xen/arch/x86/hvm/emulate.c                      |   7 -
>  xen/arch/x86/mm.c                               |   2 -
>  xen/arch/x86/mm/shadow/common.c                 |   1 -
>  xen/arch/x86/x86_emulate/x86_emulate.c          | 187 ++++--------------------
>  xen/arch/x86/x86_emulate/x86_emulate.h          |  53 -------
>  6 files changed, 30 insertions(+), 238 deletions(-)

I like this.

> @@ -4103,25 +3966,38 @@ x86_emulate(
>              goto done;
>          break;
>  
> -    case 0xcc: /* int3 */
> -        src.val = EXC_BP;
> -        swint_type = x86_swint_int3;
> -        goto swint;
> -
> -    case 0xcd: /* int imm8 */
> -        swint_type = x86_swint_int;
> -    swint:
> -        rc = inject_swint(swint_type, (uint8_t)src.val,
> -                          _regs.r(ip) - ctxt->regs->r(ip),
> -                          ctxt, ops) ? : X86EMUL_EXCEPTION;
> -        goto done;
> -
>      case 0xce: /* into */
>          if ( !(_regs.eflags & X86_EFLAGS_OF) )
>              break;
> -        src.val = EXC_OF;
> -        swint_type = x86_swint_into;
> -        goto swint;
> +        /* Fallthrough */
> +    case 0xcc: /* int3 */
> +    case 0xcd: /* int imm8 */
> +    case 0xf1: /* int1 (icebp) */
> +        ASSERT(!ctxt->event_pending);
> +        switch ( ctxt->opcode )
> +        {
> +        case 0xcc: /* int3 */
> +            ctxt->event.vector = EXC_BP;
> +            ctxt->event.type = X86_EVENTTYPE_SW_EXCEPTION;
> +            break;
> +        case 0xcd: /* int imm8 */
> +            ctxt->event.vector = src.val;

I think with our current naming and fields use it might be marginally
better to use imm1 here; the R-b applies to both cases, though.

Jan


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