On Wed, Nov 2, 2022 at 12:03 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > 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. LGTM. Paolo can you apply the name change when queueing v4 (assuming there are no other comments)? If not I'd be happy to send a v5.
