Messages in this thread | | | Date | Thu, 21 Mar 2024 16:52:14 -0700 | From | Isaku Yamahata <> | Subject | Re: [PATCH v19 030/130] KVM: TDX: Add helper functions to print TDX SEAMCALL error |
| |
On Thu, Mar 21, 2024 at 12:09:57PM +1300, "Huang, Kai" <kai.huang@intel.com> wrote:
> > Does it make sense? > > > > void pr_tdx_error(u64 op, u64 error_code) > > { > > pr_err_ratelimited("SEAMCALL (0x%016llx) failed: 0x%016llx\n", > > op, error_code); > > } > > Should we also have a _ret version? > > void pr_seamcall_err(u64 op, u64 err) > { > /* A comment to explain why using the _ratelimited() version? */
Because KVM can hit successive seamcall erorrs e.g. during desutructing TD, (it's unintentional sometimes), ratelimited version is preferred as safe guard. For example, SEAMCALL on all or some LPs (TDH_MNG_KEY_FREEID) can fail at the same time. And the number of LPs can be hundreds.
> pr_err_ratelimited(...); > } > > void pr_seamcall_err_ret(u64 op, u64 err, struct tdx_module_args *arg) > { > pr_err_seamcall(op, err); > > pr_err_ratelimited(...); > } > > (Hmm... if you look at the tdx.c in TDX host, there's similar code there, > and again, it was a little bit annoying when I did that..) > > Again, if we just use seamcall_ret() for ALL SEAMCALLs except VP.ENTER, we > can simply have one..
What about this?
void pr_seamcall_err_ret(u64 op, u64 err, struct tdx_module_args *arg) { pr_err_ratelimited("SEAMCALL (0x%016llx) failed: 0x%016llx\n", op, error_code); if (arg) pr_err_ratelimited(...); }
> > void pr_tdx_sept_error(u64 op, u64 error_code, const union tdx_sept_entry *entry, > > const union tdx_sept_level_state *level_state) > > { > > #define MSG \ > > "SEAMCALL (0x%016llx) failed: 0x%016llx entry 0x%016llx level_state 0x%016llx\n" > > pr_err_ratelimited(MSG, op, error_code, entry->raw, level_state->raw); > > } > > A higher-level wrapper to print SEPT error is fine to me, but do it in a > separate patch.
Ok, Let's postpone custom version. -- Isaku Yamahata <isaku.yamahata@intel.com>
| |