Re: [PATCH] x86emul: convert op_bytes/opc checks in SIMD emulation

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.08.2024 17:10, Andrew Cooper wrote:
> On 21/08/2024 4:03 pm, Jan Beulich wrote:
>> On 21.08.2024 15:37, Andrew Cooper wrote:
>>> I think it would be helpful to at least note the fuzzing aspect in the
>>> commit message.
>> I've added "This way even for insns not covered by the test harness
>> fuzzers will have a chance of noticing issues, should any still exist
>> or new ones be introduced" to the 2nd paragraph.
> 
> Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Thanks.

>>>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>>>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>>>> @@ -8114,13 +8114,13 @@ x86_emulate(
>>>>      }
>>>>      else if ( state->simd_size != simd_none )
>>>>      {
>>>> -        generate_exception_if(!op_bytes, X86_EXC_UD);
>>>>          generate_exception_if((vex.opcx && (d & TwoOp) &&
>>>>                                 (vex.reg != 0xf || (evex_encoded() && 
>>>> !evex.RX))),
>>>>                                X86_EXC_UD);
>>>>  
>>>> -        if ( !opc )
>>>> -            BUG();
>>>> +        EXPECT(op_bytes);
>>>> +        EXPECT(opc);
>>> This is the only BUG() in x86_emulate.c, and it's right to get rid of it
>>> IMO.
>>>
>>> Therefore, we should have a hunk removing it from
>>> tools/tests/x86_emulator/x86-emulate.h too, which will prevent
>>> reintroduction.
>>>
>>> Maybe even undef BUG somewhere in x86_emulate/private.h?
>> Both of these actions can only be taken if the other BUG() in decode.c
>> also goes away. But yes, what you suggest is probably the best course of
>> action. I guess I'll do that in yet another patch, though.
> 
> Is that BUG() local to your tree?  I cant see it in staging.

I first thought it would be when you mentioned you found only one, but it's
been there for a long time[1], in VEX/EVEX prefix decoding. With a comment
added by you[2].

Jan

[1] 
https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=11c35f84b53622c429071049b830bb9b7a880eff
[2] 
https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=933acddf74213f77a5adbb2c10ccd3f72634d456