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)