On 2/27/2025 9:41 PM, Sean Christopherson wrote: > On Thu, Feb 27, 2025, Manali Shukla wrote: >> On 2/27/2025 8:02 PM, Sean Christopherson wrote: >>> 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? >> >> The idle-HLT intercept remains '1' for the L2 guest. Now, when L2 executes HLT and there >> is no pending event available, it will still do idle-HLT exit, although Idle HLT >> was never explicitly enabled on L2 guest. > > Yes, but why is that a problem? L1 doesn't want to intercept HLT, so KVM isn't > violating the architecture by not intercepting HLT. What I initially understood was, since we're not showing the Idle HLT Intercept CPUID feature bit to L1, to fully turn off Idle HLT support for nested guests, we should completely disable the Idle HLT intercept for L2. So, when L2 runs HLT and L1 doesn't intercept it, it should trigger an HLT exit to L0, just like it does now. But after thinking a bit more, now I get what you are saying. KVM has enabled the IDLE HLT intercept for the L1 guest. So, when the HLT intercept is turned off for the L2 guest and L2 runs the HLT instruction, it does an idle HLT exit to KVM. That way, KVM is working as per the IDLE HLT design. And as you explained , the existing buggy behavior is also avoided with idle halt intercept being kept enabled for L2 to some extent. -Manali
