On 10/15/2024 11:19 PM, Sean Christopherson wrote:
> On Fri, Oct 04, 2024, Manali Shukla wrote:
...
>>
>> +static int bus_lock_exit(struct kvm_vcpu *vcpu)
>> +{
>> + struct vcpu_svm *svm = to_svm(vcpu);
>> +
>> + vcpu->run->exit_reason = KVM_EXIT_X86_BUS_LOCK;
>> + vcpu->run->flags |= KVM_RUN_X86_BUS_LOCK;
>> +
>> + /*
>> + * Reload the counter with value greater than '0'.
>
> The value quite obviously must be exactly '1', not simply greater than '0. I
> also
> think this is the wrong place to set the counter. Rather than set the
> counter at
> the time of exit, KVM should implement a vcpu->arch.complete_userspace_io
> callback
> and set the counter to '1' if and only if RIP (or LIP, but I have no
> objection to
> keeping things simple) is unchanged. It's a bit of extra complexity, but it
> will
> make it super obvious why KVM is setting the counter to '1'. And, if
> userspace
> wants to stuff state and move past the instruction, e.g. by emulating the
> guilty
> instruction, then KVM won't unnecessarily allow a bus lock in the guest.
>
> And then the comment can be:
>
> /*
> * If userspace has NOT change RIP, then KVM's ABI is to let the guest
> * execute the bus-locking instruction. Set the bus lock counter to '1'
> * to effectively step past the bus lock.
> */
>
The bus lock threshold intercept feature is available for SEV-ES and SEV-SNP
guests too. The rip where the bus lock exit occurred, is not available in
bus_lock_exit handler for SEV-ES and SEV-SNP guests, so the above-mentioned
solution won't work with SEV-ES and SEV-SNP guests.
I would propose to add the above-mentioned solution only for normal and SEV
guests
and unconditionally reloading of bus_lock_counter to 1 in complete_userspace_io
for SEV-ES and SEV-SNP guests.
Any thoughts ?
>> + * The bus lock exit on SVM happens with RIP pointing to the guilty
>> + * instruction. So, reloading the value of bus_lock_counter to '0'
>> + * results into generating continuous bus lock exits.
>> + */
>> + svm->vmcb->control.bus_lock_counter = 1;
>> +
>> + return 0;
>> +}
>> +
>> static int (*const svm_exit_handlers[])(struct kvm_vcpu *vcpu) = {
>> [SVM_EXIT_READ_CR0] = cr_interception,
>> [SVM_EXIT_READ_CR3] = cr_interception,
>> @@ -3353,6 +3374,7 @@ static int (*const svm_exit_handlers[])(struct
>> kvm_vcpu *vcpu) = {
>> [SVM_EXIT_CR4_WRITE_TRAP] = cr_trap,
>> [SVM_EXIT_CR8_WRITE_TRAP] = cr_trap,
>> [SVM_EXIT_INVPCID] = invpcid_interception,
>> + [SVM_EXIT_BUS_LOCK] = bus_lock_exit,
>> [SVM_EXIT_NPF] = npf_interception,
>> [SVM_EXIT_RSM] = rsm_interception,
>> [SVM_EXIT_AVIC_INCOMPLETE_IPI] =
>> avic_incomplete_ipi_interception,
>> @@ -5227,6 +5249,11 @@ static __init void svm_set_cpu_caps(void)
>> kvm_cpu_cap_set(X86_FEATURE_SVME_ADDR_CHK);
>> }
>>
>> + if (cpu_feature_enabled(X86_FEATURE_BUS_LOCK_THRESHOLD)) {
>> + pr_info("Bus Lock Threashold supported\n");
>> + kvm_caps.has_bus_lock_exit = true;
>> + }
>> +
>> /* CPUID 0x80000008 */
>> if (boot_cpu_has(X86_FEATURE_LS_CFG_SSBD) ||
>> boot_cpu_has(X86_FEATURE_AMD_SSBD))
>> --
>> 2.34.1
>>
-Manali
