Re: [PATCH 0/2] Introduce AMD Secure Processor device

[Greg KH]

On Fri, Jan 20, 2017 at 09:40:49AM -0600, Brijesh Singh wrote:
> 
> On 01/20/2017 02:45 AM, Greg KH wrote:
> > On Thu, Jan 19, 2017 at 02:03:12PM -0600, Brijesh Singh wrote:
> > > Hi Greg,
> > > 
> > > On 01/19/2017 12:21 PM, Greg KH wrote:
> > > > On Thu, Jan 19, 2017 at 01:07:50PM -0500, Brijesh Singh wrote:
> > > > > The CCP device (drivers/crypto/ccp/ccp.ko) is part of AMD Secure Processor,
> > > > > which is not dedicated solely to crypto. The AMD Secure Processor includes
> > > > > CCP and PSP (Platform Secure Processor) devices.
> > > > > 
> > > > > This patch series moves the CCP device driver to the misc directory and
> > > > > creates a framework that allows functional component of the AMD Secure
> > > > > Processor to be initialized and handled appropriately.
> > > > 
> > > > Why the misc directory?  I don't see the justification here...
> > > > 
> > > 
> > > Since this driver is not solely for crypto purposes and do not fit in any of
> > > the standard categories hence I thought of moving it into misc directory. I
> > > am open to other suggestions unless Herbert is ok with leaving it into
> > > crypto and allowing the addition of the Secure Processor support.
> > > 
> > > The patch series allows the CCP driver to support other Secure Processor
> > > functions, e.g Secure Encrypted Virtualization (SEV) key management. In
> > > past, I tried to add SEV support into existing CCP driver [1] but we quickly
> > > learned that CCP driver should be moved outside the crypto directory
> > > otherwise will end up adding non crypto code into drivers/crypto directory.
> > > Once this cleanup is accepted then I can work to add SEV support inside the
> > > CCP driver.
> > > 
> > > [1] http://marc.info/?l=linux-kernel&m=147204118426151&w=2
> > 
> > Ok, what type of interface will this driver have with userspace and/or
> > other parts of the kernel?  Is there a misc char device burried in there
> > somewhere (I couldn't find it in the big diff sent out), or is this
> > driver just creating specific apis that other parts of the kernel will
> > call if available?
> > 
> 
> Eventually, the driver will export functions which will be used by KVM
> to encrypt the guest memory and more. Additionally, If SEV device is
> detected then driver will create a misc char device which can be used by
> userspace to import/export certificates etc.

Why create a new api for certificates, why not just use the existing
kernel key handling for it?

Having a random char device for something like this is going to be rough
to approve, I'll wait for the patches before I start objecting really
hard :)

thanks,

greg k-h