diff mbox series

[06/21] KVM: TDX: Add accessors VMX VMCS helpers

Message ID 20240904030751.117579-7-rick.p.edgecombe@intel.com (mailing list archive)
State New, archived
Headers show
Series TDX MMU Part 2 | expand

Commit Message

Edgecombe, Rick P Sept. 4, 2024, 3:07 a.m. UTC 
From: Isaku Yamahata <isaku.yamahata@intel.com>

TDX defines SEAMCALL APIs to access TDX control structures corresponding to
the VMX VMCS.  Introduce helper accessors to hide its SEAMCALL ABI details.

Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
TDX MMU part 2 v1:
 - Update for the wrapper functions for SEAMCALLs. (Sean)
 - Eliminate kvm_mmu_free_private_spt() and open code it.
 - Fix bisectability issues in headers (Kai)
 - Updates from seamcall overhaul (Kai)

v19:
 - deleted unnecessary stub functions,
   tdvps_state_non_arch_check() and tdvps_management_check().
---
 arch/x86/kvm/vmx/tdx.h | 87 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 87 insertions(+)

Comments

Paolo Bonzini Sept. 9, 2024, 2:19 p.m. UTC | #1 
On 9/4/24 05:07, Rick Edgecombe wrote:
> +static __always_inline void td_##lclass##_clearbit##bits(struct vcpu_tdx *tdx,	\
> +							 u32 field, u64 bit)	\
> +{										\
> +	u64 err;								\
> +										\
> +	tdvps_##lclass##_check(field, bits);					\
> +	err = tdh_vp_wr(tdx, TDVPS_##uclass(field), 0, bit);			\
> +	if (KVM_BUG_ON(err, tdx->vcpu.kvm))					\
> +		pr_err("TDH_VP_WR["#uclass".0x%x] &= ~0x%llx failed: 0x%llx\n",	\
> +		       field, bit,  err);					\

Maybe a bit large when inlined?  Maybe

	if (unlikely(err))
		tdh_vp_wr_failed(tdx, field, bit, err);

and add tdh_vp_wr_failed to tdx.c.

Paolo
Edgecombe, Rick P Sept. 9, 2024, 9:29 p.m. UTC | #2 
On Mon, 2024-09-09 at 16:19 +0200, Paolo Bonzini wrote:
> On 9/4/24 05:07, Rick Edgecombe wrote:
> > +static __always_inline void td_##lclass##_clearbit##bits(struct vcpu_tdx
> > *tdx, \
> > +                                                        u32 field, u64
> > bit)    \
> > +{                                                                          
> >     \
> > +       u64
> > err;                                                                \
> > +                                                                           
> >     \
> > +       tdvps_##lclass##_check(field,
> > bits);                                    \
> > +       err = tdh_vp_wr(tdx, TDVPS_##uclass(field), 0,
> > bit);                    \
> > +       if (KVM_BUG_ON(err, tdx-
> > >vcpu.kvm))                                     \
> > +               pr_err("TDH_VP_WR["#uclass".0x%x] &= ~0x%llx failed:
> > 0x%llx\n", \
> > +                      field, bit, 
> > err);                                       \
> 
> Maybe a bit large when inlined?  Maybe
> 
>         if (unlikely(err))
>                 tdh_vp_wr_failed(tdx, field, bit, err);
> 
> and add tdh_vp_wr_failed to tdx.c.

There is a tiny bit of difference between the messages:
pr_err("TDH_VP_WR["#uclass".0x%x] = 0x%llx failed: 0x%llx\n", ...
pr_err("TDH_VP_WR["#uclass".0x%x] |= 0x%llx failed: 0x%llx\n", ...
pr_err("TDH_VP_WR["#uclass".0x%x] &= ~0x%llx failed: 0x%llx\n", ...

We can parameterize that part of the message, but it gets a bit tortured. Or
just lose that bit of detail. We can take a look. Thanks.
Paolo Bonzini Sept. 10, 2024, 10:48 a.m. UTC | #3 
On 9/9/24 23:29, Edgecombe, Rick P wrote:
>> Maybe a bit large when inlined?  Maybe
>>
>>          if (unlikely(err))
>>                  tdh_vp_wr_failed(tdx, field, bit, err);
>>
>> and add tdh_vp_wr_failed to tdx.c.
> There is a tiny bit of difference between the messages:
> pr_err("TDH_VP_WR["#uclass".0x%x] = 0x%llx failed: 0x%llx\n", ...
> pr_err("TDH_VP_WR["#uclass".0x%x] |= 0x%llx failed: 0x%llx\n", ...
> pr_err("TDH_VP_WR["#uclass".0x%x] &= ~0x%llx failed: 0x%llx\n", ...
> 
> We can parameterize that part of the message, but it gets a bit tortured. Or
> just lose that bit of detail. We can take a look. Thanks.

Yes, you can:

1) have three different functions for the failure

2) leave out the value part

3) pass the mask as well to tdh_vp_wr_failed() and use it to deduce the 
=/|=/&= part, like

	if (!~mask)
		op = "=";
	else if (!value)
		op = "&= ~", value = mask;
	else if (value == mask)
		op = "|=";
	else
		op = "??, value = ";

Paolo
diff mbox series

Patch

diff --git a/arch/x86/kvm/vmx/tdx.h b/arch/x86/kvm/vmx/tdx.h
index 7eeb54fbcae1..66540c57ed61 100644
--- a/arch/x86/kvm/vmx/tdx.h
+++ b/arch/x86/kvm/vmx/tdx.h
@@ -76,6 +76,93 @@  static __always_inline struct vcpu_tdx *to_tdx(struct kvm_vcpu *vcpu)
  */
 #include "tdx_ops.h"
 
+static __always_inline void tdvps_vmcs_check(u32 field, u8 bits)
+{
+#define VMCS_ENC_ACCESS_TYPE_MASK	0x1UL
+#define VMCS_ENC_ACCESS_TYPE_FULL	0x0UL
+#define VMCS_ENC_ACCESS_TYPE_HIGH	0x1UL
+#define VMCS_ENC_ACCESS_TYPE(field)	((field) & VMCS_ENC_ACCESS_TYPE_MASK)
+
+	/* TDX is 64bit only.  HIGH field isn't supported. */
+	BUILD_BUG_ON_MSG(__builtin_constant_p(field) &&
+			 VMCS_ENC_ACCESS_TYPE(field) == VMCS_ENC_ACCESS_TYPE_HIGH,
+			 "Read/Write to TD VMCS *_HIGH fields not supported");
+
+	BUILD_BUG_ON(bits != 16 && bits != 32 && bits != 64);
+
+#define VMCS_ENC_WIDTH_MASK	GENMASK(14, 13)
+#define VMCS_ENC_WIDTH_16BIT	(0UL << 13)
+#define VMCS_ENC_WIDTH_64BIT	(1UL << 13)
+#define VMCS_ENC_WIDTH_32BIT	(2UL << 13)
+#define VMCS_ENC_WIDTH_NATURAL	(3UL << 13)
+#define VMCS_ENC_WIDTH(field)	((field) & VMCS_ENC_WIDTH_MASK)
+
+	/* TDX is 64bit only.  i.e. natural width = 64bit. */
+	BUILD_BUG_ON_MSG(bits != 64 && __builtin_constant_p(field) &&
+			 (VMCS_ENC_WIDTH(field) == VMCS_ENC_WIDTH_64BIT ||
+			  VMCS_ENC_WIDTH(field) == VMCS_ENC_WIDTH_NATURAL),
+			 "Invalid TD VMCS access for 64-bit field");
+	BUILD_BUG_ON_MSG(bits != 32 && __builtin_constant_p(field) &&
+			 VMCS_ENC_WIDTH(field) == VMCS_ENC_WIDTH_32BIT,
+			 "Invalid TD VMCS access for 32-bit field");
+	BUILD_BUG_ON_MSG(bits != 16 && __builtin_constant_p(field) &&
+			 VMCS_ENC_WIDTH(field) == VMCS_ENC_WIDTH_16BIT,
+			 "Invalid TD VMCS access for 16-bit field");
+}
+
+#define TDX_BUILD_TDVPS_ACCESSORS(bits, uclass, lclass)				\
+static __always_inline u##bits td_##lclass##_read##bits(struct vcpu_tdx *tdx,	\
+							u32 field)		\
+{										\
+	u64 err, data;								\
+										\
+	tdvps_##lclass##_check(field, bits);					\
+	err = tdh_vp_rd(tdx, TDVPS_##uclass(field), &data);			\
+	if (KVM_BUG_ON(err, tdx->vcpu.kvm)) {					\
+		pr_err("TDH_VP_RD["#uclass".0x%x] failed: 0x%llx\n",		\
+		       field, err);						\
+		return 0;							\
+	}									\
+	return (u##bits)data;							\
+}										\
+static __always_inline void td_##lclass##_write##bits(struct vcpu_tdx *tdx,	\
+						      u32 field, u##bits val)	\
+{										\
+	u64 err;								\
+										\
+	tdvps_##lclass##_check(field, bits);					\
+	err = tdh_vp_wr(tdx, TDVPS_##uclass(field), val,			\
+		      GENMASK_ULL(bits - 1, 0));				\
+	if (KVM_BUG_ON(err, tdx->vcpu.kvm))					\
+		pr_err("TDH_VP_WR["#uclass".0x%x] = 0x%llx failed: 0x%llx\n",	\
+		       field, (u64)val, err);					\
+}										\
+static __always_inline void td_##lclass##_setbit##bits(struct vcpu_tdx *tdx,	\
+						       u32 field, u64 bit)	\
+{										\
+	u64 err;								\
+										\
+	tdvps_##lclass##_check(field, bits);					\
+	err = tdh_vp_wr(tdx, TDVPS_##uclass(field), bit, bit);			\
+	if (KVM_BUG_ON(err, tdx->vcpu.kvm))					\
+		pr_err("TDH_VP_WR["#uclass".0x%x] |= 0x%llx failed: 0x%llx\n",	\
+		       field, bit, err);					\
+}										\
+static __always_inline void td_##lclass##_clearbit##bits(struct vcpu_tdx *tdx,	\
+							 u32 field, u64 bit)	\
+{										\
+	u64 err;								\
+										\
+	tdvps_##lclass##_check(field, bits);					\
+	err = tdh_vp_wr(tdx, TDVPS_##uclass(field), 0, bit);			\
+	if (KVM_BUG_ON(err, tdx->vcpu.kvm))					\
+		pr_err("TDH_VP_WR["#uclass".0x%x] &= ~0x%llx failed: 0x%llx\n",	\
+		       field, bit,  err);					\
+}
+
+TDX_BUILD_TDVPS_ACCESSORS(16, VMCS, vmcs);
+TDX_BUILD_TDVPS_ACCESSORS(32, VMCS, vmcs);
+TDX_BUILD_TDVPS_ACCESSORS(64, VMCS, vmcs);
 #else
 static inline void tdx_bringup(void) {}
 static inline void tdx_cleanup(void) {}