>>> On 10.11.17 at 11:36, <yang.zh...@intel.com> wrote:
> Signed-off-by: Yang Zhong <yang.zh...@intel.com>
First and foremost - did you try out your own patch? There not being
any (minimal) test added makes this at least questionable.
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -385,6 +385,7 @@ static const struct {
> [0x40] = { .simd_size = simd_packed_int },
> [0x41] = { .simd_size = simd_packed_int, .two_op = 1 },
> [0xc8 ... 0xcd] = { .simd_size = simd_other },
> + [0xcf] = { .simd_size = simd_other },
Why simd_other? And if that's really the right choice, where do you
set op_bytes, which is required for this attribute due to
if ( state->simd_size )
{
generate_exception_if(!op_bytes, EXC_UD);
?
> @@ -7356,6 +7359,14 @@ x86_emulate(
> op_bytes = 16;
> goto simd_0f38_common;
>
> + case X86EMUL_OPC_66(0x0f38, 0xcf): /* gf2p8mulb xmm/m128,xmm*/
> + case X86EMUL_OPC_VEX_66(0x0f38, 0xcf): /* vgf2p8mulb xmm/m128,xmm*/
Please provide correct comments - the VEX variant comes with two
sizes. Also please add the missing blanks at the end.
> + host_and_vcpu_must_have(gfni);
> + if ( vex.opcx == vex_none )
> + goto simd_0f38_common;
> + else
Pointless else.
> + goto simd_0f_avx;
vex.w needs to be checked before this goto.
> @@ -7741,6 +7752,17 @@ x86_emulate(
> op_bytes = 16;
> goto simd_0f3a_common;
>
> + case X86EMUL_OPC_66(0x0f3a, 0xce): /* gf2p8affineqb
> $imm8,xmm/m128,xmm*/
> + case X86EMUL_OPC_VEX_66(0x0f3a, 0xce): /* vgf2p8affineqb
> $imm8,xmm/m128,xmm*/
> + case X86EMUL_OPC_66(0x0f3a, 0xcf): /* gf2p8affineinvqb
> $imm8,xmm/m128,xmm*/
> + case X86EMUL_OPC_VEX_66(0x0f3a, 0xcf): /* vgf2p8affineinvqb
> $imm8,xmm/m128,xmm*/
> + host_and_vcpu_must_have(gfni);
> + if ( vex.opcx == vex_none )
> + goto simd_0f3a_common;
> + else
> + goto simd_0f_imm8_avx;
Similar comments apply here.
> +
> +
Please don't introduce double blank lines.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
