Re: [PATCH 8/9] x86emul: support AVX-NE-CONVERT insns

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.04.2023 23:22, Andrew Cooper wrote:
> On 04/04/2023 3:54 pm, Jan Beulich wrote:
>> Matching what was done earlier, explicit tests are added only for
>> irregular insn / memory access patterns.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>,

Thanks.

> with two minor requests.
> 
>> --- a/tools/misc/xen-cpuid.c
>> +++ b/tools/misc/xen-cpuid.c
>> @@ -214,7 +214,7 @@ static const char *const str_7c1[32] =
>>  
>>  static const char *const str_7d1[32] =
>>  {
>> -    [ 4] = "avx-vnni-int8",
>> +    [ 4] = "avx-vnni-int8", [ 5] = "avx-ne-convert",
> 
> I'd leave a bit more horizontal space.  These names are getting rather
> long, and we're only 10% into this word.

Sure. I had taken neighboring arrays as reference; I've now switched
to "aligning" with rtm-always-abort.

>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>> @@ -6208,6 +6208,19 @@ x86_emulate(
>>          host_and_vcpu_must_have(avx512_vbmi2);
>>          goto avx512f_no_sae;
>>  
>> +    case X86EMUL_OPC_VEX   (0x0f38, 0xb0): /* vcvtneoph2ps mem,[xy]mm */
>> +    case X86EMUL_OPC_VEX_66(0x0f38, 0xb0): /* vcvtneeph2ps mem,[xy]mm */
>> +    case X86EMUL_OPC_VEX_F3(0x0f38, 0xb0): /* vcvtneebf162ps mem,[xy]mm */
>> +    case X86EMUL_OPC_VEX_F2(0x0f38, 0xb0): /* vcvtneobf162ps mem,[xy]mm */
>> +        generate_exception_if(ea.type != OP_MEM, EXC_UD);
>> +        /* fall through */
> 
> Only just occurred to me, but we should probably be using fallthrough;
> in new code, now there's a real attribute to use.

I did actually consider doing so (and iirc also already on an earlier
occasion), but that'll be yet another item we also need to cater for
in the harness'es x86-emulate.h. For the moment I prefer to stick to
comments, switching over - if necessary for e.g. Misra - all in one
go at some point.

Jan