[PATCH 3/6] remoteproc: qcom: Add helper function to support IOMMU devmem translation

Mukesh Ojha posted 6 patches 1 month, 3 weeks ago
[PATCH 3/6] remoteproc: qcom: Add helper function to support IOMMU devmem translation
Posted by Mukesh Ojha 1 month, 3 weeks ago
From: Shiraz Hashim <quic_shashim@quicinc.com>

Qualcomm SoCs runnning with Qualcomm EL2 hypervisor(QHEE), IOMMU
translation set up for remote processors is managed by QHEE itself
however, for a case when these remote processors has to run under KVM
hypervisor, IOMMU translation need to setup from Linux remoteproc driver
before it is brought up.

Add qcom_devmem_info and qcom_devmem_table data structure and make a
common helper functions which caller can call if these translation need
to be taken care by the driver to enable iommu devmem access for
remoteproc processors.

Signed-off-by: Shiraz Hashim <quic_shashim@quicinc.com>
Co-developed-by: Mukesh Ojha <quic_mojha@quicinc.com>
Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
---
 drivers/remoteproc/qcom_common.c | 96 ++++++++++++++++++++++++++++++++
 drivers/remoteproc/qcom_common.h | 35 ++++++++++++
 2 files changed, 131 insertions(+)

diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
index 1c7887dc65b4..644920972b58 100644
--- a/drivers/remoteproc/qcom_common.c
+++ b/drivers/remoteproc/qcom_common.c
@@ -658,5 +658,101 @@ int qcom_map_unmap_carveout(struct rproc *rproc, phys_addr_t mem_phys, size_t me
 }
 EXPORT_SYMBOL_GPL(qcom_map_unmap_carveout);
 
+/**
+ * qcom_map_devmem() - Map the device memories needed by Remoteproc using IOMMU
+ *
+ * When Qualcomm EL2 hypervisor(QHEE) present, device memories needed for remoteproc
+ * processors is managed by it and Linux remoteproc drivers should not call
+ * this and its respective unmap function in such scenario. This function
+ * should only be called if remoteproc IOMMU translation need to be managed
+ * from Linux side.
+ *
+ * @rproc: rproc handle
+ * @devmem_table: list of devmem regions to map
+ * @use_sid: decision to append sid to iova
+ * @sid: SID value
+ */
+int qcom_map_devmem(struct rproc *rproc, struct qcom_devmem_table *devmem_table,
+		    bool use_sid, unsigned long sid)
+{
+	struct qcom_devmem_info *info;
+	unsigned long sid_def_val;
+	int ret;
+	int i;
+
+	if (!rproc->has_iommu)
+		return 0;
+
+	if (!rproc->domain)
+		return -EINVAL;
+
+	/* remoteproc may not have devmem data */
+	if (!devmem_table)
+		return 0;
+
+	if (use_sid && sid)
+		sid_def_val = sid & SID_MASK_DEFAULT;
+
+	info = &devmem_table->entries[0];
+	for (i = 0; i < devmem_table->num_entries; i++, info++) {
+		/*
+		 * Remote processor like ADSP supports upto 36 bit device
+		 * address space and some of its clients like fastrpc uses
+		 * upper 32-35 bits to keep lower 4 bits of its SID to use
+		 * larger address space. To keep this consistent across other
+		 * use cases add remoteproc SID configuration for firmware
+		 * to IOMMU for carveouts.
+		 */
+		if (use_sid)
+			info->da |= (sid_def_val << 32);
+
+		ret = iommu_map(rproc->domain, info->da, info->pa, info->len, info->flags, GFP_KERNEL);
+		if (ret) {
+			dev_err(&rproc->dev, "Unable to map devmem, ret: %d\n", ret);
+			if (use_sid)
+				info->da &= ~(SID_MASK_DEFAULT << 32);
+			goto undo_mapping;
+		}
+	}
+
+	return 0;
+
+undo_mapping:
+	for (i = i - 1; i >= 0; i--, info--) {
+		iommu_unmap(rproc->domain, info->da, info->len);
+		if (use_sid)
+			info->da &= ~(SID_MASK_DEFAULT << 32);
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(qcom_map_devmem);
+
+/**
+ * qcom_unmap_devmem() -  unmap the device memories needed by Remoteproc using IOMMU
+ *
+ * @rproc:		rproc handle
+ * @devmem_table:	list of devmem regions to unmap
+ * @use_sid:		decision to append sid to iova
+ */
+void qcom_unmap_devmem(struct rproc *rproc, struct qcom_devmem_table *devmem_table, bool use_sid)
+{
+	struct qcom_devmem_info *info;
+	int i;
+
+	if (!rproc->has_iommu || !rproc->domain || !devmem_table)
+		return;
+
+	info = &devmem_table->entries[0];
+	for (i = 0; i < devmem_table->num_entries; i++, info++) {
+		iommu_unmap(rproc->domain, info->da, info->len);
+		if (use_sid)
+			info->da &= ~(SID_MASK_DEFAULT << 32);
+	}
+
+	return;
+}
+EXPORT_SYMBOL_GPL(qcom_unmap_devmem);
+
 MODULE_DESCRIPTION("Qualcomm Remoteproc helper driver");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/remoteproc/qcom_common.h b/drivers/remoteproc/qcom_common.h
index bbc41054e1ea..bbc684e1df01 100644
--- a/drivers/remoteproc/qcom_common.h
+++ b/drivers/remoteproc/qcom_common.h
@@ -41,6 +41,36 @@ struct qcom_rproc_pdm {
 	struct auxiliary_device *adev;
 };
 
+/**
+ * struct qcom_devmem_info - iommu devmem region
+ * @da: device address
+ * @pa: physical address
+ * @len: length (in bytes)
+ * @flags: iommu protection flags
+ *
+ * The resource entry carries the device address to which a physical address is
+ * to be mapped with required permissions in flag. The pa, len is expected to
+ * be a physically contiguous memory region.
+ */
+struct qcom_devmem_info {
+	u64 da;
+	u64 pa;
+	u32 len;
+	u32 flags;
+};
+
+/**
+ * struct qcom_devmem_table - iommu devmem entries
+ * @num_entries: number of devmem entries
+ * @entries: devmem entries
+ *
+ * The table that carries each devmem resource entry.
+ */
+struct qcom_devmem_table {
+	int num_entries;
+	struct qcom_devmem_info entries[0];
+};
+
 void qcom_minidump(struct rproc *rproc, unsigned int minidump_id,
 			void (*rproc_dumpfn_t)(struct rproc *rproc,
 				struct rproc_dump_segment *segment, void *dest, size_t offset,
@@ -65,6 +95,11 @@ int qcom_map_unmap_carveout(struct rproc *rproc, phys_addr_t mem_phys, size_t me
 void qcom_add_pdm_subdev(struct rproc *rproc, struct qcom_rproc_pdm *pdm);
 void qcom_remove_pdm_subdev(struct rproc *rproc, struct qcom_rproc_pdm *pdm);
 
+int qcom_map_devmem(struct rproc *rproc, struct qcom_devmem_table *table,
+		    bool use_sid, unsigned long sid);
+void qcom_unmap_devmem(struct rproc *rproc, struct qcom_devmem_table *table,
+		       bool use_sid);
+
 #if IS_ENABLED(CONFIG_QCOM_SYSMON)
 struct qcom_sysmon *qcom_add_sysmon_subdev(struct rproc *rproc,
 					   const char *name,
-- 
2.34.1
Re: [PATCH 3/6] remoteproc: qcom: Add helper function to support IOMMU devmem translation
Posted by neil.armstrong@linaro.org 1 month, 3 weeks ago
On 04/10/2024 23:23, Mukesh Ojha wrote:
> From: Shiraz Hashim <quic_shashim@quicinc.com>
> 
> Qualcomm SoCs runnning with Qualcomm EL2 hypervisor(QHEE), IOMMU
> translation set up for remote processors is managed by QHEE itself
> however, for a case when these remote processors has to run under KVM

This is not true, KVM is a Linux hypervisor, remote processors have
nothing to do with KVM, please rephrase.

> hypervisor, IOMMU translation need to setup from Linux remoteproc driver
> before it is brought up.
> 
> Add qcom_devmem_info and qcom_devmem_table data structure and make a
> common helper functions which caller can call if these translation need
> to be taken care by the driver to enable iommu devmem access for
> remoteproc processors.
> 
> Signed-off-by: Shiraz Hashim <quic_shashim@quicinc.com>
> Co-developed-by: Mukesh Ojha <quic_mojha@quicinc.com>
> Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
> ---
>   drivers/remoteproc/qcom_common.c | 96 ++++++++++++++++++++++++++++++++
>   drivers/remoteproc/qcom_common.h | 35 ++++++++++++
>   2 files changed, 131 insertions(+)
> 
> diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
> index 1c7887dc65b4..644920972b58 100644
> --- a/drivers/remoteproc/qcom_common.c
> +++ b/drivers/remoteproc/qcom_common.c
> @@ -658,5 +658,101 @@ int qcom_map_unmap_carveout(struct rproc *rproc, phys_addr_t mem_phys, size_t me
>   }
>   EXPORT_SYMBOL_GPL(qcom_map_unmap_carveout);
>   
> +/**
> + * qcom_map_devmem() - Map the device memories needed by Remoteproc using IOMMU
> + *
> + * When Qualcomm EL2 hypervisor(QHEE) present, device memories needed for remoteproc
> + * processors is managed by it and Linux remoteproc drivers should not call
> + * this and its respective unmap function in such scenario. This function
> + * should only be called if remoteproc IOMMU translation need to be managed
> + * from Linux side.
> + *
> + * @rproc: rproc handle
> + * @devmem_table: list of devmem regions to map
> + * @use_sid: decision to append sid to iova
> + * @sid: SID value
> + */
> +int qcom_map_devmem(struct rproc *rproc, struct qcom_devmem_table *devmem_table,
> +		    bool use_sid, unsigned long sid)
> +{
> +	struct qcom_devmem_info *info;
> +	unsigned long sid_def_val;
> +	int ret;
> +	int i;
> +
> +	if (!rproc->has_iommu)
> +		return 0;
> +
> +	if (!rproc->domain)
> +		return -EINVAL;
> +
> +	/* remoteproc may not have devmem data */
> +	if (!devmem_table)
> +		return 0;
> +
> +	if (use_sid && sid)
> +		sid_def_val = sid & SID_MASK_DEFAULT;
> +
> +	info = &devmem_table->entries[0];
> +	for (i = 0; i < devmem_table->num_entries; i++, info++) {
> +		/*
> +		 * Remote processor like ADSP supports upto 36 bit device
> +		 * address space and some of its clients like fastrpc uses
> +		 * upper 32-35 bits to keep lower 4 bits of its SID to use
> +		 * larger address space. To keep this consistent across other
> +		 * use cases add remoteproc SID configuration for firmware
> +		 * to IOMMU for carveouts.
> +		 */
> +		if (use_sid)
> +			info->da |= (sid_def_val << 32);
> +
> +		ret = iommu_map(rproc->domain, info->da, info->pa, info->len, info->flags, GFP_KERNEL);
> +		if (ret) {
> +			dev_err(&rproc->dev, "Unable to map devmem, ret: %d\n", ret);
> +			if (use_sid)
> +				info->da &= ~(SID_MASK_DEFAULT << 32);
> +			goto undo_mapping;
> +		}
> +	}
> +
> +	return 0;
> +
> +undo_mapping:
> +	for (i = i - 1; i >= 0; i--, info--) {
> +		iommu_unmap(rproc->domain, info->da, info->len);
> +		if (use_sid)
> +			info->da &= ~(SID_MASK_DEFAULT << 32);
> +	}
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(qcom_map_devmem);
> +
> +/**
> + * qcom_unmap_devmem() -  unmap the device memories needed by Remoteproc using IOMMU
> + *
> + * @rproc:		rproc handle
> + * @devmem_table:	list of devmem regions to unmap
> + * @use_sid:		decision to append sid to iova
> + */
> +void qcom_unmap_devmem(struct rproc *rproc, struct qcom_devmem_table *devmem_table, bool use_sid)
> +{
> +	struct qcom_devmem_info *info;
> +	int i;
> +
> +	if (!rproc->has_iommu || !rproc->domain || !devmem_table)
> +		return;
> +
> +	info = &devmem_table->entries[0];
> +	for (i = 0; i < devmem_table->num_entries; i++, info++) {
> +		iommu_unmap(rproc->domain, info->da, info->len);
> +		if (use_sid)
> +			info->da &= ~(SID_MASK_DEFAULT << 32);
> +	}
> +
> +	return;
> +}
> +EXPORT_SYMBOL_GPL(qcom_unmap_devmem);
> +
>   MODULE_DESCRIPTION("Qualcomm Remoteproc helper driver");
>   MODULE_LICENSE("GPL v2");
> diff --git a/drivers/remoteproc/qcom_common.h b/drivers/remoteproc/qcom_common.h
> index bbc41054e1ea..bbc684e1df01 100644
> --- a/drivers/remoteproc/qcom_common.h
> +++ b/drivers/remoteproc/qcom_common.h
> @@ -41,6 +41,36 @@ struct qcom_rproc_pdm {
>   	struct auxiliary_device *adev;
>   };
>   
> +/**
> + * struct qcom_devmem_info - iommu devmem region
> + * @da: device address
> + * @pa: physical address
> + * @len: length (in bytes)
> + * @flags: iommu protection flags
> + *
> + * The resource entry carries the device address to which a physical address is
> + * to be mapped with required permissions in flag. The pa, len is expected to
> + * be a physically contiguous memory region.
> + */
> +struct qcom_devmem_info {
> +	u64 da;
> +	u64 pa;
> +	u32 len;
> +	u32 flags;
> +};
> +
> +/**
> + * struct qcom_devmem_table - iommu devmem entries
> + * @num_entries: number of devmem entries
> + * @entries: devmem entries
> + *
> + * The table that carries each devmem resource entry.
> + */
> +struct qcom_devmem_table {
> +	int num_entries;
> +	struct qcom_devmem_info entries[0];
> +};
> +
>   void qcom_minidump(struct rproc *rproc, unsigned int minidump_id,
>   			void (*rproc_dumpfn_t)(struct rproc *rproc,
>   				struct rproc_dump_segment *segment, void *dest, size_t offset,
> @@ -65,6 +95,11 @@ int qcom_map_unmap_carveout(struct rproc *rproc, phys_addr_t mem_phys, size_t me
>   void qcom_add_pdm_subdev(struct rproc *rproc, struct qcom_rproc_pdm *pdm);
>   void qcom_remove_pdm_subdev(struct rproc *rproc, struct qcom_rproc_pdm *pdm);
>   
> +int qcom_map_devmem(struct rproc *rproc, struct qcom_devmem_table *table,
> +		    bool use_sid, unsigned long sid);
> +void qcom_unmap_devmem(struct rproc *rproc, struct qcom_devmem_table *table,
> +		       bool use_sid);
> +
>   #if IS_ENABLED(CONFIG_QCOM_SYSMON)
>   struct qcom_sysmon *qcom_add_sysmon_subdev(struct rproc *rproc,
>   					   const char *name,
Re: [PATCH 3/6] remoteproc: qcom: Add helper function to support IOMMU devmem translation
Posted by Mukesh Ojha 1 month, 3 weeks ago
On Mon, Oct 07, 2024 at 10:08:16AM +0200, neil.armstrong@linaro.org wrote:
> On 04/10/2024 23:23, Mukesh Ojha wrote:
> > From: Shiraz Hashim <quic_shashim@quicinc.com>
> > 
> > Qualcomm SoCs runnning with Qualcomm EL2 hypervisor(QHEE), IOMMU
> > translation set up for remote processors is managed by QHEE itself
> > however, for a case when these remote processors has to run under KVM
> 
> This is not true, KVM is a Linux hypervisor, remote processors have
> nothing to do with KVM, please rephrase.

Thanks, perhaps something like this,

"However, when same SoC runs with KVM configuration, remoteproc IOMMU
translation needs to be set from Linux host running remoteproc PAS
driver"

-Mukesh
Re: [PATCH 3/6] remoteproc: qcom: Add helper function to support IOMMU devmem translation
Posted by neil.armstrong@linaro.org 1 month, 2 weeks ago
Hi,

On 07/10/2024 16:37, Mukesh Ojha wrote:
> On Mon, Oct 07, 2024 at 10:08:16AM +0200, neil.armstrong@linaro.org wrote:
>> On 04/10/2024 23:23, Mukesh Ojha wrote:
>>> From: Shiraz Hashim <quic_shashim@quicinc.com>
>>>
>>> Qualcomm SoCs runnning with Qualcomm EL2 hypervisor(QHEE), IOMMU
>>> translation set up for remote processors is managed by QHEE itself
>>> however, for a case when these remote processors has to run under KVM
>>
>> This is not true, KVM is a Linux hypervisor, remote processors have
>> nothing to do with KVM, please rephrase.
> 
> Thanks, perhaps something like this,
> 
> "However, when same SoC runs with KVM configuration, remoteproc IOMMU
> translation needs to be set from Linux host running remoteproc PAS
> driver"

Thanks but I still don't see what KVM has to do here, KVM is an an optional
Linux kernel feature, Linux can be configured without KVM and still perfectly
startup those remoteprocs.

Neil

> 
> -Mukesh
>
Re: [PATCH 3/6] remoteproc: qcom: Add helper function to support IOMMU devmem translation
Posted by Konrad Dybcio 1 month, 1 week ago
On 10.10.2024 8:59 AM, neil.armstrong@linaro.org wrote:
> Hi,
> 
> On 07/10/2024 16:37, Mukesh Ojha wrote:
>> On Mon, Oct 07, 2024 at 10:08:16AM +0200, neil.armstrong@linaro.org wrote:
>>> On 04/10/2024 23:23, Mukesh Ojha wrote:
>>>> From: Shiraz Hashim <quic_shashim@quicinc.com>
>>>>
>>>> Qualcomm SoCs runnning with Qualcomm EL2 hypervisor(QHEE), IOMMU
>>>> translation set up for remote processors is managed by QHEE itself
>>>> however, for a case when these remote processors has to run under KVM
>>>
>>> This is not true, KVM is a Linux hypervisor, remote processors have
>>> nothing to do with KVM, please rephrase.
>>
>> Thanks, perhaps something like this,
>>
>> "However, when same SoC runs with KVM configuration, remoteproc IOMMU
>> translation needs to be set from Linux host running remoteproc PAS
>> driver"
> 
> Thanks but I still don't see what KVM has to do here, KVM is an an optional
> Linux kernel feature, Linux can be configured without KVM and still perfectly
> startup those remoteprocs.

Mukesh, KVM is a very specific use case. What you're referring to is
really "no QHEE / Gunyah". We can do s/KVM/Hyper-V (or almost any
other software running at EL2) and your claims still hold.

Konrad