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 <jbeul...@suse.com>
>
> Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>
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
