Re: [PATCH v2] x86emul: adjust BSF/BSR/LZCNT/TZCNT behavior as to EFLAGS

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.07.2025 18:19, Andrew Cooper wrote:
> On 14/07/2025 5:02 pm, Jan Beulich wrote:
>> SDM revision 087 points out that apparently as of quite some time ago on
>> Intel hardware BSF and BSR may alter all arithmetic flags, not just ZF.
>> Because of the inconsistency (and because documentation doesn't look to
> 
> It's probably worth saying errata explicitly.  There are a whole bunch
> of Intel CPUs where the behaviour doesn't match CPUID.

Okay, I've adjusted the wording slightly.

>> be quite right about PF), best we can do is simply take the flag values
>> from what the processor produces, just like we do for various other
>> arithmetic insns. (Note also that AMD and Intel have always been
>> disagreeing on arithmetic flags other than ZF.) To be both safe (against
>> further anomalies) and consistent, extend this to {L,T}ZCNT as well.
>> (Emulating the two insns correctly even when underlying hardware doesn't
>> support it was perhaps nice, but yielded guest-observable
>> inconsistencies.)
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Reviewed-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
>> @@ -5270,62 +5270,26 @@ x86_emulate(
>>          break;
>>  
>>      case X86EMUL_OPC(0x0f, 0xbc): /* bsf or tzcnt */
>> -    {
>> -        bool zf;
>> -
>> -        asm ( "bsf %2,%0" ASM_FLAG_OUT(, "; setz %1")
>> -              : "=r" (dst.val), ASM_FLAG_OUT("=@ccz", "=qm") (zf)
>> -              : "rm" (src.val) );
>> -        _regs.eflags &= ~X86_EFLAGS_ZF;
>> -        if ( (vex.pfx == vex_f3) && vcpu_has_bmi1() )
>> -        {
>> -            _regs.eflags &= ~X86_EFLAGS_CF;
>> -            if ( zf )
>> -            {
>> -                _regs.eflags |= X86_EFLAGS_CF;
>> -                dst.val = op_bytes * 8;
>> -            }
>> -            else if ( !dst.val )
>> -                _regs.eflags |= X86_EFLAGS_ZF;
>> -        }
>> -        else if ( zf )
>> +        if ( vex.pfx == vex_f3 )
>> +            emulate_2op_SrcV_srcmem("rep; bsf", src, dst, _regs.eflags);
> 
> Do we need the ; ?
> 
> We surely don't on 4.21, but I presume there are bugs in older
> binutils?  (All Clangs back to 3.5 seem happy)

Right, we don't really need this on staging. I can omit it and then hopefully
not forget to add it back when backporting (which I was intending to do).

Jan