Re: [Xen-devel] [PATCH v12 1/4] x86emul: New return code for unimplemented instruction

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.09.17 at 07:12, <ppircalabu@xxxxxxxxxxxxxxx> wrote:
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -6195,7 +6196,7 @@ x86_emulate(
>                  /* vpsll{w,d} $imm8,{x,y}mm,{x,y}mm */
>              break;
>          default:
> -            goto cannot_emulate;
> +            goto unimplemented_insn;

I would really appreciate if you were a little more patient and waited
for replies to earlier review threads before sending a new version.
As said on the v11 thread, this ought to be "unrecognized".

> @@ -6243,7 +6244,8 @@ x86_emulate(
>          case 6: /* psllq $imm8,mm */
>              goto simd_0f_shift_imm;
>          }
> -        goto cannot_emulate;
> +        rc = X86EMUL_UNRECOGNIZED;
> +        goto done;

I think it would read better if we had an "unrecognized_insn"
label just like now we gain an "unimplemented_insn" one. Whether
the _insn suffixes are really useful is another question.

> --- a/xen/arch/x86/x86_emulate/x86_emulate.h
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.h
> @@ -133,6 +133,19 @@ struct x86_emul_fpu_aux {
>    * Undefined behavior when used anywhere else.
>    */
>  #define X86EMUL_DONE           4
> + /*
> +  * Current instruction is not implemented by the emulator.
> +  * This value should only be returned by the core emulator when a valid
> +  * opcode is found but the execution logic for that instruction is missing.
> +  * It should NOT be returned by any of the x86_emulate_ops callbacks.
> +  */
> +#define X86EMUL_UNIMPLEMENTED  5
> + /*
> +  * The current instruction's opcode is not valid.
> +  * If this error code is returned by a function, an #UD trap should be
> +  * raised by the final consumer of it.
> + */
> +#define X86EMUL_UNRECOGNIZED   X86EMUL_UNIMPLEMENTED

But with this aliasing of values the comment still is somewhat off.

Jan


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