Re: [PATCH v19 030/130] KVM: TDX: Add helper functions to print TDX SEAMCALL error

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 26/02/2024 9:25 pm, isaku.yamahata@xxxxxxxxx wrote:
From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>

Add helper functions to print out errors from the TDX module in a uniform
manner.

Likely we need more information here.  See below.


Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
Reviewed-by: Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx>
Reviewed-by: Yuan Yao <yuan.yao@xxxxxxxxx>
---
v19:
- dropped unnecessary include <asm/tdx.h>

v18:
- Added Reviewed-by Binbin.

The tag doesn't show in the SoB chain.


Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
---

[...]

+void pr_tdx_error(u64 op, u64 error_code, const struct tdx_module_args *out)
+{
+	if (!out) {
+		pr_err_ratelimited("SEAMCALL (0x%016llx) failed: 0x%016llx\n",
+				   op, error_code);
+		return;
+	}

I think this is the reason you still want the @out in tdx_seamcall()?

But I am not sure either -- even if you want to have @out *here* -- why cannot you pass a NULL explicitly when you *know* the concerned SEAMCALL doesn't have a valid output?

+
+#define MSG	\
+	"SEAMCALL (0x%016llx) failed: 0x%016llx RCX 0x%016llx RDX 0x%016llx R8 0x%016llx R9 0x%016llx R10 0x%016llx R11 0x%016llx\n"
+	pr_err_ratelimited(MSG, op, error_code, out->rcx, out->rdx, out->r8,
+			   out->r9, out->r10, out->r11);
+}

Besides the regs that you are printing, there are more regs (R12-R15, RDI, RSI) in the structure.

It's not clear why you only print some, but not all.

AFAICT the VP.ENTER SEAMCALL can have all regs as valid output?

Anyway, that being said, you might need to put more text in changelog/comment to make this patch (at least more) reviewable.




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux