On 12/11/2024 2:59 pm, Jan Beulich wrote:
> Apparently I blindly copied the constants from the BEXTR case, where SF
> indeed wants leaving out. For BLSI, BLSMSK, BLSR, and BZHI SF is
> defined, and hence wants checking. This is noticable in particular for
> BLSR, where with the input we use SF will be set in the result (and
> hence is being switched to be clear on input).
>
> Convert to using named constants we have available, while omitting DF,
> TF, as well as the MBZ bits 3 and 5 from the masking values in the
> checks of the produced output. For BZHI also set SF on input, expecting
> it to transition to clear.
>
> Fixes: 771daacd197a ("x86emul: support BMI1 insns")
> Fixes: 8e20924de13d ("x86emul: support BMI2 insns")
> Signed-off-by: Jan Beulich <jbeul...@suse.com>Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com> That's horribly subtle, but I think I've been staring at the manuals long enough. However, there's a related bug elsewhere. I recently learnt that the rotate instructions are different between vendors. AMD leaves CF/OF well defined, others preserved, while Intel has CF well defined, and others undefined (seemingly zero in practice, but clearly there's a very old processor which wasn't). We test RCL and happen to fall into a common subset between vendors. At least the emulator itself dispatches to real instructions, so guests ought to see the behaviour correct for the CPU. > > --- a/tools/tests/x86_emulator/test_x86_emulator.c > +++ b/tools/tests/x86_emulator/test_x86_emulator.c > @@ -1969,10 +1969,13 @@ int main(int argc, char **argv) > > *res = 0xfedcba98; > regs.edx = (unsigned long)res; > - regs.eflags = 0xac2; > + regs.eflags = EFLAGS_ALWAYS_SET | X86_EFLAGS_OF | X86_EFLAGS_SF | \ > + X86_EFLAGS_ZF; > rc = x86_emulate(&ctxt, &emulops); > if ( (rc != X86EMUL_OKAY) || regs.ecx != 8 || *res != 0xfedcba98 || > - (regs.eflags & 0xf6b) != 0x203 || !check_eip(blsi) ) > + (regs.eflags & (EFLAGS_MASK & ~(X86_EFLAGS_AF | > X86_EFLAGS_PF))) != > + (EFLAGS_ALWAYS_SET | X86_EFLAGS_CF) || As an observation, this is really wanting for an EFL_SYM() helper like the others I have in XTF (I haven't needed one for flags specifically). The verbosity definitely interferes with the clarity. ~Andrew
