On Sat, Sep 19, 2020 at 05:09:09PM +0200, Paolo Bonzini wrote: > On 17/09/20 18:29, Sean Christopherson wrote: > >> + vcpu->arch.efer = old_efer; > >> + kvm_make_request(KVM_REQ_OUT_OF_MEMORY, vcpu); > > I really dislike KVM_REQ_OUT_OF_MEMORY. It's redundant with -ENOMEM and > > creates a huge discrepancy with respect to existing code, e.g. nVMX returns > > -ENOMEM in a similar situation. > > Maxim, your previous version was adding some error handling to > kvm_x86_ops.set_efer. I don't remember what was the issue; did you have > any problems propagating all the errors up to KVM_SET_SREGS (easy), > kvm_set_msr (harder) etc.? I objected to letting .set_efer() return a fault. A relatively minor issue is the code in vmx_set_efer() that handles lack of EFER because technically KVM can emulate EFER.SCE+SYSCALL without supporting EFER in hardware. Returning success/'0' would avoid that particular issue. My primary concern is that I'd prefer not to add another case where KVM can potentially ignore a fault indicated by a helper, a la vmx_set_cr4(). To that end, I'd be ok with adding error handling to .set_efer() if KVM enforces, via WARN in one of the .set_efer() call sites, that SVM/VMX can only return negative error codes, i.e. let SVM handle the -ENOMEM case but disallow fault injection. It doesn't actually change anything, but it'd give me a warm fuzzy feeling.
