Re: [RFC PATCH v2 15/18] KVM: TDX: Combine KVM_BUG_ON + pr_tdx_error() into TDX_BUG_ON()

[Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx>]

On 8/29/2025 10:19 PM, Sean Christopherson wrote:
> On Fri, Aug 29, 2025, Binbin Wu wrote:
> > On 8/29/2025 8:06 AM, Sean Christopherson wrote:
> > > Add TDX_BUG_ON() macros (with varying numbers of arguments) to deduplicate
> > > the myriad flows that do KVM_BUG_ON()/WARN_ON_ONCE() followed by a call to
> > > pr_tdx_error().  In addition to reducing boilerplate copy+paste code, this
> > > also helps ensure that KVM provides consistent handling of SEAMCALL errors.
> > >
> > > Opportunistically convert a handful of bare WARN_ON_ONCE() paths to the
> > > equivalent of KVM_BUG_ON(), i.e. have them terminate the VM.  If a SEAMCALL
> > > error is fatal enough to WARN on, it's fatal enough to terminate the TD.
> > >
> > > Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> > > ---
> > >    arch/x86/kvm/vmx/tdx.c | 114 +++++++++++++++++------------------------
> > >    1 file changed, 47 insertions(+), 67 deletions(-)
> > >
> > > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> > > index aa6d88629dae..df9b4496cd01 100644
> > > --- a/arch/x86/kvm/vmx/tdx.c
> > > +++ b/arch/x86/kvm/vmx/tdx.c
> > > @@ -24,20 +24,32 @@
> > >    #undef pr_fmt
> > >    #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > -#define pr_tdx_error(__fn, __err)	\
> > > -	pr_err_ratelimited("SEAMCALL %s failed: 0x%llx\n", #__fn, __err)
> > > +#define __TDX_BUG_ON(__err, __f, __kvm, __fmt, __args...)			\
> > > +({										\
> > > +	struct kvm *_kvm = (__kvm);						\
> > > +	bool __ret = !!(__err);							\
> > > +										\
> > > +	if (WARN_ON_ONCE(__ret && (!_kvm || !_kvm->vm_bugged))) {		\
> > > +		if (_kvm)							\
> > > +			kvm_vm_bugged(_kvm);					\
> > > +		pr_err_ratelimited("SEAMCALL " __f " failed: 0x%llx" __fmt "\n",\
> > > +				   __err,  __args);				\
> > > +	}									\
> > > +	unlikely(__ret);							\
> > > +})
> > > -#define __pr_tdx_error_N(__fn_str, __err, __fmt, ...)		\
> > > -	pr_err_ratelimited("SEAMCALL " __fn_str " failed: 0x%llx, " __fmt,  __err,  __VA_ARGS__)
> > > +#define TDX_BUG_ON(__err, __fn, __kvm)				\
> > > +	__TDX_BUG_ON(__err, #__fn, __kvm, "%s", "")
> > > -#define pr_tdx_error_1(__fn, __err, __rcx)		\
> > > -	__pr_tdx_error_N(#__fn, __err, "rcx 0x%llx\n", __rcx)
> > > +#define TDX_BUG_ON_1(__err, __fn, __rcx, __kvm)			\
> > > +	__TDX_BUG_ON(__err, #__fn, __kvm, ", rcx 0x%llx", __rcx)
> > > -#define pr_tdx_error_2(__fn, __err, __rcx, __rdx)	\
> > > -	__pr_tdx_error_N(#__fn, __err, "rcx 0x%llx, rdx 0x%llx\n", __rcx, __rdx)
> > > +#define TDX_BUG_ON_2(__err, __fn, __rcx, __rdx, __kvm)		\
> > > +	__TDX_BUG_ON(__err, #__fn, __kvm, ", rcx 0x%llx, rdx 0x%llx", __rcx, __rdx)
> > > +
> > > +#define TDX_BUG_ON_3(__err, __fn, __rcx, __rdx, __r8, __kvm)	\
> > > +	__TDX_BUG_ON(__err, #__fn, __kvm, ", rcx 0x%llx, rdx 0x%llx, r8 0x%llx", __rcx, __rdx, __r8)
> > > -#define pr_tdx_error_3(__fn, __err, __rcx, __rdx, __r8)	\
> > > -	__pr_tdx_error_N(#__fn, __err, "rcx 0x%llx, rdx 0x%llx, r8 0x%llx\n", __rcx, __rdx, __r8)
> > I thought you would use the format Rick proposed in
> > https://lore.kernel.org/all/9e55a0e767317d20fc45575c4ed6dafa863e1ca0.camel@xxxxxxxxx/
> >      #define TDX_BUG_ON_2(__err, __fn, arg1, arg2, __kvm)        \
> >          __TDX_BUG_ON(__err, #__fn, __kvm, ", " #arg1 " 0x%llx, " #arg2 "
> >      0x%llx", arg1, arg2)
> >
> >      so you get: entry: 0x00 level:0xF00
> >
> > No?
> Ya, see the next patch :-)

Oh, sorry for the noise.
>