On 06/19/2018 02:43 PM, Borislav Petkov wrote:
On Tue, Jun 19, 2018 at 02:22:53PM -0500, Brijesh Singh wrote:
It would be nice to have a single depends. But the main issue is, PSP
support is provided through the ccp driver (aka CRYPTO_DEV_CCP_DD).
And this sentence states also your dependency:
KVM_AMD_SEV <- PSP driver <- CCP driver
but what you do is jump over one layer
KVM_AMD_SEV <- PSP driver
KVM_AMD_SEV <- CCP driver
PSP driver <- CCP driver
Do you see the difference and how the former is much easier to express
as a dependency vs the latter? And thus a lot more robust when it comes to
random configs.
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.
Hence KVM_AMD_SEV need to have some level of dependency with ccp
driver. This is to ensure that the ccp was 'y' when kvm-amd=y for SEV
to work.
If that is so, then your dependencies are wrong. KVM doesn't care about
CCP driver - it only cares about the PSP driver which gives the SEV API
it can call. That's it. If you have to make it depend on CCP, then the
design of what depends on what is wrong.
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
....
I am sorry but I am not able to follow you on how creating a separate CRYPTO
.config item will solve this problem. Creating a separate config will be
useful if we are okay with calling 'select' from kvm (i.e if kvm-amd is 'y'
then all the symbols from crypto will be 'y').
This is just an example of what you could do.
I looked at other drivers where they have similar situation and it seems
like solution is same as what I have used above.
Copying how the others do it is just not good enough - especially since
people can create broken configs currently.
