Re: [PATCH 0/2] Introduce AMD Secure Processor device
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 -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] Introduce AMD Secure Processor device
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. I do realize that we need to get some more context on why this refactoring is needed and how it will benefit the SEV device integration. I will drop this patch for now, will include it as part of next SEV RFC patch series (which provides the complete context with examples). thanks, ~ Brijesh I think we need to see more code here, broken up into sane and reviewable pieces, before we could either say "No don't do that!" or "sure, let me go merge these!" :) thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] Introduce AMD Secure Processor device
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? I think we need to see more code here, broken up into sane and reviewable pieces, before we could either say "No don't do that!" or "sure, let me go merge these!" :) thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] Introduce AMD Secure Processor device
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 -Brijesh -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] Introduce AMD Secure Processor device
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... thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] Introduce AMD Secure Processor device
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. The patch series leaves the CCP cryptographic layer (ccp-crypto* files) in their current directory. The new interface will be used to integrate Secure Encrypted Virtualzation [1] key management and Trusted Execution Environment (TEE) services provided by PSP device. http://marc.info/?l=linux-mm&m=147190938124206&w=2 Brijesh Singh (2): crypto: move CCP device driver to misc misc: amd-sp: introduce the AMD Secure Processor device drivers/crypto/Kconfig | 11 drivers/crypto/Makefile |2 drivers/crypto/ccp/Kconfig | 22 drivers/crypto/ccp/Makefile |9 drivers/crypto/ccp/ccp-dev-v3.c | 574 --- drivers/crypto/ccp/ccp-dev-v5.c | 1021 --- drivers/crypto/ccp/ccp-dev.c| 588 --- drivers/crypto/ccp/ccp-dev.h| 647 drivers/crypto/ccp/ccp-dmaengine.c | 728 -- drivers/crypto/ccp/ccp-ops.c| 1876 --- drivers/crypto/ccp/ccp-pci.c| 354 --- drivers/crypto/ccp/ccp-platform.c | 293 - drivers/misc/Kconfig|1 drivers/misc/Makefile |1 drivers/misc/amd-sp/Kconfig | 22 drivers/misc/amd-sp/Makefile|9 drivers/misc/amd-sp/ccp-dev-v3.c| 578 +++ drivers/misc/amd-sp/ccp-dev-v5.c| 1017 +++ drivers/misc/amd-sp/ccp-dev.c | 611 +++ drivers/misc/amd-sp/ccp-dev.h | 618 drivers/misc/amd-sp/ccp-dmaengine.c | 728 ++ drivers/misc/amd-sp/ccp-ops.c | 1876 +++ drivers/misc/amd-sp/ccp-pci.c | 354 +++ drivers/misc/amd-sp/ccp-platform.c | 293 + drivers/misc/amd-sp/sp-dev.c| 309 ++ drivers/misc/amd-sp/sp-dev.h| 141 +++ drivers/misc/amd-sp/sp-pci.c| 325 ++ drivers/misc/amd-sp/sp-platform.c | 269 + include/linux/ccp.h |3 29 files changed, 7159 insertions(+), 6121 deletions(-) delete mode 100644 drivers/crypto/ccp/ccp-dev-v3.c delete mode 100644 drivers/crypto/ccp/ccp-dev-v5.c delete mode 100644 drivers/crypto/ccp/ccp-dev.c delete mode 100644 drivers/crypto/ccp/ccp-dev.h delete mode 100644 drivers/crypto/ccp/ccp-dmaengine.c delete mode 100644 drivers/crypto/ccp/ccp-ops.c delete mode 100644 drivers/crypto/ccp/ccp-pci.c delete mode 100644 drivers/crypto/ccp/ccp-platform.c create mode 100644 drivers/misc/amd-sp/Kconfig create mode 100644 drivers/misc/amd-sp/Makefile create mode 100644 drivers/misc/amd-sp/ccp-dev-v3.c create mode 100644 drivers/misc/amd-sp/ccp-dev-v5.c create mode 100644 drivers/misc/amd-sp/ccp-dev.c create mode 100644 drivers/misc/amd-sp/ccp-dev.h create mode 100644 drivers/misc/amd-sp/ccp-dmaengine.c create mode 100644 drivers/misc/amd-sp/ccp-ops.c create mode 100644 drivers/misc/amd-sp/ccp-pci.c create mode 100644 drivers/misc/amd-sp/ccp-platform.c create mode 100644 drivers/misc/amd-sp/sp-dev.c create mode 100644 drivers/misc/amd-sp/sp-dev.h create mode 100644 drivers/misc/amd-sp/sp-pci.c create mode 100644 drivers/misc/amd-sp/sp-platform.c -- Brijesh Singh -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
