On Wed, Nov 02, 2022, David Matlack wrote: > On Mon, Oct 31, 2022 at 11:28 AM Sean Christopherson <seanjc@xxxxxxxxxx> 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, Ha, and of course I found assert_exit_for_flds_emulation_failure() hard to read :-) > 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. How about "skip_flds_on_emulation_failure_exit()"? "handle" is quite generic and doesn't provide any hints as to what the function actually does under the hood.
