Re: [Part2 PATCH v5.1 12.1/31] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

From: Borislav Petkov
Date: Tue Oct 10 2017 - 16:17:08 EST


On Tue, Oct 10, 2017 at 01:43:22PM -0500, Tom Lendacky wrote:
> Maybe for the very first implementation we could do that and that was what
> was originally done for the CCP. But as you can see the CCP does not have
> a set register offset between various iterations of the device and it can
> be expected the same will hold true for the PSP. This just makes future
> changes easier in order to support newer devices.

Well, that's a simple switch-case statement which maps device iteration
to offset.

> I would prefer to keep things separated as they are. The common code is
> one place and the pci/platform specific code resides in unique files. For
> sp-pci.c, it can be excluded from compilation if CONFIG_PCI is not defined
> vs. adding #ifdefs into sp-dev.c or sp.c. The code is working nicely and,
> at least to me, seems easily maintainable this way.

But you do see that you're doing a bunch of unnecessary things during
probing. And all those different devices: SP, CCP, PSP, TEE and whatnot,
they're just too granulary and it is a bunch of registration code and
one compilation unit calling into the other for no good reason. Or at
least I don't see it.

> If we really want to avoid the extra calls during probing, etc.
> then we can take a closer look afterwards and determine what is the
> best approach taking into account the CCP and some of the other PSP
> functionality that is coming.

And that coming functionality won't make things easier - you'll most
likely have more things wanting to talk to more other things. But ok,
your call. Just note that changing things later is always harder.

--
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix ImendÃrffer, Jane Smithard, Graham Norton, HRB 21284 (AG NÃrnberg)
--