On Wed, 2025-06-04 at 21:14 +0800, Gao, Chao wrote: > > Given there will be a dedicated seamldr.c, I don't quite like having > > seamldr_prerr() in "tdx.h" and tdx.c. > > > > Now for all SEAMCALLs used by KVM, we have a dedicated wrapper implemented > > in tdx.c and exported for KVM to use. I think we can move seamcall*() out > > of <asm/tdx.h> to TDX host local since no other kernel code except the TDX > > host core is supposed to use seamcall*(). > > > > This also cleans up <asm/tdx.h> a little bit, which in general makes code > > cleaner IMHO. > > > > E.g., how about we do below patch, and then you can do changes to support > > P-SEAMLDR on top of it? > > looks good to me. I'd like to incorporate this patch into my series if > Kirill and Dave have no objections to this cleanup. I assume > seamldr_prerr() can be added to the new seamcall.h > > Thanks for this suggestion. Seems we both think this is a good cleanup. My TDX host kexec series also conflicts with this so I think I can send this patch out first to see how things will go. At the meantime, yeah please carry it in your series. > > > diff --git a/arch/x86/virt/vmx/tdx/seamcall.h b/arch/x86/virt/vmx/tdx/seamcall.h > > new file mode 100644 > > index 000000000000..54922f7bda3a > > --- /dev/null > > +++ b/arch/x86/virt/vmx/tdx/seamcall.h > > @@ -0,0 +1,71 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* Copyright (C) 2025 Intel Corporation */ > > +#include <asm/tdx.h> > > If seamcall.h is intended to provide low-level helpers, including > <asm/tdx.h>, which is meant to offer high-level APIs for other components > such as KVM, seems a bit odd to me. But I suppose we can live with this. Kinda agree, I can remove it and do: struct tdx_module_args; explicitly. But we also need to include <asm/archrandom.h> etc, so I think I will just leave it as-is until other people coming out to complain.