[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <6c7ee971-d2b1-461f-900e-d343678ec989@linux.intel.com>
Date: Mon, 1 Sep 2025 09:46:08 +0800
From: Binbin Wu <binbin.wu@...ux.intel.com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org,
linux-kernel@...r.kernel.org, Ira Weiny <ira.weiny@...el.com>,
Kai Huang <kai.huang@...el.com>, Michael Roth <michael.roth@....com>,
Yan Zhao <yan.y.zhao@...el.com>, Vishal Annapurve <vannapurve@...gle.com>,
Rick Edgecombe <rick.p.edgecombe@...el.com>,
Ackerley Tng <ackerleytng@...gle.com>
Subject: Re: [RFC PATCH v2 15/18] KVM: TDX: Combine KVM_BUG_ON +
pr_tdx_error() into TDX_BUG_ON()
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@...gle.com>
>>> ---
>>> 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@intel.com/
>> #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.
>
Powered by blists - more mailing lists