Messages in this thread | | | Subject | Re: [RFC v2 2/2] KVM: VMX: Enable bus lock VM exit | From | Xiaoyao Li <> | Date | Thu, 3 Sep 2020 08:52:41 +0800 |
| |
On 9/3/2020 6:44 AM, Sean Christopherson wrote: > On Tue, Sep 01, 2020 at 10:43:12AM +0200, Vitaly Kuznetsov wrote: >>> @@ -6809,6 +6824,19 @@ static fastpath_t vmx_vcpu_run(struct kvm_vcpu *vcpu) >>> if (unlikely(vmx->exit_reason.failed_vmentry)) >>> return EXIT_FASTPATH_NONE; >>> >>> + /* >>> + * check the exit_reason to see if there is a bus lock >>> + * happened in guest. >>> + */ >>> + if (kvm_bus_lock_exit_enabled(vmx->vcpu.kvm)) { >>> + if (vmx->exit_reason.bus_lock_detected) { >>> + vcpu->stat.bus_locks++; > > Why bother with stats? Every bus lock exits to userspace, having quick > stats doesn't seem all that interesting.
OK. We will remove it.
>>> + vcpu->arch.bus_lock_detected = true; >>> + } else { >>> + vcpu->arch.bus_lock_detected = false; >> >> This is a fast path so I'm wondering if we can move bus_lock_detected >> clearing somewhere else. > > Why even snapshot vmx->exit_reason.bus_lock_detected? I don't see any > reason why vcpu_enter_guest() needs to handle the exit to userspace, e.g. > it's just as easily handled in VMX code.
Because we want to handle the exit to userspace only in one place, i.e., after kvm_x86_ops.handle_exit(vcpu, exit_fastpath). Otherwise, we would have to check vmx->exit_reason.bus_lock_detected in every other handler, at least in those can preempt the bus lock VM-exit theoretically.
>> >>> + } >>> + } >>> + >>> vmx->loaded_vmcs->launched = 1; >>> vmx->idt_vectoring_info = vmcs_read32(IDT_VECTORING_INFO_FIELD); >>> >>> @@ -8060,6 +8088,9 @@ static __init int hardware_setup(void) >>> kvm_tsc_scaling_ratio_frac_bits = 48; >>> } >>> >>> + if (cpu_has_vmx_bus_lock_detection()) >>> + kvm_has_bus_lock_exit = true; >>> + >>> set_bit(0, vmx_vpid_bitmap); /* 0 is reserved for host */ >>> >>> if (enable_ept) > > ... > >>> @@ -4990,6 +4996,12 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm, >>> kvm->arch.exception_payload_enabled = cap->args[0]; >>> r = 0; >>> break; >>> + case KVM_CAP_X86_BUS_LOCK_EXIT: >>> + if (!kvm_has_bus_lock_exit) >>> + return -EINVAL; >> >> ... because userspace can check for -EINVAL when enabling the cap. Or we >> can return e.g. -EOPNOTSUPP here. I don't have a strong opinion on the matter.. >> >>> + kvm->arch.bus_lock_exit = cap->args[0]; > > Assuming we even want to make this per-VM, I think it'd make sense to make > args[0] a bit mask, e.g. to provide "off" and "exit" (this behavior) while > allowing for future modes, e.g. log-only.
Good idea, will do it in next version.
>>> + r = 0; >>> + break; >>> default: >>> r = -EINVAL; >>> break; >>> @@ -7732,12 +7744,23 @@ static void post_kvm_run_save(struct kvm_vcpu *vcpu)
| |