On 12/16/2024 9:08 AM, Binbin Wu wrote:
On 12/13/2024 5:32 PM, Xiaoyao Li wrote:
On 12/1/2024 11:53 AM, Binbin Wu wrote:
[...]
+
+static int tdx_map_gpa(struct kvm_vcpu *vcpu)
+{
+ struct vcpu_tdx * tdx = to_tdx(vcpu);
+ u64 gpa = tdvmcall_a0_read(vcpu);
We can use kvm_r12_read() directly, which is more intuitive. And we
can drop the MACRO for a0/a1/a2/a3 accessors in patch 022.
I am neutral about it.
a0, a1, a2, a3, are the name convention for KVM's hypercall. It makes
sense when serving as the parameters to __kvm_emulate_hypercall().
However, now __kvm_emulate_hypercall() is made to a MACRO and we don't
need the temp variable like a0 = kvm_xx_read().
For TDVMCALL leafs other than normal KVM hypercalls, they are all TDX
specific and defined in TDX GHCI spec, a0/a1/a2/a3 makes no sense for them.
+ u64 size = tdvmcall_a1_read(vcpu);
+ u64 ret;
+
+ /*
+ * Converting TDVMCALL_MAP_GPA to KVM_HC_MAP_GPA_RANGE requires
+ * userspace to enable KVM_CAP_EXIT_HYPERCALL with
KVM_HC_MAP_GPA_RANGE
+ * bit set. If not, the error code is not defined in GHCI for
TDX, use
+ * TDVMCALL_STATUS_INVALID_OPERAND for this case.
+ */
+ if (!user_exit_on_hypercall(vcpu->kvm, KVM_HC_MAP_GPA_RANGE)) {
+ ret = TDVMCALL_STATUS_INVALID_OPERAND;
+ goto error;
+ }
+
+ if (gpa + size <= gpa || !kvm_vcpu_is_legal_gpa(vcpu, gpa) ||
+ !kvm_vcpu_is_legal_gpa(vcpu, gpa + size -1) ||
+ (vt_is_tdx_private_gpa(vcpu->kvm, gpa) !=
+ vt_is_tdx_private_gpa(vcpu->kvm, gpa + size -1))) {
+ ret = TDVMCALL_STATUS_INVALID_OPERAND;
+ goto error;
+ }
+
+ if (!PAGE_ALIGNED(gpa) || !PAGE_ALIGNED(size)) {
+ ret = TDVMCALL_STATUS_ALIGN_ERROR;
+ goto error;
+ }
+
+ tdx->map_gpa_end = gpa + size;
+ tdx->map_gpa_next = gpa;
+
+ __tdx_map_gpa(tdx);
+ /* Forward request to userspace. */
+ return 0;
Maybe let __tdx_map_gpa() return a int to decide whether it needs to
return to userspace and
return __tdx_map_gpa(tdx);
?
To save one line of code and the comment?
No. Just I found most of the cases that need to exit to usespace, comes
with "return 0" after setting up the run->exit_reason and run->(fields).
Because MapGPA always goes to userspace, I don't want to make a function
return
a int that is a fixed value.
And if the multiple comments bother you, I think the comments can be
removed.
+
+error:
+ tdvmcall_set_return_code(vcpu, ret);
+ kvm_r11_write(vcpu, gpa);
+ return 1;
+}
+
static int handle_tdvmcall(struct kvm_vcpu *vcpu)
{
if (tdvmcall_exit_type(vcpu))
return tdx_emulate_vmcall(vcpu);
switch (tdvmcall_leaf(vcpu)) {
+ case TDVMCALL_MAP_GPA:
+ return tdx_map_gpa(vcpu);
default:
break;
}
diff --git a/arch/x86/kvm/vmx/tdx.h b/arch/x86/kvm/vmx/tdx.h
index 1abc94b046a0..bfae70887695 100644
--- a/arch/x86/kvm/vmx/tdx.h
+++ b/arch/x86/kvm/vmx/tdx.h
@@ -71,6 +71,9 @@ struct vcpu_tdx {
enum tdx_prepare_switch_state prep_switch_state;
u64 msr_host_kernel_gs_base;
+
+ u64 map_gpa_next;
+ u64 map_gpa_end;
};
void tdh_vp_rd_failed(struct vcpu_tdx *tdx, char *uclass, u32
field, u64 err);
