These patches clean up a leftover duplicate assignment following the
initializer that already set the same values; followed by introducing
two extra SCM calls that allow setting the maximum IOMMU pool size and
changing the pagetable addressing mode to AArch32 LPAE or AArch64
(per-context).
AngeloGioacchino Del Regno (2):
firmware: qcom: scm: Add function to set the maximum IOMMU pool size
firmware: qcom: scm: Add function to set IOMMU pagetable addressing
Marijn Suijten (1):
firmware: qcom: scm: Remove reassignment to desc following initializer
drivers/firmware/qcom_scm.c | 37 +++++++++++++++++++++++++++++++------
drivers/firmware/qcom_scm.h | 2 ++
include/linux/qcom_scm.h | 2 ++
3 files changed, 35 insertions(+), 6 deletions(-)
base-commit: fa55b7dcdc43c1aa1ba12bca9d2dd4318c2a0dbf
--
2.34.1
From: AngeloGioacchino Del Regno <[email protected]>
This is not necessary for basic functionality of the IOMMU, but
it's an optimization that tells to the TZ what's the maximum
mappable size for the secure IOMMUs, so that it can optimize
the data structures in the TZ itself.
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
[Marijn: ported from 5.3 to the unified architecture in 5.11]
Signed-off-by: Marijn Suijten <[email protected]>
Reviewed-by: Konrad Dybcio <[email protected]>
---
drivers/firmware/qcom_scm.c | 15 +++++++++++++++
drivers/firmware/qcom_scm.h | 1 +
include/linux/qcom_scm.h | 1 +
3 files changed, 17 insertions(+)
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 3f67bf774821..d5a9ba15e2ba 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -759,6 +759,21 @@ int qcom_scm_iommu_secure_ptbl_init(u64 addr, u32 size, u32 spare)
}
EXPORT_SYMBOL(qcom_scm_iommu_secure_ptbl_init);
+int qcom_scm_iommu_set_cp_pool_size(u32 spare, u32 size)
+{
+ struct qcom_scm_desc desc = {
+ .svc = QCOM_SCM_SVC_MP,
+ .cmd = QCOM_SCM_MP_IOMMU_SET_CP_POOL_SIZE,
+ .arginfo = QCOM_SCM_ARGS(2),
+ .args[0] = size,
+ .args[1] = spare,
+ .owner = ARM_SMCCC_OWNER_SIP,
+ };
+
+ return qcom_scm_call(__scm->dev, &desc, NULL);
+}
+EXPORT_SYMBOL(qcom_scm_iommu_set_cp_pool_size);
+
int qcom_scm_mem_protect_video_var(u32 cp_start, u32 cp_size,
u32 cp_nonpixel_start,
u32 cp_nonpixel_size)
diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
index d92156ceb3ac..bb627941702b 100644
--- a/drivers/firmware/qcom_scm.h
+++ b/drivers/firmware/qcom_scm.h
@@ -100,6 +100,7 @@ extern int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc,
#define QCOM_SCM_MP_RESTORE_SEC_CFG 0x02
#define QCOM_SCM_MP_IOMMU_SECURE_PTBL_SIZE 0x03
#define QCOM_SCM_MP_IOMMU_SECURE_PTBL_INIT 0x04
+#define QCOM_SCM_MP_IOMMU_SET_CP_POOL_SIZE 0x05
#define QCOM_SCM_MP_VIDEO_VAR 0x08
#define QCOM_SCM_MP_ASSIGN 0x16
diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
index 81cad9e1e412..8a065f8660c1 100644
--- a/include/linux/qcom_scm.h
+++ b/include/linux/qcom_scm.h
@@ -83,6 +83,7 @@ extern bool qcom_scm_restore_sec_cfg_available(void);
extern int qcom_scm_restore_sec_cfg(u32 device_id, u32 spare);
extern int qcom_scm_iommu_secure_ptbl_size(u32 spare, size_t *size);
extern int qcom_scm_iommu_secure_ptbl_init(u64 addr, u32 size, u32 spare);
+extern int qcom_scm_iommu_set_cp_pool_size(u32 spare, u32 size);
extern int qcom_scm_mem_protect_video_var(u32 cp_start, u32 cp_size,
u32 cp_nonpixel_start,
u32 cp_nonpixel_size);
--
2.34.1
From: AngeloGioacchino Del Regno <[email protected]>
Add a function to change the IOMMU pagetable addressing to
AArch32 LPAE or AArch64. If doing that, then this must be
done for each IOMMU context (not necessarily at the same time).
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
[Marijn: ported from 5.3 to the unified architecture in 5.11]
Signed-off-by: Marijn Suijten <[email protected]>
Reviewed-by: Konrad Dybcio <[email protected]>
---
drivers/firmware/qcom_scm.c | 16 ++++++++++++++++
drivers/firmware/qcom_scm.h | 1 +
include/linux/qcom_scm.h | 1 +
3 files changed, 18 insertions(+)
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index d5a9ba15e2ba..6f7096120023 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -1140,6 +1140,22 @@ int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
}
EXPORT_SYMBOL(qcom_scm_hdcp_req);
+int qcom_scm_iommu_set_pt_format(u32 sec_id, u32 ctx_num, u32 pt_fmt)
+{
+ struct qcom_scm_desc desc = {
+ .svc = QCOM_SCM_SVC_SMMU_PROGRAM,
+ .cmd = QCOM_SCM_SMMU_PT_FORMAT,
+ .arginfo = QCOM_SCM_ARGS(3),
+ .args[0] = sec_id,
+ .args[1] = ctx_num,
+ .args[2] = pt_fmt, /* 0: LPAE AArch32 - 1: AArch64 */
+ .owner = ARM_SMCCC_OWNER_SIP,
+ };
+
+ return qcom_scm_call(__scm->dev, &desc, NULL);
+}
+EXPORT_SYMBOL(qcom_scm_iommu_set_pt_format);
+
int qcom_scm_qsmmu500_wait_safe_toggle(bool en)
{
struct qcom_scm_desc desc = {
diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
index bb627941702b..a348f2c214e5 100644
--- a/drivers/firmware/qcom_scm.h
+++ b/drivers/firmware/qcom_scm.h
@@ -120,6 +120,7 @@ extern int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc,
#define QCOM_SCM_LMH_LIMIT_DCVSH 0x10
#define QCOM_SCM_SVC_SMMU_PROGRAM 0x15
+#define QCOM_SCM_SMMU_PT_FORMAT 0x01
#define QCOM_SCM_SMMU_CONFIG_ERRATA1 0x03
#define QCOM_SCM_SMMU_CONFIG_ERRATA1_CLIENT_ALL 0x02
diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
index 8a065f8660c1..ca4a88d7cbdc 100644
--- a/include/linux/qcom_scm.h
+++ b/include/linux/qcom_scm.h
@@ -108,6 +108,7 @@ 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 int qcom_scm_iommu_set_pt_format(u32 sec_id, u32 ctx_num, u32 pt_fmt);
extern int qcom_scm_qsmmu500_wait_safe_toggle(bool en);
extern int qcom_scm_lmh_dcvsh(u32 payload_fn, u32 payload_reg, u32 payload_val,
--
2.34.1
Member assignments to qcom_scm_desc were moved into struct initializers
in 57d3b816718c ("firmware: qcom_scm: Remove thin wrappers") including
the case in qcom_scm_iommu_secure_ptbl_init, except that the - now
duplicate - assignment to desc was left in place. While not harmful,
remove this unnecessary extra reassignment.
Fixes: 57d3b816718c ("firmware: qcom_scm: Remove thin wrappers")
Signed-off-by: Marijn Suijten <[email protected]>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/firmware/qcom_scm.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 7db8066b19fd..3f67bf774821 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -749,12 +749,6 @@ int qcom_scm_iommu_secure_ptbl_init(u64 addr, u32 size, u32 spare)
};
int ret;
- desc.args[0] = addr;
- desc.args[1] = size;
- desc.args[2] = spare;
- desc.arginfo = QCOM_SCM_ARGS(3, QCOM_SCM_RW, QCOM_SCM_VAL,
- QCOM_SCM_VAL);
-
ret = qcom_scm_call(__scm->dev, &desc, NULL);
/* the pg table has been initialized already, ignore the error */
--
2.34.1
On 12/8/21 2:34 AM, Marijn Suijten wrote:
> Member assignments to qcom_scm_desc were moved into struct initializers
> in 57d3b816718c ("firmware: qcom_scm: Remove thin wrappers") including
> the case in qcom_scm_iommu_secure_ptbl_init, except that the - now
> duplicate - assignment to desc was left in place. While not harmful,
> remove this unnecessary extra reassignment.
>
> Fixes: 57d3b816718c ("firmware: qcom_scm: Remove thin wrappers")
> Signed-off-by: Marijn Suijten <[email protected]>
> Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
Looks good.
Reviewed-by: Alex Elder <[email protected]>
> ---
> drivers/firmware/qcom_scm.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> index 7db8066b19fd..3f67bf774821 100644
> --- a/drivers/firmware/qcom_scm.c
> +++ b/drivers/firmware/qcom_scm.c
> @@ -749,12 +749,6 @@ int qcom_scm_iommu_secure_ptbl_init(u64 addr, u32 size, u32 spare)
> };
> int ret;
>
> - desc.args[0] = addr;
> - desc.args[1] = size;
> - desc.args[2] = spare;
> - desc.arginfo = QCOM_SCM_ARGS(3, QCOM_SCM_RW, QCOM_SCM_VAL,
> - QCOM_SCM_VAL);
> -
> ret = qcom_scm_call(__scm->dev, &desc, NULL);
>
> /* the pg table has been initialized already, ignore the error */
>
On 12/8/21 2:34 AM, Marijn Suijten wrote:
> From: AngeloGioacchino Del Regno <[email protected]>
>
> This is not necessary for basic functionality of the IOMMU, but
> it's an optimization that tells to the TZ what's the maximum
> mappable size for the secure IOMMUs, so that it can optimize
> the data structures in the TZ itself.
Are there no users of this function? -Alex
>
> Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
> [Marijn: ported from 5.3 to the unified architecture in 5.11]
> Signed-off-by: Marijn Suijten <[email protected]>
> Reviewed-by: Konrad Dybcio <[email protected]>
> ---
> drivers/firmware/qcom_scm.c | 15 +++++++++++++++
> drivers/firmware/qcom_scm.h | 1 +
> include/linux/qcom_scm.h | 1 +
> 3 files changed, 17 insertions(+)
>
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> index 3f67bf774821..d5a9ba15e2ba 100644
> --- a/drivers/firmware/qcom_scm.c
> +++ b/drivers/firmware/qcom_scm.c
> @@ -759,6 +759,21 @@ int qcom_scm_iommu_secure_ptbl_init(u64 addr, u32 size, u32 spare)
> }
> EXPORT_SYMBOL(qcom_scm_iommu_secure_ptbl_init);
>
> +int qcom_scm_iommu_set_cp_pool_size(u32 spare, u32 size)
> +{
> + struct qcom_scm_desc desc = {
> + .svc = QCOM_SCM_SVC_MP,
> + .cmd = QCOM_SCM_MP_IOMMU_SET_CP_POOL_SIZE,
> + .arginfo = QCOM_SCM_ARGS(2),
> + .args[0] = size,
> + .args[1] = spare,
> + .owner = ARM_SMCCC_OWNER_SIP,
> + };
> +
> + return qcom_scm_call(__scm->dev, &desc, NULL);
> +}
> +EXPORT_SYMBOL(qcom_scm_iommu_set_cp_pool_size);
> +
> int qcom_scm_mem_protect_video_var(u32 cp_start, u32 cp_size,
> u32 cp_nonpixel_start,
> u32 cp_nonpixel_size)
> diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
> index d92156ceb3ac..bb627941702b 100644
> --- a/drivers/firmware/qcom_scm.h
> +++ b/drivers/firmware/qcom_scm.h
> @@ -100,6 +100,7 @@ extern int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc,
> #define QCOM_SCM_MP_RESTORE_SEC_CFG 0x02
> #define QCOM_SCM_MP_IOMMU_SECURE_PTBL_SIZE 0x03
> #define QCOM_SCM_MP_IOMMU_SECURE_PTBL_INIT 0x04
> +#define QCOM_SCM_MP_IOMMU_SET_CP_POOL_SIZE 0x05
> #define QCOM_SCM_MP_VIDEO_VAR 0x08
> #define QCOM_SCM_MP_ASSIGN 0x16
>
> diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
> index 81cad9e1e412..8a065f8660c1 100644
> --- a/include/linux/qcom_scm.h
> +++ b/include/linux/qcom_scm.h
> @@ -83,6 +83,7 @@ extern bool qcom_scm_restore_sec_cfg_available(void);
> extern int qcom_scm_restore_sec_cfg(u32 device_id, u32 spare);
> extern int qcom_scm_iommu_secure_ptbl_size(u32 spare, size_t *size);
> extern int qcom_scm_iommu_secure_ptbl_init(u64 addr, u32 size, u32 spare);
> +extern int qcom_scm_iommu_set_cp_pool_size(u32 spare, u32 size);
> extern int qcom_scm_mem_protect_video_var(u32 cp_start, u32 cp_size,
> u32 cp_nonpixel_start,
> u32 cp_nonpixel_size);
>
On 12/8/21 2:34 AM, Marijn Suijten wrote:
> From: AngeloGioacchino Del Regno <[email protected]>
>
> Add a function to change the IOMMU pagetable addressing to
> AArch32 LPAE or AArch64. If doing that, then this must be
> done for each IOMMU context (not necessarily at the same time).
>
> Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
> [Marijn: ported from 5.3 to the unified architecture in 5.11]
> Signed-off-by: Marijn Suijten <[email protected]>
> Reviewed-by: Konrad Dybcio <[email protected]>
Are there no users of this function? -Alex
> ---
> drivers/firmware/qcom_scm.c | 16 ++++++++++++++++
> drivers/firmware/qcom_scm.h | 1 +
> include/linux/qcom_scm.h | 1 +
> 3 files changed, 18 insertions(+)
>
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> index d5a9ba15e2ba..6f7096120023 100644
> --- a/drivers/firmware/qcom_scm.c
> +++ b/drivers/firmware/qcom_scm.c
> @@ -1140,6 +1140,22 @@ int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
> }
> EXPORT_SYMBOL(qcom_scm_hdcp_req);
>
> +int qcom_scm_iommu_set_pt_format(u32 sec_id, u32 ctx_num, u32 pt_fmt)
> +{
> + struct qcom_scm_desc desc = {
> + .svc = QCOM_SCM_SVC_SMMU_PROGRAM,
> + .cmd = QCOM_SCM_SMMU_PT_FORMAT,
> + .arginfo = QCOM_SCM_ARGS(3),
> + .args[0] = sec_id,
> + .args[1] = ctx_num,
> + .args[2] = pt_fmt, /* 0: LPAE AArch32 - 1: AArch64 */
> + .owner = ARM_SMCCC_OWNER_SIP,
> + };
> +
> + return qcom_scm_call(__scm->dev, &desc, NULL);
> +}
> +EXPORT_SYMBOL(qcom_scm_iommu_set_pt_format);
> +
> int qcom_scm_qsmmu500_wait_safe_toggle(bool en)
> {
> struct qcom_scm_desc desc = {
> diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
> index bb627941702b..a348f2c214e5 100644
> --- a/drivers/firmware/qcom_scm.h
> +++ b/drivers/firmware/qcom_scm.h
> @@ -120,6 +120,7 @@ extern int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc,
> #define QCOM_SCM_LMH_LIMIT_DCVSH 0x10
>
> #define QCOM_SCM_SVC_SMMU_PROGRAM 0x15
> +#define QCOM_SCM_SMMU_PT_FORMAT 0x01
> #define QCOM_SCM_SMMU_CONFIG_ERRATA1 0x03
> #define QCOM_SCM_SMMU_CONFIG_ERRATA1_CLIENT_ALL 0x02
>
> diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
> index 8a065f8660c1..ca4a88d7cbdc 100644
> --- a/include/linux/qcom_scm.h
> +++ b/include/linux/qcom_scm.h
> @@ -108,6 +108,7 @@ 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 int qcom_scm_iommu_set_pt_format(u32 sec_id, u32 ctx_num, u32 pt_fmt);
> extern int qcom_scm_qsmmu500_wait_safe_toggle(bool en);
>
> extern int qcom_scm_lmh_dcvsh(u32 payload_fn, u32 payload_reg, u32 payload_val,
>
On 2021-12-08 07:30:28, Alex Elder wrote:
> On 12/8/21 2:34 AM, Marijn Suijten wrote:
> > From: AngeloGioacchino Del Regno <[email protected]>
> >
> > This is not necessary for basic functionality of the IOMMU, but
> > it's an optimization that tells to the TZ what's the maximum
> > mappable size for the secure IOMMUs, so that it can optimize
> > the data structures in the TZ itself.
>
> Are there no users of this function? -Alex
I should have probably mentioned in the cover letter that this function
and the one introduced in patch 3/3 are going to be used in upcoming
patches for IOMMUs found in msm8976, msm8974 and related SoCs (with the
side-note that I don't see this particular set_cp_pool_size used in the
branch that this was submitted from, but it's most likely used elsewhere
or planned ahead to be used in the near future - I expect Angelo to be
able to comment on that more accurately).
> >
> > Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
> > [Marijn: ported from 5.3 to the unified architecture in 5.11]
> > Signed-off-by: Marijn Suijten <[email protected]>
> > Reviewed-by: Konrad Dybcio <[email protected]>
> > ---
> > drivers/firmware/qcom_scm.c | 15 +++++++++++++++
> > drivers/firmware/qcom_scm.h | 1 +
> > include/linux/qcom_scm.h | 1 +
> > 3 files changed, 17 insertions(+)
> >
> > diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> > index 3f67bf774821..d5a9ba15e2ba 100644
> > --- a/drivers/firmware/qcom_scm.c
> > +++ b/drivers/firmware/qcom_scm.c
> > @@ -759,6 +759,21 @@ int qcom_scm_iommu_secure_ptbl_init(u64 addr, u32 size, u32 spare)
> > }
> > EXPORT_SYMBOL(qcom_scm_iommu_secure_ptbl_init);
> >
> > +int qcom_scm_iommu_set_cp_pool_size(u32 spare, u32 size)
> > +{
> > + struct qcom_scm_desc desc = {
> > + .svc = QCOM_SCM_SVC_MP,
> > + .cmd = QCOM_SCM_MP_IOMMU_SET_CP_POOL_SIZE,
> > + .arginfo = QCOM_SCM_ARGS(2),
> > + .args[0] = size,
> > + .args[1] = spare,
> > + .owner = ARM_SMCCC_OWNER_SIP,
> > + };
> > +
> > + return qcom_scm_call(__scm->dev, &desc, NULL);
> > +}
> > +EXPORT_SYMBOL(qcom_scm_iommu_set_cp_pool_size);
> > +
> > int qcom_scm_mem_protect_video_var(u32 cp_start, u32 cp_size,
> > u32 cp_nonpixel_start,
> > u32 cp_nonpixel_size)
> > diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
> > index d92156ceb3ac..bb627941702b 100644
> > --- a/drivers/firmware/qcom_scm.h
> > +++ b/drivers/firmware/qcom_scm.h
> > @@ -100,6 +100,7 @@ extern int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc,
> > #define QCOM_SCM_MP_RESTORE_SEC_CFG 0x02
> > #define QCOM_SCM_MP_IOMMU_SECURE_PTBL_SIZE 0x03
> > #define QCOM_SCM_MP_IOMMU_SECURE_PTBL_INIT 0x04
> > +#define QCOM_SCM_MP_IOMMU_SET_CP_POOL_SIZE 0x05
> > #define QCOM_SCM_MP_VIDEO_VAR 0x08
> > #define QCOM_SCM_MP_ASSIGN 0x16
> >
> > diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
> > index 81cad9e1e412..8a065f8660c1 100644
> > --- a/include/linux/qcom_scm.h
> > +++ b/include/linux/qcom_scm.h
> > @@ -83,6 +83,7 @@ extern bool qcom_scm_restore_sec_cfg_available(void);
> > extern int qcom_scm_restore_sec_cfg(u32 device_id, u32 spare);
> > extern int qcom_scm_iommu_secure_ptbl_size(u32 spare, size_t *size);
> > extern int qcom_scm_iommu_secure_ptbl_init(u64 addr, u32 size, u32 spare);
> > +extern int qcom_scm_iommu_set_cp_pool_size(u32 spare, u32 size);
> > extern int qcom_scm_mem_protect_video_var(u32 cp_start, u32 cp_size,
> > u32 cp_nonpixel_start,
> > u32 cp_nonpixel_size);
> >
>
Il 09/12/21 00:44, Marijn Suijten ha scritto:
> On 2021-12-08 07:30:28, Alex Elder wrote:
>> On 12/8/21 2:34 AM, Marijn Suijten wrote:
>>> From: AngeloGioacchino Del Regno <[email protected]>
>>>
>>> This is not necessary for basic functionality of the IOMMU, but
>>> it's an optimization that tells to the TZ what's the maximum
>>> mappable size for the secure IOMMUs, so that it can optimize
>>> the data structures in the TZ itself.
>>
>> Are there no users of this function? -Alex
>
> I should have probably mentioned in the cover letter that this function
> and the one introduced in patch 3/3 are going to be used in upcoming
> patches for IOMMUs found in msm8976, msm8974 and related SoCs (with the
> side-note that I don't see this particular set_cp_pool_size used in the
> branch that this was submitted from, but it's most likely used elsewhere
> or planned ahead to be used in the near future - I expect Angelo to be
> able to comment on that more accurately).
>
This function is used in the secured iommu pagetable setup, but not for
all of the "SCM feature versions" (only for version >= 1.1.1, downstream
reads it with a call to scm_feat_version()).
It's not strictly necessary for functionality, hence why Marijn isn't
seeing any call to this in the branch that he was browsing: the spirit here
is to first introduce code that does a minimal (but, of course, working)
setup of the IOMMUs found in MSM8956/76 (which can be adapted with very
minimal changes to other SoCs), and *only then* to add these performance
enhancements to the mix.
...and this is why this commit is here :))
By the way, if my memory isn't failing me, that SCM call should be usable
on all AArch64 SoCs (so, 8956, 8994 and the rest).
On 12/10/21 10:28 AM, AngeloGioacchino Del Regno wrote:
> Il 09/12/21 00:44, Marijn Suijten ha scritto:
>> On 2021-12-08 07:30:28, Alex Elder wrote:
>>> On 12/8/21 2:34 AM, Marijn Suijten wrote:
>>>> From: AngeloGioacchino Del Regno
>>>> <[email protected]>
>>>>
>>>> This is not necessary for basic functionality of the IOMMU, but
>>>> it's an optimization that tells to the TZ what's the maximum
>>>> mappable size for the secure IOMMUs, so that it can optimize
>>>> the data structures in the TZ itself.
>>>
>>> Are there no users of this function? -Alex
>>
>> I should have probably mentioned in the cover letter that this function
>> and the one introduced in patch 3/3 are going to be used in upcoming
>> patches for IOMMUs found in msm8976, msm8974 and related SoCs (with the
>> side-note that I don't see this particular set_cp_pool_size used in the
>> branch that this was submitted from, but it's most likely used elsewhere
>> or planned ahead to be used in the near future - I expect Angelo to be
>> able to comment on that more accurately).
>>
>
> This function is used in the secured iommu pagetable setup, but not for
> all of the "SCM feature versions" (only for version >= 1.1.1, downstream
> reads it with a call to scm_feat_version()).
>
> It's not strictly necessary for functionality, hence why Marijn isn't
> seeing any call to this in the branch that he was browsing: the spirit here
> is to first introduce code that does a minimal (but, of course, working)
> setup of the IOMMUs found in MSM8956/76 (which can be adapted with very
> minimal changes to other SoCs), and *only then* to add these performance
> enhancements to the mix.
>
> ...and this is why this commit is here :))
>
> By the way, if my memory isn't failing me, that SCM call should be usable
> on all AArch64 SoCs (so, 8956, 8994 and the rest).
Thanks for your responses. I don't actually know what
you use as downstream for this. If I did, I might
compare what I see there to what you do and confirm
it does the same thing...
So I'm not offering any review on this or patch 3.
But I'm glad to know the users of these functions
will be coming soon.
-Alex
On Wed, 8 Dec 2021 09:34:20 +0100, Marijn Suijten wrote:
> These patches clean up a leftover duplicate assignment following the
> initializer that already set the same values; followed by introducing
> two extra SCM calls that allow setting the maximum IOMMU pool size and
> changing the pagetable addressing mode to AArch32 LPAE or AArch64
> (per-context).
>
> AngeloGioacchino Del Regno (2):
> firmware: qcom: scm: Add function to set the maximum IOMMU pool size
> firmware: qcom: scm: Add function to set IOMMU pagetable addressing
>
> [...]
Applied, thanks!
[1/3] firmware: qcom: scm: Remove reassignment to desc following initializer
commit: 7823e5aa5d1dd9ed5849923c165eb8f29ad23c54
[2/3] firmware: qcom: scm: Add function to set the maximum IOMMU pool size
commit: 943515090ec67f81f6f93febfddb8c9118357e97
[3/3] firmware: qcom: scm: Add function to set IOMMU pagetable addressing
commit: 071a13332de894cb3c38b17c82350f1e4167c023
Best regards,
--
Bjorn Andersson <[email protected]>