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

David Matlack
2022-11-02T18:17:44Z

On Mon, Oct 31, 2022 at 11:28 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Mon, Oct 31, 2022, David Matlack wrote:
> > +
> > +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.

I kept them separate for readability, but otherwise I have no argument
against bundling. I find skip_emulated*() somewhat misleading since
flds is not actually emulated (successfully). I'm trending toward
something like handle_flds_emulation_failure_exit() for v4.