Re: [PATCH] crypto: ccp - Have it depend on AMD_IOMMU

[Borislav Petkov]

On Fri, Feb 09, 2024 at 11:51:37AM -0600, Tom Lendacky wrote:
> Or should the ifdef around amd_iommu_snp_disable() in
> include/linux/amd-iommu.h instead be:
> 
> #if defined(CONFIG_AMD_IOMMU) && defined(CONFIG_KVM_AMD_SEV)

Pretty much, except the thunk should say that it is the thunk that gets
called:

#if defined(CONFIG_AMD_IOMMU) && defined(CONFIG_KVM_AMD_SEV)
int amd_iommu_snp_disable(void);
#else
static inline int amd_iommu_snp_disable(void) { return -ENODEV; }
#endif

and return -ENODEV to denote that so that the caller doesn't wonder
what's going on.

> I think this would be better in case these should be referenced
> elsewhere in the future.

However, looking at all that code, its design still looks iffy to me.

iommu_page_make_shared() and amd_iommu_snp_disable() is functionality
which the *IOMMU* provides in order to deal with SEV* guests. So that
functionality should be behind

CONFIG_AMD_IOMMU_SEV

or so, on which all its users depend or select. I'd prefer depend tho.

Putting IOMMU facilities around a KVM Kconfig symbol CONFIG_KVM_AMD_SEV
is simply bad design. There are providers of functionality which is
behind Kconfig symbols and users which depend on those. The current
thing is a hack IMNSVHO.

Thx.

-- 
Regards/Gruss,
    Boris.

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