Re: [PATCH v2 5/8] KVM: TDX: Handle TDG.VP.VMCALL<MapGPA>

[Binbin Wu <binbin.wu@linux.intel.com>]

On 2/11/2025 2:54 PM, Yan Zhao wrote:
> On Tue, Feb 11, 2025 at 10:54:39AM +0800, Binbin Wu wrote:
>> +static int tdx_complete_vmcall_map_gpa(struct kvm_vcpu *vcpu)
>> +{
>> +	struct vcpu_tdx *tdx = to_tdx(vcpu);
>> +
>> +	if (vcpu->run->hypercall.ret) {
>> +		tdvmcall_set_return_code(vcpu, TDVMCALL_STATUS_INVALID_OPERAND);
>> +		tdx->vp_enter_args.r11 = tdx->map_gpa_next;
>> +		return 1;
>> +	}
>> +
>> +	tdx->map_gpa_next += TDX_MAP_GPA_MAX_LEN;
>> +	if (tdx->map_gpa_next >= tdx->map_gpa_end)
>> +		return 1;
>> +
>> +	/*
>> +	 * Stop processing the remaining part if there is pending interrupt.
>> +	 * Skip checking pending virtual interrupt (reflected by
>> +	 * TDX_VCPU_STATE_DETAILS_INTR_PENDING bit) to save a seamcall because
>> +	 * if guest disabled interrupt, it's OK not returning back to guest
>> +	 * due to non-NMI interrupt. Also it's rare to TDVMCALL_MAP_GPA
>> +	 * immediately after STI or MOV/POP SS.
>> +	 */
>> +	if (pi_has_pending_interrupt(vcpu) ||
>> +	    kvm_test_request(KVM_REQ_NMI, vcpu) || vcpu->arch.nmi_pending) {
> Should here also use "kvm_vcpu_has_events()" to replace
> "pi_has_pending_interrupt(vcpu) ||
>   kvm_test_request(KVM_REQ_NMI, vcpu) || vcpu->arch.nmi_pending" as Sean
> suggested at [1]?
>
> [1] https://lore.kernel.org/all/Z4rIGv4E7Jdmhl8P@google.com

For TDX guests, kvm_vcpu_has_events() will check pending virtual interrupt
via a SEAM call.  As noted in the comments, the check for pending virtual
interrupt is intentionally skipped to save the SEAM call. Additionally,
unnecessarily returning back to guest will has performance impact.

But according to the discussion thread above, it seems that Sean prioritized
code readability (i.e. reuse the common helper to make TDX code less special)
over performance considerations?

>
>> +		tdvmcall_set_return_code(vcpu, TDVMCALL_STATUS_RETRY);
>> +		tdx->vp_enter_args.r11 = tdx->map_gpa_next;
>> +		return 1;
>> +	}
>> +
>> +	__tdx_map_gpa(tdx);
>> +	return 0;
>> +}
>