On Thu, Feb 27, 2025, Manali Shukla wrote:
On 2/26/2025 3:37 AM, Sean Christopherson wrote:
@@ -5225,6 +5230,8 @@ static __init void svm_set_cpu_caps(void) if (vnmi) kvm_cpu_cap_set(X86_FEATURE_VNMI);
kvm_cpu_cap_check_and_set(X86_FEATURE_IDLE_HLT);I am 99% certain this is wrong. Or at the very least, severly lacking an explanation of why it's correct. If L1 enables Idle HLT but not HLT interception, then it is KVM's responsibility to NOT exit to L1 if there is a pending V_IRQ or V_NMI.
Yeah, it's buggy. But, it's buggy in part because *existing* KVM support is buggy. If L1 disables HLT exiting, but it's enabled in KVM, then KVM will run L2 with HLT exiting and so it becomes KVM's responsibility to check for valid L2 wake events prior to scheduling out the vCPU if L2 executes HLT. E.g. nVMX handles this by reading vmcs02.GUEST_INTERRUPT_STATUS.RVI as part of vmx_has_nested_events(). I don't see the equivalent in nSVM.
Amusingly, that means Idle HLT is actually a bug fix to some extent. E.g. if there is a pending V_IRQ/V_NMI in vmcb02, then running with Idle HLT will naturally do the right thing, i.e. not hang the vCPU.
Anyways, for now, I think the easiest and best option is to simply skip full nested support for the moment.
Got it, I see the issue you're talking about. I'll need to look into it a bit more to fully understand it. So yeah, we can hold off on full nested support for idle HLT intercept for now.
Since we are planning to disable Idle HLT support on nested guests, should we do something like this ?
@@ -167,10 +167,15 @@ void recalc_intercepts(struct vcpu_svm *svm) if (!nested_svm_l2_tlb_flush_enabled(&svm->vcpu)) vmcb_clr_intercept(c, INTERCEPT_VMMCALL);
if (!guest_cpu_cap_has(&svm->vcpu, X86_FEATURE_IDLE_HLT))vmcb_clr_intercept(c, INTERCEPT_IDLE_HLT);When recalc_intercepts copies the intercept values from vmc01 to vmcb02, it also copies the IDLE HLT intercept bit, which is set to 1 in vmcb01. Normally, this isn't a problem because the HLT intercept takes priority when it's on. But if the HLT intercept gets turned off for some reason, the IDLE HLT intercept will stay on, which is not what we want.
Why don't we want that?