[PATCH] x86: tdx: hide unused tdx_dump_mce_info()

[PATCH] x86: tdx: hide unused tdx_dump_mce_info()

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

When TDX is enabled but MCE is not, the tdx_dump_mce_info() function
fails to link:

ld.lld: error: undefined symbol: mce_is_memory_error
ld.lld: error: undefined symbol: mce_usable_address
>>> referenced by usercopy_64.c
>>>               vmlinux.o:(tdx_dump_mce_info)

In this configuration, there is also no caller for the function, so
avoid the problem by enclosing it in an #ifdef block.

Fixes: 45f31973967d ("x86/mce: Differentiate real hardware #MCs from TDX erratum ones")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/x86/virt/vmx/tdx/tdx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 13df68ef40b5..3af7a7e2d8d0 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1245,6 +1245,7 @@ int tdx_enable(void)
 }
 EXPORT_SYMBOL_GPL(tdx_enable);
 
+#ifdef CONFIG_X86_MCE
 static bool is_pamt_page(unsigned long phys)
 {
 	struct tdmr_info_list *tdmr_list = &tdx_tdmr_list;
@@ -1344,6 +1345,7 @@ const char *tdx_dump_mce_info(struct mce *m)
 
 	return "TDX private memory error. Possible kernel bug.";
 }
+#endif
 
 static __init int record_keyid_partitioning(u32 *tdx_keyid_start,
 					    u32 *nr_tdx_keyids)
-- 
2.39.2

Re: [PATCH] x86: tdx: hide unused tdx_dump_mce_info()

[Dave Hansen]

On 12/12/23 13:36, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> When TDX is enabled but MCE is not, the tdx_dump_mce_info() function
> fails to link:

Thanks for the report, Arnd.

The only way that TDX has to report integrity errors is an MCE.  I'm not
sure it even makes sense to have TDX support but not MCE support.  Maybe
we should just make TDX host support depend on MCE.

Re: [PATCH] x86: tdx: hide unused tdx_dump_mce_info()

[Borislav Petkov]

On Tue, Dec 12, 2023 at 01:42:09PM -0800, Dave Hansen wrote:
> On 12/12/23 13:36, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> > 
> > When TDX is enabled but MCE is not, the tdx_dump_mce_info() function
> > fails to link:
> 
> Thanks for the report, Arnd.
> 
> The only way that TDX has to report integrity errors is an MCE.  I'm not
> sure it even makes sense to have TDX support but not MCE support.  Maybe
> we should just make TDX host support depend on MCE.

... or provide an empty stub of tdx_dump_mce_info() for the !CONFIG_X86_MCE
case.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86: tdx: hide unused tdx_dump_mce_info()

[Kirill A. Shutemov]

On Tue, Dec 12, 2023 at 01:42:09PM -0800, Dave Hansen wrote:
> On 12/12/23 13:36, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> > 
> > When TDX is enabled but MCE is not, the tdx_dump_mce_info() function
> > fails to link:
> 
> Thanks for the report, Arnd.
> 
> The only way that TDX has to report integrity errors is an MCE.  I'm not
> sure it even makes sense to have TDX support but not MCE support.  Maybe
> we should just make TDX host support depend on MCE.

I agree. Silently ignore integrity errors is not good idea.

TDX module spec also supports it:

"The machine-check exception handler is expected to be implemented in the
VMM."

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCH] x86: tdx: hide unused tdx_dump_mce_info()

[Huang, Kai]

On Wed, 2023-12-13 at 15:02 +0300, Kirill A. Shutemov wrote:
> On Tue, Dec 12, 2023 at 01:42:09PM -0800, Dave Hansen wrote:
> > On 12/12/23 13:36, Arnd Bergmann wrote:
> > > From: Arnd Bergmann <arnd@arndb.de>
> > > 
> > > When TDX is enabled but MCE is not, the tdx_dump_mce_info() function
> > > fails to link:
> > 
> > Thanks for the report, Arnd.
> > 
> > The only way that TDX has to report integrity errors is an MCE.  I'm not
> > sure it even makes sense to have TDX support but not MCE support.  Maybe
> > we should just make TDX host support depend on MCE.
> 
> I agree. Silently ignore integrity errors is not good idea.
> 
> TDX module spec also supports it:
> 
> "The machine-check exception handler is expected to be implemented in the
> VMM."

I also agree.  Thanks.

Re: [PATCH] x86: tdx: hide unused tdx_dump_mce_info()

[Dave Hansen]

On 12/13/23 12:11, Huang, Kai wrote:
>> "The machine-check exception handler is expected to be implemented in the
>> VMM."
> I also agree.  Thanks.

Kai, could you please send a patch to fix this build problem that Arnd
reported?

Re: [PATCH] x86: tdx: hide unused tdx_dump_mce_info()

[Huang, Kai]

On Wed, 2023-12-13 at 12:13 -0800, Dave Hansen wrote:
> On 12/13/23 12:11, Huang, Kai wrote:
> > > "The machine-check exception handler is expected to be implemented in the
> > > VMM."
> > I also agree.  Thanks.
> 
> Kai, could you please send a patch to fix this build problem that Arnd
> reported?

Yeah will do asap.

Re: [PATCH] x86: tdx: hide unused tdx_dump_mce_info()

[Huang, Kai]

On Wed, 2023-12-13 at 12:13 -0800, Dave Hansen wrote:
> On 12/13/23 12:11, Huang, Kai wrote:
> > > "The machine-check exception handler is expected to be implemented in the
> > > VMM."
> > I also agree.  Thanks.
> 
> Kai, could you please send a patch to fix this build problem that Arnd
> reported?

Turns out INTEL_TDX_GUEST selects X86_MCE, I think we can also just select
X86_MCE for TDX host?

config INTEL_TDX_GUEST
        bool "Intel TDX (Trust Domain Extensions) - Guest Support"
        depends on X86_64 && CPU_SUP_INTEL
	...
        select X86_MCE
	

Re: [PATCH] x86: tdx: hide unused tdx_dump_mce_info()

[Huang, Kai]

On Wed, 2023-12-13 at 20:41 +0000, Huang, Kai wrote:
> On Wed, 2023-12-13 at 12:13 -0800, Dave Hansen wrote:
> > On 12/13/23 12:11, Huang, Kai wrote:
> > > > "The machine-check exception handler is expected to be implemented in the
> > > > VMM."
> > > I also agree.  Thanks.
> > 
> > Kai, could you please send a patch to fix this build problem that Arnd
> > reported?
> 
> Turns out INTEL_TDX_GUEST selects X86_MCE, I think we can also just select
> X86_MCE for TDX host?
> 
> config INTEL_TDX_GUEST
>         bool "Intel TDX (Trust Domain Extensions) - Guest Support"
>         depends on X86_64 && CPU_SUP_INTEL
> 	...
>         select X86_MCE
> 	

Turns out we have to use 'depend on' but not 'select' because with the latter I
got "mm/Kconfig:772:error: recursive dependency detected!".

I just sent out the fixing patch.  I did kernel built of multiple combinations
of TDX_GUEST/TDX_HOST/X86_MCE in the Kconfig, and they all passed.