lkml.org 
[lkml]   [2020]   [Oct]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RESEND v4 2/2] KVM: VMX: Enable bus lock VM exit
From
Date


On 10/21/2020 6:19 AM, Sean Christopherson wrote:
> 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.
>

Got it.

>> + 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.
>

Make sense, I will add the prep patch.

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

\
 
 \ /
  Last update: 2020-10-26 06:33    [W:0.042 / U:0.128 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site