Re: [PATCHv1] Add Intel Stratix10 service layer driver

From: Richard Gong
Date: Mon Jan 29 2018 - 21:06:12 EST


Hi Greg,

Many thanks for your reviews.


On 01/25/2018 10:53 AM, Greg KH wrote:
On Thu, Jan 25, 2018 at 10:39:03AM -0600, richard.gong@xxxxxxxxxxxxxxx wrote:
From: Richard Gong <richard.gong@xxxxxxxxx>

Intel Stratix10 SoC is composed of a 64 bit quad-core ARM Cortex A53 hard
processor system (HPS) and Secure Device Manager (SDM). SDM is the hardware
which does the FPGA configuration, QSPI, Crypto and warm reset.

When the FPGA is configured from HPS, there needs to be a way for HPS to
notify SDM the location and size of the configuration data. Then SDM will
get the configuration data from that location and perform the FPGA configuration.

To meet the whole system security needs and support virtual machine
requesting communication with SDM, only the secure world of software (EL3,
Exception Level 3) can interface with SDM. All software entities running
on other exception levels must channel through the EL3 software whenever it
needs service from SDM.

Intel Stratix10 service layer driver is added to provide the service for
FPGA configuration. Running at privileged exception level (EL1, Exception
Level 1), Intel Stratix10 service layer driver interfaces with the service
provider at EL1 (Intel Stratix10 FPGA Manager) and manages secure monitor
call (SMC) to communicate with secure monitor software at secure monitor
exception level (EL3).

Later the Intel Stratix10 service layer driver will be extended to provide
services for QSPI, Crypto and warm reset.

Richard Gong (1):
driver: misc: add Intel Stratix10 service layer driver

drivers/misc/Kconfig | 3 +-
drivers/misc/Makefile | 3 +-
drivers/misc/intel-service/Kconfig | 9 +
drivers/misc/intel-service/Makefile | 2 +
drivers/misc/intel-service/intel_service.c | 703 +++++++++++++++++++++++++++++
include/linux/intel-service-client.h | 227 ++++++++++
include/linux/intel-service.h | 122 +++++
include/linux/intel-smc.h | 246 ++++++++++
Simple questions first:
- why do you have 3 different .h files for a single .c file?
This is because service layer driver interface with both the service provider and secure monitor SW.
intel-service-client.h is created to define interface between service providers (FPGA manager is one of them) and service layer. Alan Tull's FPGA manager .c file includes this header file
intel-smc.h defines the secure monitor call (SMC) message protocols used for service layer driver in normal world (EL1) to communicate with secure monitor SW in secure monitor exception level 3 (EL3). Also this header file is shared with firmware since both (FW, service layer) utilizes the same SMC message protocol.
intel-sevice.h is created to define service layer's own data structures (service controller, channel for communicating with service provider, shared memory region, private data etc)

- why do you have any public .h files for a single .c file?
intel-service-client.h is public .h and should be at include/linux/
intel-service.h and intel-smc.h are private .h files, should be in driver/misc/ (assume I move .c file from driver/misc/intel-service/ to driver/misc/)
- use the correct SPDX markers for your file licenses, Intel legal
knows all about this, please follow their rules.
I will follow those rules.
- why is this in a subdirectory for a single .c file?
Currently service layer is implemented to support FPGA configuration only. We have the new requirements and need to extend service layer to support additional features such as QSPI, Crypto and warm reset. It is expected that a few new files will be added later.
For now I can move the single .c file from driver/misc/intel-service/ to driver/misc/.

Regards,
Richard
thanks,

greg k-h