[PATCH] firmware: qcom: scm: Peripheral Authentication Service

[PATCH] firmware: qcom: scm: Peripheral Authentication Service

[Bjorn Andersson]

This adds the Peripheral Authentication Service (PAS) interface to the
Qualcomm SCM interface. The API is used to authenticate and boot a range
of external processors in various Qualcomm platforms.

Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---

While adding the 64 bit interface it might be of interest to extract the dma
allocation in __qcom_scm_pas_init_image() into the common code and pass the
physical pointer to instead, but I haven't looked at the differences to the 64
bit interface.

 drivers/firmware/qcom_scm-32.c | 93 ++++++++++++++++++++++++++++++++++++++++++
 drivers/firmware/qcom_scm.c    | 78 +++++++++++++++++++++++++++++++++++
 drivers/firmware/qcom_scm.h    | 12 ++++++
 include/linux/qcom_scm.h       |  6 +++
 4 files changed, 189 insertions(+)

diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
index 1bd6f9c34331..5674d36a9df9 100644
--- a/drivers/firmware/qcom_scm-32.c
+++ b/drivers/firmware/qcom_scm-32.c
@@ -23,6 +23,7 @@
 #include <linux/errno.h>
 #include <linux/err.h>
 #include <linux/qcom_scm.h>
+#include <linux/dma-mapping.h>
 
 #include <asm/outercache.h>
 #include <asm/cacheflush.h>
@@ -501,3 +502,95 @@ int __qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
 	return qcom_scm_call(QCOM_SCM_SVC_HDCP, QCOM_SCM_CMD_HDCP,
 		req, req_cnt * sizeof(*req), resp, sizeof(*resp));
 }
+
+bool __qcom_scm_pas_supported(u32 peripheral)
+{
+	u32 ret_val;
+	int ret;
+
+	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_IS_SUPPORTED_CMD,
+			    &peripheral, sizeof(peripheral),
+			    &ret_val, sizeof(ret_val));
+
+	return ret ? false : !!ret_val;
+}
+
+int __qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size)
+{
+	dma_addr_t mdata_phys;
+	void *mdata_buf;
+	u32 scm_ret;
+	int ret;
+	struct pas_init_image_req {
+		u32 proc;
+		u32 image_addr;
+	} request;
+
+	/*
+	 * During the scm call memory protection will be enabled for the meta
+	 * data blob, so make sure it's physically contiguous, 4K aligned and
+	 * non-cachable to avoid XPU violations.
+	 */
+	mdata_buf = dma_alloc_coherent(NULL, size, &mdata_phys, GFP_KERNEL);
+	if (!mdata_buf) {
+		pr_err("Allocation of metadata buffer failed.\n");
+		return -ENOMEM;
+	}
+	memcpy(mdata_buf, metadata, size);
+
+	request.proc = peripheral;
+	request.image_addr = mdata_phys;
+
+	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_INIT_IMAGE_CMD,
+			    &request, sizeof(request),
+			    &scm_ret, sizeof(scm_ret));
+
+	dma_free_coherent(NULL, size, mdata_buf, mdata_phys);
+
+	return ret ? : scm_ret;
+}
+
+int __qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, phys_addr_t size)
+{
+	u32 scm_ret;
+	int ret;
+	struct pas_init_image_req {
+		u32 proc;
+		u32 addr;
+		u32 len;
+	} request;
+
+	request.proc = peripheral;
+	request.addr = addr;
+	request.len = size;
+
+	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_MEM_SETUP_CMD,
+			    &request, sizeof(request),
+			    &scm_ret, sizeof(scm_ret));
+
+	return ret ? : scm_ret;
+}
+
+int __qcom_scm_pas_auth_and_reset(u32 peripheral)
+{
+	u32 scm_ret;
+	int ret;
+
+	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_AUTH_AND_RESET_CMD,
+			    &peripheral, sizeof(peripheral),
+			    &scm_ret, sizeof(scm_ret));
+
+	return ret ? : scm_ret;
+}
+
+int __qcom_scm_pas_shutdown(u32 peripheral)
+{
+	u32 scm_ret;
+	int ret;
+
+	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_SHUTDOWN_CMD,
+			    &peripheral, sizeof(peripheral),
+			    &scm_ret, sizeof(scm_ret));
+
+	return ret ? : scm_ret;
+}
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 45c008d68891..ee7150d93b29 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -94,3 +94,81 @@ int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
 	return __qcom_scm_hdcp_req(req, req_cnt, resp);
 }
 EXPORT_SYMBOL(qcom_scm_hdcp_req);
+
+/**
+ * qcom_scm_pas_supported() - Check if the peripheral authentication service is
+ *			      available for the given peripherial
+ * @peripheral:	peripheral id
+ *
+ * Returns true if PAS is supported for this peripheral, otherwise false.
+ */
+bool qcom_scm_pas_supported(u32 peripheral)
+{
+	int ret;
+
+	ret = __qcom_scm_is_call_available(QCOM_SCM_SVC_PIL,
+					   QCOM_SCM_PAS_IS_SUPPORTED_CMD);
+	if (ret <= 0)
+		return false;
+
+	return __qcom_scm_pas_supported(peripheral);
+}
+EXPORT_SYMBOL(qcom_scm_pas_supported);
+
+/**
+ * qcom_scm_pas_init_image() - Initialize peripheral authentication service
+ *			       state machine for a given peripheral, using the
+ *			       metadata
+ * @peripheral: peripheral id
+ * @metadata:	pointer to memory containing ELF header, program header table
+ *		and optional blob of data used for authenticating the metadata
+ *		and the rest of the firmware
+ * @size:	size of the metadata
+ *
+ * Returns 0 on success.
+ */
+int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size)
+{
+	return __qcom_scm_pas_init_image(peripheral, metadata, size);
+}
+EXPORT_SYMBOL(qcom_scm_pas_init_image);
+
+/**
+ * qcom_scm_pas_mem_setup() - Prepare the memory related to a given peripheral
+ *			      for firmware loading
+ * @peripheral:	peripheral id
+ * @addr:	start address of memory area to prepare
+ * @size:	size of the memory area to prepare
+ *
+ * Returns 0 on success.
+ */
+int qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, phys_addr_t size)
+{
+	return __qcom_scm_pas_mem_setup(peripheral, addr, size);
+}
+EXPORT_SYMBOL(qcom_scm_pas_mem_setup);
+
+/**
+ * qcom_scm_pas_auth_and_reset() - Authenticate the given peripheral firmware
+ *				   and reset the remote processor
+ * @peripheral:	peripheral id
+ *
+ * Return 0 on success.
+ */
+int qcom_scm_pas_auth_and_reset(u32 peripheral)
+{
+	return __qcom_scm_pas_auth_and_reset(peripheral);
+}
+EXPORT_SYMBOL(qcom_scm_pas_auth_and_reset);
+
+/**
+ * qcom_scm_pas_shutdown() - Shut down the remote processor
+ * @peripheral: peripheral id
+ *
+ * Returns 0 on success.
+ */
+int qcom_scm_pas_shutdown(u32 peripheral)
+{
+	return __qcom_scm_pas_shutdown(peripheral);
+}
+EXPORT_SYMBOL(qcom_scm_pas_shutdown);
diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
index 2cce75c08b99..c0700590193a 100644
--- a/drivers/firmware/qcom_scm.h
+++ b/drivers/firmware/qcom_scm.h
@@ -36,6 +36,18 @@ extern int __qcom_scm_is_call_available(u32 svc_id, u32 cmd_id);
 extern int __qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt,
 		u32 *resp);
 
+#define QCOM_SCM_SVC_PIL		0x2
+#define QCOM_SCM_PAS_INIT_IMAGE_CMD	0x1
+#define QCOM_SCM_PAS_MEM_SETUP_CMD	0x2
+#define QCOM_SCM_PAS_AUTH_AND_RESET_CMD	0x5
+#define QCOM_SCM_PAS_SHUTDOWN_CMD	0x6
+#define QCOM_SCM_PAS_IS_SUPPORTED_CMD	0x7
+extern bool __qcom_scm_pas_supported(u32 peripheral);
+extern int  __qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size);
+extern int  __qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, phys_addr_t size);
+extern int  __qcom_scm_pas_auth_and_reset(u32 peripheral);
+extern int  __qcom_scm_pas_shutdown(u32 peripheral);
+
 /* common error codes */
 #define QCOM_SCM_ENOMEM		-5
 #define QCOM_SCM_EOPNOTSUPP	-4
diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
index 6e7d5ec65838..46d41e44d432 100644
--- a/include/linux/qcom_scm.h
+++ b/include/linux/qcom_scm.h
@@ -27,6 +27,12 @@ extern bool qcom_scm_hdcp_available(void);
 extern int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt,
 		u32 *resp);
 
+extern bool qcom_scm_pas_supported(u32 peripheral);
+extern int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size);
+extern int qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, phys_addr_t size);
+extern int qcom_scm_pas_auth_and_reset(u32 peripheral);
+extern int qcom_scm_pas_shutdown(u32 peripheral);
+
 #define QCOM_SCM_CPU_PWR_DOWN_L2_ON	0x0
 #define QCOM_SCM_CPU_PWR_DOWN_L2_OFF	0x1
 
-- 
1.8.2.2

[PATCH] firmware: qcom: scm: Peripheral Authentication Service

[Bjorn Andersson]

This adds the Peripheral Authentication Service (PAS) interface to the
Qualcomm SCM interface. The API is used to authenticate and boot a range
of external processors in various Qualcomm platforms.

Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---

While adding the 64 bit interface it might be of interest to extract the dma
allocation in __qcom_scm_pas_init_image() into the common code and pass the
physical pointer to instead, but I haven't looked at the differences to the 64
bit interface.

 drivers/firmware/qcom_scm-32.c | 93 ++++++++++++++++++++++++++++++++++++++++++
 drivers/firmware/qcom_scm.c    | 78 +++++++++++++++++++++++++++++++++++
 drivers/firmware/qcom_scm.h    | 12 ++++++
 include/linux/qcom_scm.h       |  6 +++
 4 files changed, 189 insertions(+)

diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
index 1bd6f9c34331..5674d36a9df9 100644
--- a/drivers/firmware/qcom_scm-32.c
+++ b/drivers/firmware/qcom_scm-32.c
@@ -23,6 +23,7 @@
 #include <linux/errno.h>
 #include <linux/err.h>
 #include <linux/qcom_scm.h>
+#include <linux/dma-mapping.h>
 
 #include <asm/outercache.h>
 #include <asm/cacheflush.h>
@@ -501,3 +502,95 @@ int __qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
 	return qcom_scm_call(QCOM_SCM_SVC_HDCP, QCOM_SCM_CMD_HDCP,
 		req, req_cnt * sizeof(*req), resp, sizeof(*resp));
 }
+
+bool __qcom_scm_pas_supported(u32 peripheral)
+{
+	u32 ret_val;
+	int ret;
+
+	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_IS_SUPPORTED_CMD,
+			    &peripheral, sizeof(peripheral),
+			    &ret_val, sizeof(ret_val));
+
+	return ret ? false : !!ret_val;
+}
+
+int __qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size)
+{
+	dma_addr_t mdata_phys;
+	void *mdata_buf;
+	u32 scm_ret;
+	int ret;
+	struct pas_init_image_req {
+		u32 proc;
+		u32 image_addr;
+	} request;
+
+	/*
+	 * During the scm call memory protection will be enabled for the meta
+	 * data blob, so make sure it's physically contiguous, 4K aligned and
+	 * non-cachable to avoid XPU violations.
+	 */
+	mdata_buf = dma_alloc_coherent(NULL, size, &mdata_phys, GFP_KERNEL);
+	if (!mdata_buf) {
+		pr_err("Allocation of metadata buffer failed.\n");
+		return -ENOMEM;
+	}
+	memcpy(mdata_buf, metadata, size);
+
+	request.proc = peripheral;
+	request.image_addr = mdata_phys;
+
+	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_INIT_IMAGE_CMD,
+			    &request, sizeof(request),
+			    &scm_ret, sizeof(scm_ret));
+
+	dma_free_coherent(NULL, size, mdata_buf, mdata_phys);
+
+	return ret ? : scm_ret;
+}
+
+int __qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, phys_addr_t size)
+{
+	u32 scm_ret;
+	int ret;
+	struct pas_init_image_req {
+		u32 proc;
+		u32 addr;
+		u32 len;
+	} request;
+
+	request.proc = peripheral;
+	request.addr = addr;
+	request.len = size;
+
+	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_MEM_SETUP_CMD,
+			    &request, sizeof(request),
+			    &scm_ret, sizeof(scm_ret));
+
+	return ret ? : scm_ret;
+}
+
+int __qcom_scm_pas_auth_and_reset(u32 peripheral)
+{
+	u32 scm_ret;
+	int ret;
+
+	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_AUTH_AND_RESET_CMD,
+			    &peripheral, sizeof(peripheral),
+			    &scm_ret, sizeof(scm_ret));
+
+	return ret ? : scm_ret;
+}
+
+int __qcom_scm_pas_shutdown(u32 peripheral)
+{
+	u32 scm_ret;
+	int ret;
+
+	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_SHUTDOWN_CMD,
+			    &peripheral, sizeof(peripheral),
+			    &scm_ret, sizeof(scm_ret));
+
+	return ret ? : scm_ret;
+}
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 45c008d68891..ee7150d93b29 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -94,3 +94,81 @@ int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
 	return __qcom_scm_hdcp_req(req, req_cnt, resp);
 }
 EXPORT_SYMBOL(qcom_scm_hdcp_req);
+
+/**
+ * qcom_scm_pas_supported() - Check if the peripheral authentication service is
+ *			      available for the given peripherial
+ * @peripheral:	peripheral id
+ *
+ * Returns true if PAS is supported for this peripheral, otherwise false.
+ */
+bool qcom_scm_pas_supported(u32 peripheral)
+{
+	int ret;
+
+	ret = __qcom_scm_is_call_available(QCOM_SCM_SVC_PIL,
+					   QCOM_SCM_PAS_IS_SUPPORTED_CMD);
+	if (ret <= 0)
+		return false;
+
+	return __qcom_scm_pas_supported(peripheral);
+}
+EXPORT_SYMBOL(qcom_scm_pas_supported);
+
+/**
+ * qcom_scm_pas_init_image() - Initialize peripheral authentication service
+ *			       state machine for a given peripheral, using the
+ *			       metadata
+ * @peripheral: peripheral id
+ * @metadata:	pointer to memory containing ELF header, program header table
+ *		and optional blob of data used for authenticating the metadata
+ *		and the rest of the firmware
+ * @size:	size of the metadata
+ *
+ * Returns 0 on success.
+ */
+int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size)
+{
+	return __qcom_scm_pas_init_image(peripheral, metadata, size);
+}
+EXPORT_SYMBOL(qcom_scm_pas_init_image);
+
+/**
+ * qcom_scm_pas_mem_setup() - Prepare the memory related to a given peripheral
+ *			      for firmware loading
+ * @peripheral:	peripheral id
+ * @addr:	start address of memory area to prepare
+ * @size:	size of the memory area to prepare
+ *
+ * Returns 0 on success.
+ */
+int qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, phys_addr_t size)
+{
+	return __qcom_scm_pas_mem_setup(peripheral, addr, size);
+}
+EXPORT_SYMBOL(qcom_scm_pas_mem_setup);
+
+/**
+ * qcom_scm_pas_auth_and_reset() - Authenticate the given peripheral firmware
+ *				   and reset the remote processor
+ * @peripheral:	peripheral id
+ *
+ * Return 0 on success.
+ */
+int qcom_scm_pas_auth_and_reset(u32 peripheral)
+{
+	return __qcom_scm_pas_auth_and_reset(peripheral);
+}
+EXPORT_SYMBOL(qcom_scm_pas_auth_and_reset);
+
+/**
+ * qcom_scm_pas_shutdown() - Shut down the remote processor
+ * @peripheral: peripheral id
+ *
+ * Returns 0 on success.
+ */
+int qcom_scm_pas_shutdown(u32 peripheral)
+{
+	return __qcom_scm_pas_shutdown(peripheral);
+}
+EXPORT_SYMBOL(qcom_scm_pas_shutdown);
diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
index 2cce75c08b99..c0700590193a 100644
--- a/drivers/firmware/qcom_scm.h
+++ b/drivers/firmware/qcom_scm.h
@@ -36,6 +36,18 @@ extern int __qcom_scm_is_call_available(u32 svc_id, u32 cmd_id);
 extern int __qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt,
 		u32 *resp);
 
+#define QCOM_SCM_SVC_PIL		0x2
+#define QCOM_SCM_PAS_INIT_IMAGE_CMD	0x1
+#define QCOM_SCM_PAS_MEM_SETUP_CMD	0x2
+#define QCOM_SCM_PAS_AUTH_AND_RESET_CMD	0x5
+#define QCOM_SCM_PAS_SHUTDOWN_CMD	0x6
+#define QCOM_SCM_PAS_IS_SUPPORTED_CMD	0x7
+extern bool __qcom_scm_pas_supported(u32 peripheral);
+extern int  __qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size);
+extern int  __qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, phys_addr_t size);
+extern int  __qcom_scm_pas_auth_and_reset(u32 peripheral);
+extern int  __qcom_scm_pas_shutdown(u32 peripheral);
+
 /* common error codes */
 #define QCOM_SCM_ENOMEM		-5
 #define QCOM_SCM_EOPNOTSUPP	-4
diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
index 6e7d5ec65838..46d41e44d432 100644
--- a/include/linux/qcom_scm.h
+++ b/include/linux/qcom_scm.h
@@ -27,6 +27,12 @@ extern bool qcom_scm_hdcp_available(void);
 extern int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt,
 		u32 *resp);
 
+extern bool qcom_scm_pas_supported(u32 peripheral);
+extern int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size);
+extern int qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, phys_addr_t size);
+extern int qcom_scm_pas_auth_and_reset(u32 peripheral);
+extern int qcom_scm_pas_shutdown(u32 peripheral);
+
 #define QCOM_SCM_CPU_PWR_DOWN_L2_ON	0x0
 #define QCOM_SCM_CPU_PWR_DOWN_L2_OFF	0x1
 
-- 
1.8.2.2

Re: [PATCH] firmware: qcom: scm: Peripheral Authentication Service

[Andy Gross]

On Tue, Jun 30, 2015 at 12:46:19PM -0700, Bjorn Andersson wrote:

<snip>

> While adding the 64 bit interface it might be of interest to extract the dma
> allocation in __qcom_scm_pas_init_image() into the common code and pass the
> physical pointer to instead, but I haven't looked at the differences to the 64
> bit interface.

I am currently delving into this.  I'll take this patch and see what needs to be
done.

<snip>

> +int __qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size)
> +{
> +	dma_addr_t mdata_phys;
> +	void *mdata_buf;
> +	u32 scm_ret;
> +	int ret;
> +	struct pas_init_image_req {
> +		u32 proc;
> +		u32 image_addr;
> +	} request;
> +
> +	/*
> +	 * During the scm call memory protection will be enabled for the meta
> +	 * data blob, so make sure it's physically contiguous, 4K aligned and
> +	 * non-cachable to avoid XPU violations.
> +	 */
> +	mdata_buf = dma_alloc_coherent(NULL, size, &mdata_phys, GFP_KERNEL);

Not sure this gives you a 4k aligned buffer every time.

<snip>

Otherwise everything looks fine to me.

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] firmware: qcom: scm: Peripheral Authentication Service

[Stephen Boyd]

On 06/30/2015 12:46 PM, Bjorn Andersson wrote:
> +
> +int __qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size)
> +{
> +	dma_addr_t mdata_phys;
> +	void *mdata_buf;
> +	u32 scm_ret;
> +	int ret;
> +	struct pas_init_image_req {
> +		u32 proc;
> +		u32 image_addr;

Please mark these as __le32 and do the appropriate conversions to little
endian. Also consider removing the struct name and just make it
anonymous, i.e.

    struct {
        __le32 proc;
        __le32 image_addr;
    } cmd;

> +	} request;
> +
> +	/*
> +	 * During the scm call memory protection will be enabled for the meta
> +	 * data blob, so make sure it's physically contiguous, 4K aligned and
> +	 * non-cachable to avoid XPU violations.
> +	 */
> +	mdata_buf = dma_alloc_coherent(NULL, size, &mdata_phys, GFP_KERNEL);
> +	if (!mdata_buf) {
> +		pr_err("Allocation of metadata buffer failed.\n");
> +		return -ENOMEM;
> +	}
> +	memcpy(mdata_buf, metadata, size);
> +
> +	request.proc = peripheral;
> +	request.image_addr = mdata_phys;
> +
> +	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_INIT_IMAGE_CMD,
> +			    &request, sizeof(request),
> +			    &scm_ret, sizeof(scm_ret));
> +
> +	dma_free_coherent(NULL, size, mdata_buf, mdata_phys);
> +
> +	return ret ? : scm_ret;
> +}
> +
> +int __qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, phys_addr_t size)
> +{
> +	u32 scm_ret;
> +	int ret;
> +	struct pas_init_image_req {
> +		u32 proc;
> +		u32 addr;
> +		u32 len;
> +	} request;

Same comment here.

> +
> +	request.proc = peripheral;
> +	request.addr = addr;
> +	request.len = size;
> +
> +	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_MEM_SETUP_CMD,
> +			    &request, sizeof(request),
> +			    &scm_ret, sizeof(scm_ret));
> +
> +	return ret ? : scm_ret;
> +}
> +
>
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] firmware: qcom: scm: Peripheral Authentication Service

[Bjorn Andersson]

On Thu 02 Jul 09:19 PDT 2015, Stephen Boyd wrote:

> On 06/30/2015 12:46 PM, Bjorn Andersson wrote:
> > +
> > +int __qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size)
> > +{
> > +	dma_addr_t mdata_phys;
> > +	void *mdata_buf;
> > +	u32 scm_ret;
> > +	int ret;
> > +	struct pas_init_image_req {
> > +		u32 proc;
> > +		u32 image_addr;
> 
> Please mark these as __le32 and do the appropriate conversions to little
> endian.

Just out of curiosity, are there any big endian systems out there that
will run this code?

Either way I will fix this, to follow the style of the other parts of
this file.

> Also consider removing the struct name and just make it
> anonymous, i.e.
> 
>     struct {
>         __le32 proc;
>         __le32 image_addr;
>     } cmd;
> 

Right, there's no reason to name these.

Thanks,
Bjorn

Re: [PATCH] firmware: qcom: scm: Peripheral Authentication Service

[Stephen Boyd]

On 07/02/2015 10:37 AM, Bjorn Andersson wrote:
> On Thu 02 Jul 09:19 PDT 2015, Stephen Boyd wrote:
>
>> On 06/30/2015 12:46 PM, Bjorn Andersson wrote:
>>> +
>>> +int __qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size)
>>> +{
>>> +	dma_addr_t mdata_phys;
>>> +	void *mdata_buf;
>>> +	u32 scm_ret;
>>> +	int ret;
>>> +	struct pas_init_image_req {
>>> +		u32 proc;
>>> +		u32 image_addr;
>> Please mark these as __le32 and do the appropriate conversions to little
>> endian.
> Just out of curiosity, are there any big endian systems out there that
> will run this code?

The option is available to turn on if someone wants to. I'm not aware of
anyone that's doing it besides me though.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2] firmware: qcom: scm: Peripheral Authentication Service

[Bjorn Andersson]

This adds the Peripheral Authentication Service (PAS) interface to the
Qualcomm SCM interface. The API is used to authenticate and boot a range
of external processors in various Qualcomm platforms.

Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---

Changes since v1:
- Big endian compatibility

 drivers/firmware/qcom_scm-32.c | 93 ++++++++++++++++++++++++++++++++++++++++++
 drivers/firmware/qcom_scm.c    | 78 +++++++++++++++++++++++++++++++++++
 drivers/firmware/qcom_scm.h    | 12 ++++++
 include/linux/qcom_scm.h       |  6 +++
 4 files changed, 189 insertions(+)

diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
index 1bd6f9c34331..81d9c59f3ccc 100644
--- a/drivers/firmware/qcom_scm-32.c
+++ b/drivers/firmware/qcom_scm-32.c
@@ -23,6 +23,7 @@
 #include <linux/errno.h>
 #include <linux/err.h>
 #include <linux/qcom_scm.h>
+#include <linux/dma-mapping.h>
 
 #include <asm/outercache.h>
 #include <asm/cacheflush.h>
@@ -501,3 +502,95 @@ int __qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
 	return qcom_scm_call(QCOM_SCM_SVC_HDCP, QCOM_SCM_CMD_HDCP,
 		req, req_cnt * sizeof(*req), resp, sizeof(*resp));
 }
+
+bool __qcom_scm_pas_supported(u32 peripheral)
+{
+	u32 ret_val;
+	int ret;
+
+	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_IS_SUPPORTED_CMD,
+			    &peripheral, sizeof(peripheral),
+			    &ret_val, sizeof(ret_val));
+
+	return ret ? false : !!ret_val;
+}
+
+int __qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size)
+{
+	dma_addr_t mdata_phys;
+	void *mdata_buf;
+	__le32 scm_ret;
+	int ret;
+	struct pas_init_image_req {
+		__le32 proc;
+		__le32 image_addr;
+	} request;
+
+	/*
+	 * During the scm call memory protection will be enabled for the meta
+	 * data blob, so make sure it's physically contiguous, 4K aligned and
+	 * non-cachable to avoid XPU violations.
+	 */
+	mdata_buf = dma_alloc_coherent(NULL, size, &mdata_phys, GFP_KERNEL);
+	if (!mdata_buf) {
+		pr_err("Allocation of metadata buffer failed.\n");
+		return -ENOMEM;
+	}
+	memcpy(mdata_buf, metadata, size);
+
+	request.proc = cpu_to_le32(peripheral);
+	request.image_addr = cpu_to_le32(mdata_phys);
+
+	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_INIT_IMAGE_CMD,
+			    &request, sizeof(request),
+			    &scm_ret, sizeof(scm_ret));
+
+	dma_free_coherent(NULL, size, mdata_buf, mdata_phys);
+
+	return ret ? : le32_to_cpu(scm_ret);
+}
+
+int __qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, phys_addr_t size)
+{
+	__le32 scm_ret;
+	int ret;
+	struct pas_init_image_req {
+		__le32 proc;
+		__le32 addr;
+		__le32 len;
+	} request;
+
+	request.proc = cpu_to_le32(peripheral);
+	request.addr = cpu_to_le32(addr);
+	request.len = cpu_to_le32(size);
+
+	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_MEM_SETUP_CMD,
+			    &request, sizeof(request),
+			    &scm_ret, sizeof(scm_ret));
+
+	return ret ? : le32_to_cpu(scm_ret);
+}
+
+int __qcom_scm_pas_auth_and_reset(u32 peripheral)
+{
+	__le32 scm_ret;
+	int ret;
+
+	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_AUTH_AND_RESET_CMD,
+			    &peripheral, sizeof(peripheral),
+			    &scm_ret, sizeof(scm_ret));
+
+	return ret ? : le32_to_cpu(scm_ret);
+}
+
+int __qcom_scm_pas_shutdown(u32 peripheral)
+{
+	__le32 scm_ret;
+	int ret;
+
+	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_SHUTDOWN_CMD,
+			    &peripheral, sizeof(peripheral),
+			    &scm_ret, sizeof(scm_ret));
+
+	return ret ? : le32_to_cpu(scm_ret);
+}
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 45c008d68891..ee7150d93b29 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -94,3 +94,81 @@ int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
 	return __qcom_scm_hdcp_req(req, req_cnt, resp);
 }
 EXPORT_SYMBOL(qcom_scm_hdcp_req);
+
+/**
+ * qcom_scm_pas_supported() - Check if the peripheral authentication service is
+ *			      available for the given peripherial
+ * @peripheral:	peripheral id
+ *
+ * Returns true if PAS is supported for this peripheral, otherwise false.
+ */
+bool qcom_scm_pas_supported(u32 peripheral)
+{
+	int ret;
+
+	ret = __qcom_scm_is_call_available(QCOM_SCM_SVC_PIL,
+					   QCOM_SCM_PAS_IS_SUPPORTED_CMD);
+	if (ret <= 0)
+		return false;
+
+	return __qcom_scm_pas_supported(peripheral);
+}
+EXPORT_SYMBOL(qcom_scm_pas_supported);
+
+/**
+ * qcom_scm_pas_init_image() - Initialize peripheral authentication service
+ *			       state machine for a given peripheral, using the
+ *			       metadata
+ * @peripheral: peripheral id
+ * @metadata:	pointer to memory containing ELF header, program header table
+ *		and optional blob of data used for authenticating the metadata
+ *		and the rest of the firmware
+ * @size:	size of the metadata
+ *
+ * Returns 0 on success.
+ */
+int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size)
+{
+	return __qcom_scm_pas_init_image(peripheral, metadata, size);
+}
+EXPORT_SYMBOL(qcom_scm_pas_init_image);
+
+/**
+ * qcom_scm_pas_mem_setup() - Prepare the memory related to a given peripheral
+ *			      for firmware loading
+ * @peripheral:	peripheral id
+ * @addr:	start address of memory area to prepare
+ * @size:	size of the memory area to prepare
+ *
+ * Returns 0 on success.
+ */
+int qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, phys_addr_t size)
+{
+	return __qcom_scm_pas_mem_setup(peripheral, addr, size);
+}
+EXPORT_SYMBOL(qcom_scm_pas_mem_setup);
+
+/**
+ * qcom_scm_pas_auth_and_reset() - Authenticate the given peripheral firmware
+ *				   and reset the remote processor
+ * @peripheral:	peripheral id
+ *
+ * Return 0 on success.
+ */
+int qcom_scm_pas_auth_and_reset(u32 peripheral)
+{
+	return __qcom_scm_pas_auth_and_reset(peripheral);
+}
+EXPORT_SYMBOL(qcom_scm_pas_auth_and_reset);
+
+/**
+ * qcom_scm_pas_shutdown() - Shut down the remote processor
+ * @peripheral: peripheral id
+ *
+ * Returns 0 on success.
+ */
+int qcom_scm_pas_shutdown(u32 peripheral)
+{
+	return __qcom_scm_pas_shutdown(peripheral);
+}
+EXPORT_SYMBOL(qcom_scm_pas_shutdown);
diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
index 2cce75c08b99..c0700590193a 100644
--- a/drivers/firmware/qcom_scm.h
+++ b/drivers/firmware/qcom_scm.h
@@ -36,6 +36,18 @@ extern int __qcom_scm_is_call_available(u32 svc_id, u32 cmd_id);
 extern int __qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt,
 		u32 *resp);
 
+#define QCOM_SCM_SVC_PIL		0x2
+#define QCOM_SCM_PAS_INIT_IMAGE_CMD	0x1
+#define QCOM_SCM_PAS_MEM_SETUP_CMD	0x2
+#define QCOM_SCM_PAS_AUTH_AND_RESET_CMD	0x5
+#define QCOM_SCM_PAS_SHUTDOWN_CMD	0x6
+#define QCOM_SCM_PAS_IS_SUPPORTED_CMD	0x7
+extern bool __qcom_scm_pas_supported(u32 peripheral);
+extern int  __qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size);
+extern int  __qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, phys_addr_t size);
+extern int  __qcom_scm_pas_auth_and_reset(u32 peripheral);
+extern int  __qcom_scm_pas_shutdown(u32 peripheral);
+
 /* common error codes */
 #define QCOM_SCM_ENOMEM		-5
 #define QCOM_SCM_EOPNOTSUPP	-4
diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
index 6e7d5ec65838..46d41e44d432 100644
--- a/include/linux/qcom_scm.h
+++ b/include/linux/qcom_scm.h
@@ -27,6 +27,12 @@ extern bool qcom_scm_hdcp_available(void);
 extern int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt,
 		u32 *resp);
 
+extern bool qcom_scm_pas_supported(u32 peripheral);
+extern int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size);
+extern int qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, phys_addr_t size);
+extern int qcom_scm_pas_auth_and_reset(u32 peripheral);
+extern int qcom_scm_pas_shutdown(u32 peripheral);
+
 #define QCOM_SCM_CPU_PWR_DOWN_L2_ON	0x0
 #define QCOM_SCM_CPU_PWR_DOWN_L2_OFF	0x1
 
-- 
1.8.2.2

[PATCH v2] firmware: qcom: scm: Peripheral Authentication Service

[Bjorn Andersson]

This adds the Peripheral Authentication Service (PAS) interface to the
Qualcomm SCM interface. The API is used to authenticate and boot a range
of external processors in various Qualcomm platforms.

Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---

Changes since v1:
- Big endian compatibility

 drivers/firmware/qcom_scm-32.c | 93 ++++++++++++++++++++++++++++++++++++++++++
 drivers/firmware/qcom_scm.c    | 78 +++++++++++++++++++++++++++++++++++
 drivers/firmware/qcom_scm.h    | 12 ++++++
 include/linux/qcom_scm.h       |  6 +++
 4 files changed, 189 insertions(+)

diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
index 1bd6f9c34331..81d9c59f3ccc 100644
--- a/drivers/firmware/qcom_scm-32.c
+++ b/drivers/firmware/qcom_scm-32.c
@@ -23,6 +23,7 @@
 #include <linux/errno.h>
 #include <linux/err.h>
 #include <linux/qcom_scm.h>
+#include <linux/dma-mapping.h>
 
 #include <asm/outercache.h>
 #include <asm/cacheflush.h>
@@ -501,3 +502,95 @@ int __qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
 	return qcom_scm_call(QCOM_SCM_SVC_HDCP, QCOM_SCM_CMD_HDCP,
 		req, req_cnt * sizeof(*req), resp, sizeof(*resp));
 }
+
+bool __qcom_scm_pas_supported(u32 peripheral)
+{
+	u32 ret_val;
+	int ret;
+
+	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_IS_SUPPORTED_CMD,
+			    &peripheral, sizeof(peripheral),
+			    &ret_val, sizeof(ret_val));
+
+	return ret ? false : !!ret_val;
+}
+
+int __qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size)
+{
+	dma_addr_t mdata_phys;
+	void *mdata_buf;
+	__le32 scm_ret;
+	int ret;
+	struct pas_init_image_req {
+		__le32 proc;
+		__le32 image_addr;
+	} request;
+
+	/*
+	 * During the scm call memory protection will be enabled for the meta
+	 * data blob, so make sure it's physically contiguous, 4K aligned and
+	 * non-cachable to avoid XPU violations.
+	 */
+	mdata_buf = dma_alloc_coherent(NULL, size, &mdata_phys, GFP_KERNEL);
+	if (!mdata_buf) {
+		pr_err("Allocation of metadata buffer failed.\n");
+		return -ENOMEM;
+	}
+	memcpy(mdata_buf, metadata, size);
+
+	request.proc = cpu_to_le32(peripheral);
+	request.image_addr = cpu_to_le32(mdata_phys);
+
+	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_INIT_IMAGE_CMD,
+			    &request, sizeof(request),
+			    &scm_ret, sizeof(scm_ret));
+
+	dma_free_coherent(NULL, size, mdata_buf, mdata_phys);
+
+	return ret ? : le32_to_cpu(scm_ret);
+}
+
+int __qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, phys_addr_t size)
+{
+	__le32 scm_ret;
+	int ret;
+	struct pas_init_image_req {
+		__le32 proc;
+		__le32 addr;
+		__le32 len;
+	} request;
+
+	request.proc = cpu_to_le32(peripheral);
+	request.addr = cpu_to_le32(addr);
+	request.len = cpu_to_le32(size);
+
+	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_MEM_SETUP_CMD,
+			    &request, sizeof(request),
+			    &scm_ret, sizeof(scm_ret));
+
+	return ret ? : le32_to_cpu(scm_ret);
+}
+
+int __qcom_scm_pas_auth_and_reset(u32 peripheral)
+{
+	__le32 scm_ret;
+	int ret;
+
+	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_AUTH_AND_RESET_CMD,
+			    &peripheral, sizeof(peripheral),
+			    &scm_ret, sizeof(scm_ret));
+
+	return ret ? : le32_to_cpu(scm_ret);
+}
+
+int __qcom_scm_pas_shutdown(u32 peripheral)
+{
+	__le32 scm_ret;
+	int ret;
+
+	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_SHUTDOWN_CMD,
+			    &peripheral, sizeof(peripheral),
+			    &scm_ret, sizeof(scm_ret));
+
+	return ret ? : le32_to_cpu(scm_ret);
+}
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 45c008d68891..ee7150d93b29 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -94,3 +94,81 @@ int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
 	return __qcom_scm_hdcp_req(req, req_cnt, resp);
 }
 EXPORT_SYMBOL(qcom_scm_hdcp_req);
+
+/**
+ * qcom_scm_pas_supported() - Check if the peripheral authentication service is
+ *			      available for the given peripherial
+ * @peripheral:	peripheral id
+ *
+ * Returns true if PAS is supported for this peripheral, otherwise false.
+ */
+bool qcom_scm_pas_supported(u32 peripheral)
+{
+	int ret;
+
+	ret = __qcom_scm_is_call_available(QCOM_SCM_SVC_PIL,
+					   QCOM_SCM_PAS_IS_SUPPORTED_CMD);
+	if (ret <= 0)
+		return false;
+
+	return __qcom_scm_pas_supported(peripheral);
+}
+EXPORT_SYMBOL(qcom_scm_pas_supported);
+
+/**
+ * qcom_scm_pas_init_image() - Initialize peripheral authentication service
+ *			       state machine for a given peripheral, using the
+ *			       metadata
+ * @peripheral: peripheral id
+ * @metadata:	pointer to memory containing ELF header, program header table
+ *		and optional blob of data used for authenticating the metadata
+ *		and the rest of the firmware
+ * @size:	size of the metadata
+ *
+ * Returns 0 on success.
+ */
+int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size)
+{
+	return __qcom_scm_pas_init_image(peripheral, metadata, size);
+}
+EXPORT_SYMBOL(qcom_scm_pas_init_image);
+
+/**
+ * qcom_scm_pas_mem_setup() - Prepare the memory related to a given peripheral
+ *			      for firmware loading
+ * @peripheral:	peripheral id
+ * @addr:	start address of memory area to prepare
+ * @size:	size of the memory area to prepare
+ *
+ * Returns 0 on success.
+ */
+int qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, phys_addr_t size)
+{
+	return __qcom_scm_pas_mem_setup(peripheral, addr, size);
+}
+EXPORT_SYMBOL(qcom_scm_pas_mem_setup);
+
+/**
+ * qcom_scm_pas_auth_and_reset() - Authenticate the given peripheral firmware
+ *				   and reset the remote processor
+ * @peripheral:	peripheral id
+ *
+ * Return 0 on success.
+ */
+int qcom_scm_pas_auth_and_reset(u32 peripheral)
+{
+	return __qcom_scm_pas_auth_and_reset(peripheral);
+}
+EXPORT_SYMBOL(qcom_scm_pas_auth_and_reset);
+
+/**
+ * qcom_scm_pas_shutdown() - Shut down the remote processor
+ * @peripheral: peripheral id
+ *
+ * Returns 0 on success.
+ */
+int qcom_scm_pas_shutdown(u32 peripheral)
+{
+	return __qcom_scm_pas_shutdown(peripheral);
+}
+EXPORT_SYMBOL(qcom_scm_pas_shutdown);
diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
index 2cce75c08b99..c0700590193a 100644
--- a/drivers/firmware/qcom_scm.h
+++ b/drivers/firmware/qcom_scm.h
@@ -36,6 +36,18 @@ extern int __qcom_scm_is_call_available(u32 svc_id, u32 cmd_id);
 extern int __qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt,
 		u32 *resp);
 
+#define QCOM_SCM_SVC_PIL		0x2
+#define QCOM_SCM_PAS_INIT_IMAGE_CMD	0x1
+#define QCOM_SCM_PAS_MEM_SETUP_CMD	0x2
+#define QCOM_SCM_PAS_AUTH_AND_RESET_CMD	0x5
+#define QCOM_SCM_PAS_SHUTDOWN_CMD	0x6
+#define QCOM_SCM_PAS_IS_SUPPORTED_CMD	0x7
+extern bool __qcom_scm_pas_supported(u32 peripheral);
+extern int  __qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size);
+extern int  __qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, phys_addr_t size);
+extern int  __qcom_scm_pas_auth_and_reset(u32 peripheral);
+extern int  __qcom_scm_pas_shutdown(u32 peripheral);
+
 /* common error codes */
 #define QCOM_SCM_ENOMEM		-5
 #define QCOM_SCM_EOPNOTSUPP	-4
diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
index 6e7d5ec65838..46d41e44d432 100644
--- a/include/linux/qcom_scm.h
+++ b/include/linux/qcom_scm.h
@@ -27,6 +27,12 @@ extern bool qcom_scm_hdcp_available(void);
 extern int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt,
 		u32 *resp);
 
+extern bool qcom_scm_pas_supported(u32 peripheral);
+extern int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size);
+extern int qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, phys_addr_t size);
+extern int qcom_scm_pas_auth_and_reset(u32 peripheral);
+extern int qcom_scm_pas_shutdown(u32 peripheral);
+
 #define QCOM_SCM_CPU_PWR_DOWN_L2_ON	0x0
 #define QCOM_SCM_CPU_PWR_DOWN_L2_OFF	0x1
 
-- 
1.8.2.2

Re: [PATCH v2] firmware: qcom: scm: Peripheral Authentication Service

[Andy Gross]

On Wed, Jul 15, 2015 at 11:58:06AM -0700, Bjorn Andersson wrote:

<snip>

> +int __qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size)
> +{
> +	dma_addr_t mdata_phys;
> +	void *mdata_buf;
> +	__le32 scm_ret;
> +	int ret;
> +	struct pas_init_image_req {
> +		__le32 proc;
> +		__le32 image_addr;
> +	} request;
> +
> +	/*
> +	 * During the scm call memory protection will be enabled for the meta
> +	 * data blob, so make sure it's physically contiguous, 4K aligned and
> +	 * non-cachable to avoid XPU violations.
> +	 */
> +	mdata_buf = dma_alloc_coherent(NULL, size, &mdata_phys, GFP_KERNEL);

It'd be nice to send in the dev so we can attribute the allocation correctly.
For 64 bit, we actually have to do this because NULL is not allowed right now.

> +	if (!mdata_buf) {
> +		pr_err("Allocation of metadata buffer failed.\n");
> +		return -ENOMEM;
> +	}
> +	memcpy(mdata_buf, metadata, size);
> +
> +	request.proc = cpu_to_le32(peripheral);
> +	request.image_addr = cpu_to_le32(mdata_phys);
> +
> +	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_INIT_IMAGE_CMD,
> +			    &request, sizeof(request),
> +			    &scm_ret, sizeof(scm_ret));
> +
> +	dma_free_coherent(NULL, size, mdata_buf, mdata_phys);
> +
> +	return ret ? : le32_to_cpu(scm_ret);
> +}
> +

Otherwise, looks good.  Let me know if you plan to modify for the device above.
If you want me to do it later, thats fine.  But it means API change.

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2] firmware: qcom: scm: Peripheral Authentication Service

[Stephen Boyd]

On 07/15, Bjorn Andersson wrote:
> This adds the Peripheral Authentication Service (PAS) interface to the
> Qualcomm SCM interface. The API is used to authenticate and boot a range
> of external processors in various Qualcomm platforms.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> ---
> 
> Changes since v1:
> - Big endian compatibility

Did you try out a big endian kernel?

> diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
> index 1bd6f9c34331..81d9c59f3ccc 100644
> --- a/drivers/firmware/qcom_scm-32.c
> +++ b/drivers/firmware/qcom_scm-32.c
> @@ -501,3 +502,95 @@ int __qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
>  	return qcom_scm_call(QCOM_SCM_SVC_HDCP, QCOM_SCM_CMD_HDCP,
>  		req, req_cnt * sizeof(*req), resp, sizeof(*resp));
>  }
> +
> +bool __qcom_scm_pas_supported(u32 peripheral)
> +{
> +	u32 ret_val;

I guess we don't have to care about __le32 here because it's just
a zero or non-zero value?

> +	int ret;
> +
> +	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_IS_SUPPORTED_CMD,
> +			    &peripheral, sizeof(peripheral),

Peripheral needs cpu_to_le32() treatment.

> +			    &ret_val, sizeof(ret_val));
> +
> +	return ret ? false : !!ret_val;
> +}
> +
> +int __qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size)
> +{
> +	dma_addr_t mdata_phys;
> +	void *mdata_buf;
> +	__le32 scm_ret;
> +	int ret;
> +	struct pas_init_image_req {
> +		__le32 proc;
> +		__le32 image_addr;
> +	} request;
> +
> +	/*
> +	 * During the scm call memory protection will be enabled for the meta
> +	 * data blob, so make sure it's physically contiguous, 4K aligned and
> +	 * non-cachable to avoid XPU violations.
> +	 */
> +	mdata_buf = dma_alloc_coherent(NULL, size, &mdata_phys, GFP_KERNEL);

This should pass a device pointer instead of NULL here. Please
make struct device an argument of this function and pass
something there besides NULL in the calling driver. Or we should
make SCM into a platform device driver with a node in DT (named
firmware?). Then if we need to do anything special for DMA to the
firmware, etc. we have a struct device that can describe that.

Also, dma_alloc_coherent() doesn't do enough to prevent XPU
violations because memory returned from that function on ARM is
not guaranteed to be device memory and so we could speculatively
access the locked down metadata region. This is why we added the
strongly ordered mapping property and pass that to
dma_alloc_attrs in the downstream code so we can change the page
table attributes of the mapping to be device memory. Not doing
this can lead to random crashes when some read speculates on the
metadata and the secure world intercepts it and shuts the system
down.

I was going to say we could try to use the carveout/reserved
memory code but that doesn't look fool proof. From what I can
tell CMA doesn't use the same page table attributes for the
mapping that dma-coherent does, so if we use dma-coherent it will
use ioremap and work but if we use CMA it won't (at least it
looks like bufferable memory there). Can we add a way to request
memory doesn't allow speculatioan through the DMA APIs?

> +	if (!mdata_buf) {
> +		pr_err("Allocation of metadata buffer failed.\n");
> +		return -ENOMEM;
> +	}
> +	memcpy(mdata_buf, metadata, size);
> +
> +	request.proc = cpu_to_le32(peripheral);
> +	request.image_addr = cpu_to_le32(mdata_phys);
> +
> +	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_INIT_IMAGE_CMD,
> +			    &request, sizeof(request),
> +			    &scm_ret, sizeof(scm_ret));
> +
> +	dma_free_coherent(NULL, size, mdata_buf, mdata_phys);
> +
> +	return ret ? : le32_to_cpu(scm_ret);
> +}
> +
> +int __qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, phys_addr_t size)
> +{
> +	__le32 scm_ret;
> +	int ret;
> +	struct pas_init_image_req {

I thought this was going to be an unnamed structure?

	struct {
		__le32 proc;
		__le32 addr;
		__le32 len;
	} cmd;

> +		__le32 proc;
> +		__le32 addr;
> +		__le32 len;
> +	} request;
> +
> +	request.proc = cpu_to_le32(peripheral);
> +	request.addr = cpu_to_le32(addr);
> +	request.len = cpu_to_le32(size);
> +
> +	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_MEM_SETUP_CMD,
> +			    &request, sizeof(request),
> +			    &scm_ret, sizeof(scm_ret));
> +
> +	return ret ? : le32_to_cpu(scm_ret);
> +}
> +
> +int __qcom_scm_pas_auth_and_reset(u32 peripheral)
> +{
> +	__le32 scm_ret;
> +	int ret;
> +
> +	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_AUTH_AND_RESET_CMD,
> +			    &peripheral, sizeof(peripheral),

peripheral needs cpu_to_le32() treatment.

> +			    &scm_ret, sizeof(scm_ret));
> +
> +	return ret ? : le32_to_cpu(scm_ret);
> +}
> +
> +int __qcom_scm_pas_shutdown(u32 peripheral)
> +{
> +	__le32 scm_ret;
> +	int ret;
> +
> +	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_SHUTDOWN_CMD,
> +			    &peripheral, sizeof(peripheral),

peripheral needs cpu_to_le32() treatment.

> +			    &scm_ret, sizeof(scm_ret));
> +
> +	return ret ? : le32_to_cpu(scm_ret);
> +}

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2] firmware: qcom: scm: Peripheral Authentication Service

[Bjorn Andersson]

On Wed 15 Jul 16:43 PDT 2015, Stephen Boyd wrote:

> On 07/15, Bjorn Andersson wrote:
> > This adds the Peripheral Authentication Service (PAS) interface to the
> > Qualcomm SCM interface. The API is used to authenticate and boot a range
> > of external processors in various Qualcomm platforms.
> > 
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> > ---
> > 
> > Changes since v1:
> > - Big endian compatibility
> 
> Did you try out a big endian kernel?
> 

No, you're still the only one.

> > diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
> > index 1bd6f9c34331..81d9c59f3ccc 100644
> > --- a/drivers/firmware/qcom_scm-32.c
> > +++ b/drivers/firmware/qcom_scm-32.c
> > @@ -501,3 +502,95 @@ int __qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
> >  	return qcom_scm_call(QCOM_SCM_SVC_HDCP, QCOM_SCM_CMD_HDCP,
> >  		req, req_cnt * sizeof(*req), resp, sizeof(*resp));
> >  }
> > +
> > +bool __qcom_scm_pas_supported(u32 peripheral)
> > +{
> > +	u32 ret_val;
> 
> I guess we don't have to care about __le32 here because it's just
> a zero or non-zero value?
> 

Right, but could be fixed up for completeness.

> > +	int ret;
> > +
> > +	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_IS_SUPPORTED_CMD,
> > +			    &peripheral, sizeof(peripheral),
> 
> Peripheral needs cpu_to_le32() treatment.
> 

Sorry, missed that one.

> > +			    &ret_val, sizeof(ret_val));
> > +
> > +	return ret ? false : !!ret_val;
> > +}
> > +
> > +int __qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size)
> > +{
> > +	dma_addr_t mdata_phys;
> > +	void *mdata_buf;
> > +	__le32 scm_ret;
> > +	int ret;
> > +	struct pas_init_image_req {
> > +		__le32 proc;
> > +		__le32 image_addr;
> > +	} request;
> > +
> > +	/*
> > +	 * During the scm call memory protection will be enabled for the meta
> > +	 * data blob, so make sure it's physically contiguous, 4K aligned and
> > +	 * non-cachable to avoid XPU violations.
> > +	 */
> > +	mdata_buf = dma_alloc_coherent(NULL, size, &mdata_phys, GFP_KERNEL);
> 
> This should pass a device pointer instead of NULL here. Please
> make struct device an argument of this function and pass
> something there besides NULL in the calling driver. Or we should
> make SCM into a platform device driver with a node in DT (named
> firmware?). Then if we need to do anything special for DMA to the
> firmware, etc. we have a struct device that can describe that.
> 

I think making scm into a platform driver seems very much overkill,
passing the callers device * sounds reasonable. My only concern would be
if associating this dma allocation with the client has any further
implications, but I'll have to read up a little bit on how that works.

> Also, dma_alloc_coherent() doesn't do enough to prevent XPU
> violations because memory returned from that function on ARM is
> not guaranteed to be device memory and so we could speculatively
> access the locked down metadata region. This is why we added the
> strongly ordered mapping property and pass that to
> dma_alloc_attrs in the downstream code so we can change the page
> table attributes of the mapping to be device memory. Not doing
> this can lead to random crashes when some read speculates on the
> metadata and the secure world intercepts it and shuts the system
> down.
> 

The code is taken verbatim from msm-3.4 and the comment is picked from
the git log, sorry to hear that this is not enough.

> I was going to say we could try to use the carveout/reserved
> memory code but that doesn't look fool proof. From what I can
> tell CMA doesn't use the same page table attributes for the
> mapping that dma-coherent does, so if we use dma-coherent it will
> use ioremap and work but if we use CMA it won't (at least it
> looks like bufferable memory there). Can we add a way to request
> memory doesn't allow speculatioan through the DMA APIs?
> 

I haven't looked enough at dma allocations, but this is what worries me
when using the clients dev pointer (I'm under the impression that these
choices follow the dev*).

> > +	if (!mdata_buf) {
> > +		pr_err("Allocation of metadata buffer failed.\n");
> > +		return -ENOMEM;
> > +	}
> > +	memcpy(mdata_buf, metadata, size);
> > +
> > +	request.proc = cpu_to_le32(peripheral);
> > +	request.image_addr = cpu_to_le32(mdata_phys);
> > +
> > +	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_INIT_IMAGE_CMD,
> > +			    &request, sizeof(request),
> > +			    &scm_ret, sizeof(scm_ret));
> > +
> > +	dma_free_coherent(NULL, size, mdata_buf, mdata_phys);
> > +
> > +	return ret ? : le32_to_cpu(scm_ret);
> > +}
> > +
> > +int __qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, phys_addr_t size)
> > +{
> > +	__le32 scm_ret;
> > +	int ret;
> > +	struct pas_init_image_req {
> 
> I thought this was going to be an unnamed structure?
> 
> 	struct {
> 		__le32 proc;
> 		__le32 addr;
> 		__le32 len;
> 	} cmd;
> 

It was, sorry about that.

> > +		__le32 proc;
> > +		__le32 addr;
> > +		__le32 len;
> > +	} request;
> > +
> > +	request.proc = cpu_to_le32(peripheral);
> > +	request.addr = cpu_to_le32(addr);
> > +	request.len = cpu_to_le32(size);
> > +
> > +	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_MEM_SETUP_CMD,
> > +			    &request, sizeof(request),
> > +			    &scm_ret, sizeof(scm_ret));
> > +
> > +	return ret ? : le32_to_cpu(scm_ret);
> > +}
> > +
> > +int __qcom_scm_pas_auth_and_reset(u32 peripheral)
> > +{
> > +	__le32 scm_ret;
> > +	int ret;
> > +
> > +	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_AUTH_AND_RESET_CMD,
> > +			    &peripheral, sizeof(peripheral),
> 
> peripheral needs cpu_to_le32() treatment.
> 

Sorry, missed these.

> > +			    &scm_ret, sizeof(scm_ret));
> > +
> > +	return ret ? : le32_to_cpu(scm_ret);
> > +}
> > +
> > +int __qcom_scm_pas_shutdown(u32 peripheral)
> > +{
> > +	__le32 scm_ret;
> > +	int ret;
> > +
> > +	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_SHUTDOWN_CMD,
> > +			    &peripheral, sizeof(peripheral),
> 
> peripheral needs cpu_to_le32() treatment.
> 

Ditto

> > +			    &scm_ret, sizeof(scm_ret));
> > +
> > +	return ret ? : le32_to_cpu(scm_ret);
> > +}

Regards,
Bjorn

Re: [PATCH v2] firmware: qcom: scm: Peripheral Authentication Service

[Bjorn Andersson]

On Wed 15 Jul 16:43 PDT 2015, Stephen Boyd wrote:

> On 07/15, Bjorn Andersson wrote:
> > This adds the Peripheral Authentication Service (PAS) interface to the
> > Qualcomm SCM interface. The API is used to authenticate and boot a range
> > of external processors in various Qualcomm platforms.
> > 
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> > ---
> > 
> > Changes since v1:
> > - Big endian compatibility
> 
> Did you try out a big endian kernel?
> 

No, you're still the only one.

> > diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
> > index 1bd6f9c34331..81d9c59f3ccc 100644
> > --- a/drivers/firmware/qcom_scm-32.c
> > +++ b/drivers/firmware/qcom_scm-32.c
> > @@ -501,3 +502,95 @@ int __qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
> >  	return qcom_scm_call(QCOM_SCM_SVC_HDCP, QCOM_SCM_CMD_HDCP,
> >  		req, req_cnt * sizeof(*req), resp, sizeof(*resp));
> >  }
> > +
> > +bool __qcom_scm_pas_supported(u32 peripheral)
> > +{
> > +	u32 ret_val;
> 
> I guess we don't have to care about __le32 here because it's just
> a zero or non-zero value?
> 

Right, but could be fixed up for completeness.

> > +	int ret;
> > +
> > +	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_IS_SUPPORTED_CMD,
> > +			    &peripheral, sizeof(peripheral),
> 
> Peripheral needs cpu_to_le32() treatment.
> 

Sorry, missed that one.

> > +			    &ret_val, sizeof(ret_val));
> > +
> > +	return ret ? false : !!ret_val;
> > +}
> > +
> > +int __qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size)
> > +{
> > +	dma_addr_t mdata_phys;
> > +	void *mdata_buf;
> > +	__le32 scm_ret;
> > +	int ret;
> > +	struct pas_init_image_req {
> > +		__le32 proc;
> > +		__le32 image_addr;
> > +	} request;
> > +
> > +	/*
> > +	 * During the scm call memory protection will be enabled for the meta
> > +	 * data blob, so make sure it's physically contiguous, 4K aligned and
> > +	 * non-cachable to avoid XPU violations.
> > +	 */
> > +	mdata_buf = dma_alloc_coherent(NULL, size, &mdata_phys, GFP_KERNEL);
> 
> This should pass a device pointer instead of NULL here. Please
> make struct device an argument of this function and pass
> something there besides NULL in the calling driver. Or we should
> make SCM into a platform device driver with a node in DT (named
> firmware?). Then if we need to do anything special for DMA to the
> firmware, etc. we have a struct device that can describe that.
> 

I think making scm into a platform driver seems very much overkill,
passing the callers device * sounds reasonable. My only concern would be
if associating this dma allocation with the client has any further
implications, but I'll have to read up a little bit on how that works.

> Also, dma_alloc_coherent() doesn't do enough to prevent XPU
> violations because memory returned from that function on ARM is
> not guaranteed to be device memory and so we could speculatively
> access the locked down metadata region. This is why we added the
> strongly ordered mapping property and pass that to
> dma_alloc_attrs in the downstream code so we can change the page
> table attributes of the mapping to be device memory. Not doing
> this can lead to random crashes when some read speculates on the
> metadata and the secure world intercepts it and shuts the system
> down.
> 

The code is taken verbatim from msm-3.4 and the comment is picked from
the git log, sorry to hear that this is not enough.

> I was going to say we could try to use the carveout/reserved
> memory code but that doesn't look fool proof. From what I can
> tell CMA doesn't use the same page table attributes for the
> mapping that dma-coherent does, so if we use dma-coherent it will
> use ioremap and work but if we use CMA it won't (at least it
> looks like bufferable memory there). Can we add a way to request
> memory doesn't allow speculatioan through the DMA APIs?
> 

I haven't looked enough at dma allocations, but this is what worries me
when using the clients dev pointer (I'm under the impression that these
choices follow the dev*).

> > +	if (!mdata_buf) {
> > +		pr_err("Allocation of metadata buffer failed.\n");
> > +		return -ENOMEM;
> > +	}
> > +	memcpy(mdata_buf, metadata, size);
> > +
> > +	request.proc = cpu_to_le32(peripheral);
> > +	request.image_addr = cpu_to_le32(mdata_phys);
> > +
> > +	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_INIT_IMAGE_CMD,
> > +			    &request, sizeof(request),
> > +			    &scm_ret, sizeof(scm_ret));
> > +
> > +	dma_free_coherent(NULL, size, mdata_buf, mdata_phys);
> > +
> > +	return ret ? : le32_to_cpu(scm_ret);
> > +}
> > +
> > +int __qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, phys_addr_t size)
> > +{
> > +	__le32 scm_ret;
> > +	int ret;
> > +	struct pas_init_image_req {
> 
> I thought this was going to be an unnamed structure?
> 
> 	struct {
> 		__le32 proc;
> 		__le32 addr;
> 		__le32 len;
> 	} cmd;
> 

It was, sorry about that.

> > +		__le32 proc;
> > +		__le32 addr;
> > +		__le32 len;
> > +	} request;
> > +
> > +	request.proc = cpu_to_le32(peripheral);
> > +	request.addr = cpu_to_le32(addr);
> > +	request.len = cpu_to_le32(size);
> > +
> > +	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_MEM_SETUP_CMD,
> > +			    &request, sizeof(request),
> > +			    &scm_ret, sizeof(scm_ret));
> > +
> > +	return ret ? : le32_to_cpu(scm_ret);
> > +}
> > +
> > +int __qcom_scm_pas_auth_and_reset(u32 peripheral)
> > +{
> > +	__le32 scm_ret;
> > +	int ret;
> > +
> > +	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_AUTH_AND_RESET_CMD,
> > +			    &peripheral, sizeof(peripheral),
> 
> peripheral needs cpu_to_le32() treatment.
> 

Sorry, missed these.

> > +			    &scm_ret, sizeof(scm_ret));
> > +
> > +	return ret ? : le32_to_cpu(scm_ret);
> > +}
> > +
> > +int __qcom_scm_pas_shutdown(u32 peripheral)
> > +{
> > +	__le32 scm_ret;
> > +	int ret;
> > +
> > +	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_SHUTDOWN_CMD,
> > +			    &peripheral, sizeof(peripheral),
> 
> peripheral needs cpu_to_le32() treatment.
> 

Ditto

> > +			    &scm_ret, sizeof(scm_ret));
> > +
> > +	return ret ? : le32_to_cpu(scm_ret);
> > +}

Regards,
Bjorn

Re: [PATCH v2] firmware: qcom: scm: Peripheral Authentication Service

[Bjorn Andersson]

On Wed 15 Jul 14:33 PDT 2015, Andy Gross wrote:

> On Wed, Jul 15, 2015 at 11:58:06AM -0700, Bjorn Andersson wrote:
> 
> <snip>
> 
> > +int __qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size)
> > +{
> > +	dma_addr_t mdata_phys;
> > +	void *mdata_buf;
> > +	__le32 scm_ret;
> > +	int ret;
> > +	struct pas_init_image_req {
> > +		__le32 proc;
> > +		__le32 image_addr;
> > +	} request;
> > +
> > +	/*
> > +	 * During the scm call memory protection will be enabled for the meta
> > +	 * data blob, so make sure it's physically contiguous, 4K aligned and
> > +	 * non-cachable to avoid XPU violations.
> > +	 */
> > +	mdata_buf = dma_alloc_coherent(NULL, size, &mdata_phys, GFP_KERNEL);
> 
> It'd be nice to send in the dev so we can attribute the allocation correctly.
> For 64 bit, we actually have to do this because NULL is not allowed right now.
> 

As per Stephen's answer I need to respin this anyways, but I think I
need to read up a little bit more on the implications of passing e.g.
the PIL dev* down here.

> > +	if (!mdata_buf) {
> > +		pr_err("Allocation of metadata buffer failed.\n");
> > +		return -ENOMEM;
> > +	}
> > +	memcpy(mdata_buf, metadata, size);
> > +
> > +	request.proc = cpu_to_le32(peripheral);
> > +	request.image_addr = cpu_to_le32(mdata_phys);
> > +
> > +	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_INIT_IMAGE_CMD,
> > +			    &request, sizeof(request),
> > +			    &scm_ret, sizeof(scm_ret));
> > +
> > +	dma_free_coherent(NULL, size, mdata_buf, mdata_phys);
> > +
> > +	return ret ? : le32_to_cpu(scm_ret);
> > +}
> > +
> 
> Otherwise, looks good.  Let me know if you plan to modify for the device above.
> If you want me to do it later, thats fine.  But it means API change.
> 

I'll respin this in a bit, need to read up a little bit more on the dma
allocation. But most likely I'll just move the allocation to the common
code, pass the physical address here and expect a dev* in the public
api.

Regards,
Bjorn

Re: [PATCH v2] firmware: qcom: scm: Peripheral Authentication Service

[Bjorn Andersson]

On Wed 15 Jul 14:33 PDT 2015, Andy Gross wrote:

> On Wed, Jul 15, 2015 at 11:58:06AM -0700, Bjorn Andersson wrote:
> 
> <snip>
> 
> > +int __qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size)
> > +{
> > +	dma_addr_t mdata_phys;
> > +	void *mdata_buf;
> > +	__le32 scm_ret;
> > +	int ret;
> > +	struct pas_init_image_req {
> > +		__le32 proc;
> > +		__le32 image_addr;
> > +	} request;
> > +
> > +	/*
> > +	 * During the scm call memory protection will be enabled for the meta
> > +	 * data blob, so make sure it's physically contiguous, 4K aligned and
> > +	 * non-cachable to avoid XPU violations.
> > +	 */
> > +	mdata_buf = dma_alloc_coherent(NULL, size, &mdata_phys, GFP_KERNEL);
> 
> It'd be nice to send in the dev so we can attribute the allocation correctly.
> For 64 bit, we actually have to do this because NULL is not allowed right now.
> 

As per Stephen's answer I need to respin this anyways, but I think I
need to read up a little bit more on the implications of passing e.g.
the PIL dev* down here.

> > +	if (!mdata_buf) {
> > +		pr_err("Allocation of metadata buffer failed.\n");
> > +		return -ENOMEM;
> > +	}
> > +	memcpy(mdata_buf, metadata, size);
> > +
> > +	request.proc = cpu_to_le32(peripheral);
> > +	request.image_addr = cpu_to_le32(mdata_phys);
> > +
> > +	ret = qcom_scm_call(QCOM_SCM_SVC_PIL, QCOM_SCM_PAS_INIT_IMAGE_CMD,
> > +			    &request, sizeof(request),
> > +			    &scm_ret, sizeof(scm_ret));
> > +
> > +	dma_free_coherent(NULL, size, mdata_buf, mdata_phys);
> > +
> > +	return ret ? : le32_to_cpu(scm_ret);
> > +}
> > +
> 
> Otherwise, looks good.  Let me know if you plan to modify for the device above.
> If you want me to do it later, thats fine.  But it means API change.
> 

I'll respin this in a bit, need to read up a little bit more on the dma
allocation. But most likely I'll just move the allocation to the common
code, pass the physical address here and expect a dev* in the public
api.

Regards,
Bjorn

Re: [PATCH v2] firmware: qcom: scm: Peripheral Authentication Service

[Stephen Boyd]

On 07/15/2015 05:35 PM, Bjorn Andersson wrote:
> On Wed 15 Jul 16:43 PDT 2015, Stephen Boyd wrote:
>
>> On 07/15, Bjorn Andersson wrote:
>>> This adds the Peripheral Authentication Service (PAS) interface to the
>>> Qualcomm SCM interface. The API is used to authenticate and boot a range
>>> of external processors in various Qualcomm platforms.
>>>
>>> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
>>> ---
>>>
>>> Changes since v1:
>>> - Big endian compatibility
>> Did you try out a big endian kernel?
>>
> No, you're still the only one.

:)

>>> +			    &ret_val, sizeof(ret_val));
>>> +
>>> +	return ret ? false : !!ret_val;
>>> +}
>>> +
>>> +int __qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size)
>>> +{
>>> +	dma_addr_t mdata_phys;
>>> +	void *mdata_buf;
>>> +	__le32 scm_ret;
>>> +	int ret;
>>> +	struct pas_init_image_req {
>>> +		__le32 proc;
>>> +		__le32 image_addr;
>>> +	} request;
>>> +
>>> +	/*
>>> +	 * During the scm call memory protection will be enabled for the meta
>>> +	 * data blob, so make sure it's physically contiguous, 4K aligned and
>>> +	 * non-cachable to avoid XPU violations.
>>> +	 */
>>> +	mdata_buf = dma_alloc_coherent(NULL, size, &mdata_phys, GFP_KERNEL);
>> This should pass a device pointer instead of NULL here. Please
>> make struct device an argument of this function and pass
>> something there besides NULL in the calling driver. Or we should
>> make SCM into a platform device driver with a node in DT (named
>> firmware?). Then if we need to do anything special for DMA to the
>> firmware, etc. we have a struct device that can describe that.
>>
> I think making scm into a platform driver seems very much overkill,
> passing the callers device * sounds reasonable. My only concern would be
> if associating this dma allocation with the client has any further
> implications, but I'll have to read up a little bit on how that works.
>
>> Also, dma_alloc_coherent() doesn't do enough to prevent XPU
>> violations because memory returned from that function on ARM is
>> not guaranteed to be device memory and so we could speculatively
>> access the locked down metadata region. This is why we added the
>> strongly ordered mapping property and pass that to
>> dma_alloc_attrs in the downstream code so we can change the page
>> table attributes of the mapping to be device memory. Not doing
>> this can lead to random crashes when some read speculates on the
>> metadata and the secure world intercepts it and shuts the system
>> down.
>>
> The code is taken verbatim from msm-3.4 and the comment is picked from
> the git log, sorry to hear that this is not enough.

Please move up to msm-3.14 or msm-3.10. Try to find the newest stuff if 
it's code like this that isn't specific for a particular SoC. Otherwise 
we're going to miss random bug fixes that haven't trickled down to trees 
for chips that are two to three years old.

>
>> I was going to say we could try to use the carveout/reserved
>> memory code but that doesn't look fool proof. From what I can
>> tell CMA doesn't use the same page table attributes for the
>> mapping that dma-coherent does, so if we use dma-coherent it will
>> use ioremap and work but if we use CMA it won't (at least it
>> looks like bufferable memory there). Can we add a way to request
>> memory doesn't allow speculatioan through the DMA APIs?
>>
> I haven't looked enough at dma allocations, but this is what worries me
> when using the clients dev pointer (I'm under the impression that these
> choices follow the dev*).

Yes it does. If the device is cache coherent (e.g. the video processor 
may be cache coherent) or even if we want to have two different regions 
of memory carved out for the device then using the client's dev pointer 
won't work well.

I think for this sort of allocation it makes sense to make SCM into a 
platform driver/device so that we can assign the right attributes to a 
memory carveout associated with it. It will also help when we need to 
max out crypto clocks and bus bandwidth or other things that are 
strictly related to what the firmware needs and not the remote 
processor. The trouble is probe defer, so we may need to have some sort 
of get/put API that returns EPROBE_DEFER so that client drivers can 
figure out when they need to wait for SCM to be ready.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2] firmware: qcom: scm: Peripheral Authentication Service

[Bjorn Andersson]

On Wed 15 Jul 17:55 PDT 2015, Stephen Boyd wrote:

> On 07/15/2015 05:35 PM, Bjorn Andersson wrote:
> >On Wed 15 Jul 16:43 PDT 2015, Stephen Boyd wrote:
> >
> >>On 07/15, Bjorn Andersson wrote:
[..]
> >>Also, dma_alloc_coherent() doesn't do enough to prevent XPU
> >>violations because memory returned from that function on ARM is
> >>not guaranteed to be device memory and so we could speculatively
> >>access the locked down metadata region. This is why we added the
> >>strongly ordered mapping property and pass that to
> >>dma_alloc_attrs in the downstream code so we can change the page
> >>table attributes of the mapping to be device memory. Not doing
> >>this can lead to random crashes when some read speculates on the
> >>metadata and the secure world intercepts it and shuts the system
> >>down.
> >>
> >The code is taken verbatim from msm-3.4 and the comment is picked from
> >the git log, sorry to hear that this is not enough.
> 
> Please move up to msm-3.14 or msm-3.10. Try to find the newest stuff if it's
> code like this that isn't specific for a particular SoC. Otherwise we're
> going to miss random bug fixes that haven't trickled down to trees for chips
> that are two to three years old.
> 

Right, with the introduction of the 64 bit platforms this code was
altered to specify the strictly ordered attribute. I have to look at how
this should be done in mainline, as I'm moving this out to the common
code.

> >
> >>I was going to say we could try to use the carveout/reserved
> >>memory code but that doesn't look fool proof. From what I can
> >>tell CMA doesn't use the same page table attributes for the
> >>mapping that dma-coherent does, so if we use dma-coherent it will
> >>use ioremap and work but if we use CMA it won't (at least it
> >>looks like bufferable memory there). Can we add a way to request
> >>memory doesn't allow speculatioan through the DMA APIs?
> >>
> >I haven't looked enough at dma allocations, but this is what worries me
> >when using the clients dev pointer (I'm under the impression that these
> >choices follow the dev*).
> 
> Yes it does. If the device is cache coherent (e.g. the video processor may
> be cache coherent) or even if we want to have two different regions of
> memory carved out for the device then using the client's dev pointer won't
> work well.
> 

I would like to allocate the peripheral memory in PIL from CMA, if so I
guess we have this issue ;)

> I think for this sort of allocation it makes sense to make SCM into a
> platform driver/device so that we can assign the right attributes to a
> memory carveout associated with it. It will also help when we need to max
> out crypto clocks and bus bandwidth or other things that are strictly
> related to what the firmware needs and not the remote processor. The trouble
> is probe defer, so we may need to have some sort of get/put API that returns
> EPROBE_DEFER so that client drivers can figure out when they need to wait
> for SCM to be ready.
> 

Right, it would definitely clarify the ownership and handling of the
crypto clocks.

Regards,
Bjorn

Re: [PATCH v2] firmware: qcom: scm: Peripheral Authentication Service

[Bjorn Andersson]

On Wed 15 Jul 17:55 PDT 2015, Stephen Boyd wrote:

> On 07/15/2015 05:35 PM, Bjorn Andersson wrote:
> >On Wed 15 Jul 16:43 PDT 2015, Stephen Boyd wrote:
> >
> >>On 07/15, Bjorn Andersson wrote:
[..]
> >>Also, dma_alloc_coherent() doesn't do enough to prevent XPU
> >>violations because memory returned from that function on ARM is
> >>not guaranteed to be device memory and so we could speculatively
> >>access the locked down metadata region. This is why we added the
> >>strongly ordered mapping property and pass that to
> >>dma_alloc_attrs in the downstream code so we can change the page
> >>table attributes of the mapping to be device memory. Not doing
> >>this can lead to random crashes when some read speculates on the
> >>metadata and the secure world intercepts it and shuts the system
> >>down.
> >>
> >The code is taken verbatim from msm-3.4 and the comment is picked from
> >the git log, sorry to hear that this is not enough.
> 
> Please move up to msm-3.14 or msm-3.10. Try to find the newest stuff if it's
> code like this that isn't specific for a particular SoC. Otherwise we're
> going to miss random bug fixes that haven't trickled down to trees for chips
> that are two to three years old.
> 

Right, with the introduction of the 64 bit platforms this code was
altered to specify the strictly ordered attribute. I have to look at how
this should be done in mainline, as I'm moving this out to the common
code.

> >
> >>I was going to say we could try to use the carveout/reserved
> >>memory code but that doesn't look fool proof. From what I can
> >>tell CMA doesn't use the same page table attributes for the
> >>mapping that dma-coherent does, so if we use dma-coherent it will
> >>use ioremap and work but if we use CMA it won't (at least it
> >>looks like bufferable memory there). Can we add a way to request
> >>memory doesn't allow speculatioan through the DMA APIs?
> >>
> >I haven't looked enough at dma allocations, but this is what worries me
> >when using the clients dev pointer (I'm under the impression that these
> >choices follow the dev*).
> 
> Yes it does. If the device is cache coherent (e.g. the video processor may
> be cache coherent) or even if we want to have two different regions of
> memory carved out for the device then using the client's dev pointer won't
> work well.
> 

I would like to allocate the peripheral memory in PIL from CMA, if so I
guess we have this issue ;)

> I think for this sort of allocation it makes sense to make SCM into a
> platform driver/device so that we can assign the right attributes to a
> memory carveout associated with it. It will also help when we need to max
> out crypto clocks and bus bandwidth or other things that are strictly
> related to what the firmware needs and not the remote processor. The trouble
> is probe defer, so we may need to have some sort of get/put API that returns
> EPROBE_DEFER so that client drivers can figure out when they need to wait
> for SCM to be ready.
> 

Right, it would definitely clarify the ownership and handling of the
crypto clocks.

Regards,
Bjorn