lkml.org 
[lkml]   [2020]   [Jun]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 30/30] KVM: nSVM: implement KVM_GET_NESTED_STATE and KVM_SET_NESTED_STATE
    From
    Date
    Sorry I missed this.

    On 02/06/20 02:11, Krish Sadhukhan wrote:
    >>
    >> +
    >> +    /* SMM temporarily disables SVM, so we cannot be in guest mode.  */
    >> +    if (is_smm(vcpu) && (kvm_state->flags &
    >> KVM_STATE_NESTED_GUEST_MODE))
    >> +        return -EINVAL;
    >> +
    >> +    if (!(kvm_state->flags & KVM_STATE_NESTED_GUEST_MODE)) {
    >
    >
    > Should this be done up at the beginning of the function ? If this flag
    > isn't set, we probably don't want to come this far.

    So far we have only done consistency checks. These have to be done no
    matter what (for example checking that GIF=1 if SVME=0).

    >> +        svm_leave_nested(svm);
    >> +        goto out_set_gif;
    >> +    }
    >> +
    >> +    if (!page_address_valid(vcpu, kvm_state->hdr.svm.vmcb_pa))
    >> +        return -EINVAL;
    >> +    if (kvm_state->size < sizeof(*kvm_state) +
    >> KVM_STATE_NESTED_SVM_VMCB_SIZE)
    >> +        return -EINVAL;
    >> +    if (copy_from_user(&ctl, &user_vmcb->control, sizeof(ctl)))
    >> +        return -EFAULT;
    >> +    if (copy_from_user(&save, &user_vmcb->save, sizeof(save)))
    >> +        return -EFAULT;
    >> +
    >> +    if (!nested_vmcb_check_controls(&ctl))
    >> +        return -EINVAL;
    >> +
    >> +    /*
    >> +     * Processor state contains L2 state.  Check that it is
    >> +     * valid for guest mode (see nested_vmcb_checks).
    >> +     */
    >> +    cr0 = kvm_read_cr0(vcpu);
    >> +        if (((cr0 & X86_CR0_CD) == 0) && (cr0 & X86_CR0_NW))
    >> +                return -EINVAL;
    >
    >
    > Does it make sense to create a wrapper for the CR0 checks ? We have
    > these checks in nested_vmcb_check_controls() also.

    Not in nested_vmcb_check_controls (rather nested_vmcb_checks as
    mentioned in the comments).

    If there are more checks it certainly makes sense to have them. Right
    now however there are only two checks in svm_set_nested_state, and they
    come from two different functions so I chose to duplicate them.

    Paolo

    \
     
     \ /
      Last update: 2020-06-04 16:48    [W:8.836 / U:0.348 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site