On 2023/4/11 2:35, Sean Christopherson wrote:
On Fri, Mar 31, 2023, Robert Hoo wrote:
Sean Christopherson <seanjc@xxxxxxxxxx> 于2023年3月16日周四 00:36写道：
Sorry I don't follow you.
My point is that, given it has passed kvm_is_valid_cr4() (in kvm_set_cr4()),
we can assert boot_cpu_has(X86_FEATURE_UMIP) and vmx_umip_emulated() must be
at least one true.
This assertion is wrong for the case where guest.CR4.UMIP=0. The below code is
not guarded with a check on guest.CR4.UMIP. If the vmx_umip_emulated() check goes
away and guest.CR4.UMIP=0, KVM will attempt to write secondary controls.
Sorry still don't follow you. Do you mean in nested case? the "guest"
above is L1?
Please take the time to walk through the code with possible inputs/scenarios before
asking these types of questions, e.g. if necessary use a whiteboard, pen+paper, etc.
I'm happy to explain subtleties and add answer specific questions, but as evidenced
by my delayed response, I simply do not have the bandwidth to answer questions where
the answer is literally a trace-through of a small, fully contained section of code.
Sure, fully understand your busyness, thanks for still providing detailed
explanation. Sorry for the annoyance.
Given that you've merged host UMIP cap check into vmx_umip_emulated(), I
might have not tangled in this point.
We can close this thread of patch set now.
if (!boot_cpu_has(X86_FEATURE_UMIP)) { <= evaluates true when UMIP is NOT supported
if (cr4 & X86_CR4_UMIP) { <= evaluates false when guest.CR4.UMIP == 0
secondary_exec_controls_setbit(vmx, SECONDARY_EXEC_DESC);
hw_cr4 &= ~X86_CR4_UMIP;
} else if (!is_guest_mode(vcpu) || <= evalutes true when L2 is NOT active
!nested_cpu_has2(get_vmcs12(vcpu), SECONDARY_EXEC_DESC)) {
secondary_exec_controls_clearbit(vmx, SECONDARY_EXEC_DESC); <= KVM "blindly" writes secondary controls
}
}
