Re: [PATCH v3 04/10] KVM: selftests: Move flds instruction emulation failure handling to header

[Sean Christopherson <seanjc@google.com>]

On Mon, Oct 31, 2022, David Matlack wrote:
> Move the flds instruction emulation failure handling code to a header
> so it can be re-used in an upcoming test.
> 
> No functional change intended.
> 
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
>  .../selftests/kvm/x86_64/flds_emulation.h     | 59 +++++++++++++++++++
>  .../smaller_maxphyaddr_emulation_test.c       | 45 ++------------
>  2 files changed, 64 insertions(+), 40 deletions(-)
>  create mode 100644 tools/testing/selftests/kvm/x86_64/flds_emulation.h
> 
> diff --git a/tools/testing/selftests/kvm/x86_64/flds_emulation.h b/tools/testing/selftests/kvm/x86_64/flds_emulation.h
> new file mode 100644
> index 000000000000..be0b4e0dd722
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86_64/flds_emulation.h
> @@ -0,0 +1,59 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef SELFTEST_KVM_FLDS_EMULATION_H
> +#define SELFTEST_KVM_FLDS_EMULATION_H
> +
> +#include "kvm_util.h"
> +
> +#define FLDS_MEM_EAX ".byte 0xd9, 0x00"
> +
> +/*
> + * flds is an instruction that the KVM instruction emulator is known not to
> + * support. This can be used in guest code along with a mechanism to force
> + * KVM to emulate the instruction (e.g. by providing an MMIO address) to
> + * exercise emulation failures.
> + */
> +static inline void flds(uint64_t address)
> +{
> +	__asm__ __volatile__(FLDS_MEM_EAX :: "a"(address));
> +}
> +
> +static inline void assert_exit_for_flds_emulation_failure(struct kvm_vcpu *vcpu)

I think it makes sense to keeping the bundling of the assert+skip.  As written,
the last test doesn't need to skip, but that may not always hold true, e.g. if
the test adds more stages to verify KVM handles page splits correctly, and even
when a skip is required, it does no harm.  I can't think of a scenario where a
test would want an FLDS emulation error but wouldn't want to skip the instruction,
e.g. injecting a fault from userspace is largely an orthogonal test.

Maybe this as a helper name?  I don't think it's necessary to include "assert"
anywhere in the name, the idea being that "emulated" provides a hint that it's a
non-trivial helper.  

  static inline void skip_emulated_flds(struct kvm_vcpu *vcpu)

or skip_emulated_flds_instruction() if we're concerned that it might not be obvious
"flds" is an instruction mnemonic.