Messages in this thread | | | Date | Tue, 19 Jun 2018 23:32:56 +0200 | From | Borislav Petkov <> | Subject | Re: [PATCH] Use 'imply' with SEV Kconfig CRYPTO dependencies |
| |
On Tue, Jun 19, 2018 at 03:46:40PM -0500, Brijesh Singh wrote: > In case if it was not clear, we don't have a standalone PSP driver. > The PSP support is provided by the CCP driver. If you look at config > changes I proposed then it says if PSP is available then we can support > SEV. But since PSP support is provided by the CCP driver hence we > need to have module dependency with CCP. So, we are using your former > expression in the dependency but have to extend it a bit more.
It doesn't matter whether the driver is standalong or not as long as you specifically have to express explicit dependency on:
depends CRYPTO_DEV_SP_PSP
Now that's the PSP component or whatever but as long as it has a separate Kconfig symbol, it is the only thing that is important here.
Now, KVM_AMD_SEV needs the functionality behind CRYPTO_DEV_SP_PSP. So far so good?
> We had discussion about this during our patch review process but lets > revisit again. CCP driver manages CCP and PSP devices. Ideally the > driver should have been called SP driver but ccp name existed well > before we added high level SP interface. IIRC, during SP patch review it > was recommended not to rename the driver from ccp->sp because it may > break folks who are already using with ccp name. > > Here is how the config looks: > > +------ CRYPTO_DEV_SP_PSP > | > CRYPTO_DEV_CCP_DD * > (ccp.ko) | +-- ccpv3 > +------ CRYPTO_DEV_SP_CCP --| > +-- ccpv5 > ....
Let me repeat my point more detailed again because it seems like it is still not clear:
KVM needs SEV functionality which is behind CRYPTO_DEV_SP_PSP. Which means, KVM_AMD_SEV should depend *only* on that. This is the symbol which gives that functionality.
Now, you say "CCP driver manages CCP and PSP devices". So CRYPTO_DEV_SP_PSP should select/depend on *everything* it needs in order to function properly in order to provide that functionalty to KVM.
BUT! - and this is the key point - KVM should *not* care what PSP needs to depend on in order to provide that functionality to KVM. PSP should provide all that functionality itself and not rely on KVM_AMD_SEV to select it for the PSP functionality.
IOW, you should have this:
config KVM_AMD_SEV bool "AMD Secure Encrypted Virtualization (SEV) support" depends KVM_AMD && X86_64 && CRYPTO_DEV_SP_PSP
No more symbols.
CRYPTO_DEV_SP_PSP should then do the proper selection so that CRYPTO_DEV_CCP_DD has the proper setting wrt whether KVM is a module or builtin. When you add CRYPTO_DEV_CCP_DD to the dependencies list, you "fix" the design issue that ccp.ko is the driver which provides PSP functionality.
I'm not sure if you could do something with a separate symbol as I suggested earlier:
bool CRYPTO_PSP_FOR_KVM depends on CRYPTO_DEV_CCP_DD && CRYPTO_DEV_SP_PSP
or so and have KVM_AMD_SEV depend on that so that you have one indirection more which wraps the CCP and PSP dependency.
But that seems unnecessary especially since we already have
config CRYPTO_DEV_SP_PSP bool "Platform Security Processor (PSP) device" default y depends on CRYPTO_DEV_CCP_DD && X86_64
so CRYPTO_DEV_SP_PSP already depends on CRYPTO_DEV_CCP_DD!
So why should KVM_AMD_SEV depend on CRYPTO_DEV_CCP_DD too?!? Do you catch my drift?
-- Regards/Gruss, Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --
| |