On 04/04/2023 3:52 pm, Jan Beulich wrote: > Unconditionally wire this through the ->rmw() hook. Since x86_emul_rmw() > now wants to construct and invoke a stub, make stub_exn available to it > via a new field in the emulator state structure.
IMO, patch 5 should be re-ordered with this, because it removes one incidental change that's not really related to CMPccXADD. > > Signed-off-by: Jan Beulich <jbeul...@suse.com> > --- > # SDE: -grr or -srf The ISE makes a point of noting that CMPccXADD is implicitly locked, like XCHG. (Unlike XCHG, there isn't a valid reg/reg encoding.) Right now, the xchg emulation overrides lock_prefix, but I have a feeling that's stale now with the rmw() hook in place. But it is dubious that we let xchg fall back to a non-atomic exchange if the rmw() hook is missing. Either way, I think it would be nice to clean that up so we don't have differences in the handling of instructions which the ISE at least claims are similar. Tangentially, what about the RAO instructions? > --- a/tools/tests/x86_emulator/x86-emulate.h > +++ b/tools/tests/x86_emulator/x86-emulate.h > @@ -934,6 +935,8 @@ decode_0f38(struct x86_emulate_state *s, > ctxt->opcode |= MASK_INSR(s->vex.pfx, X86EMUL_OPC_PFX_MASK); > break; > > + case X86EMUL_OPC_VEX_66(0, 0xe0) > + ... X86EMUL_OPC_VEX_66(0, 0xef): /* cmp<cc>xadd */ I know the style is a little mixed in the emulator, but + case X86EMUL_OPC_VEX_66(0, 0xe0) ... + X86EMUL_OPC_VEX_66(0, 0xef): /* cmp<cc>xadd */ is more consistent with Xen style (because it's somewhat of a binary operator), and more readable IMO. > --- a/xen/include/public/arch-x86/cpufeatureset.h > +++ b/xen/include/public/arch-x86/cpufeatureset.h > @@ -278,6 +278,7 @@ XEN_CPUFEATURE(SSBD, 9*32+31) / > /* Intel-defined CPU features, CPUID level 0x00000007:1.eax, word 10 */ > XEN_CPUFEATURE(AVX_VNNI, 10*32+ 4) /*A AVX-VNNI Instructions */ > XEN_CPUFEATURE(AVX512_BF16, 10*32+ 5) /*A AVX512 BFloat16 Instructions */ > +XEN_CPUFEATURE(CMPCCXADD, 10*32+ 7) /*A CMPccXADD Instructions */ Given the non-triviality of this instruction, I'd prefer to keep this "a" until we've tried it on real hardware. ~Andrew
