lkml.org 
[lkml]   [2020]   [Oct]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RESEND v4 2/2] KVM: VMX: Enable bus lock VM exit
    On Mon, Oct 12, 2020 at 11:35:42AM +0800, Chenyi Qiang wrote:
    > @@ -6138,6 +6149,26 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath)
    > return 0;
    > }
    >
    > +static int vmx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath)
    > +{
    > + int ret = __vmx_handle_exit(vcpu, exit_fastpath);
    > +
    > + /*
    > + * Even when current exit reason is handled by KVM internally, we
    > + * still need to exit to user space when bus lock detected to inform
    > + * that there is a bus lock in guest.
    > + */
    > + if (to_vmx(vcpu)->exit_reason.bus_lock_detected) {
    > + if (ret > 0)
    > + vcpu->run->exit_reason = KVM_EXIT_BUS_LOCK;
    > + else
    > + vcpu->run->flags |= KVM_RUN_BUS_LOCK;

    This should always set flags.KVM_RUN_BUS_LOCK, e.g. so that userspace can
    always check flags.KVM_RUN_BUS_LOCK instead of having to check both the flag
    and the exit reason. As is, it's really bad because the flag is undefined,
    which could teach userspace to do the wrong thing.

    > + return 0;
    > + }
    > + vcpu->run->flags &= ~KVM_RUN_BUS_LOCK;

    Hmm, I feel like explicitly clearing flags is should be unnecessary. By
    that, I mean that's it's necessary in the current patch, bit I think we should
    figure out how to make that not be the case. With the current approach, every
    chunk of code that needs to set a flag also needs to clear it, which increases
    the odds of missing a case and ending up with a flag in an undefined state.

    The easiest way I can think of is to add another prep patch that zeros
    run->flags at the beginning of kvm_arch_vcpu_ioctl_run(), and changes
    post_kvm_run_save() to do:

    if (is_smm(vcpu))
    kvm_run->flags |= KVM_RUN_X86_SMM;

    Then this patch can omit clearing KVM_RUN_BUS_LOCK, and doesn't have to touch
    the SMM flag.

    > + return ret;
    > +}
    > +
    > /*
    > * Software based L1D cache flush which is used when microcode providing
    > * the cache control MSR is not loaded.

    \
     
     \ /
      Last update: 2020-10-21 00:20    [W:2.643 / U:1.432 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site