On 01/10/2019 23:02, khol...@gmail.com wrote:
From: AngeloGioacchino Del Regno <khol...@gmail.com>

As specified in this driver, the context banks are 0x1000 apart.
Problem is that sometimes the context number (our asid) does not
match this logic and we end up using the wrong one: this starts
being a problem in the case that we need to send TZ commands
to do anything on a specific context.

For this reason, read the ASID from the DT if the property
"qcom,ctx-num" is present on the IOMMU context node.

Can you clarify exactly what the situation is here? Is the hypervisor broken in that it presents context banks at made-up offsets and fails to correctly associate those offsets with the real underlying hardware context in some cases? Or is the hypervisor actually intercepting TTBR/CONTEXTIDR/TLBI* writes such that we don't actually have control over the ASID used by a given context bank (which I would also consider broken)? Or is "context number" a specific thing in the SCM interface which represents neither of the above?

At the moment, it seems like a bit too much is being conflated here just because of some implicit behaviour in the driver.

Robin.

Signed-off-by: AngeloGioacchino Del Regno <khol...@gmail.com>
---
  .../devicetree/bindings/iommu/qcom,iommu.txt    |  1 +
  drivers/iommu/qcom_iommu.c                      | 17 ++++++++++++++---
  2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/iommu/qcom,iommu.txt 
b/Documentation/devicetree/bindings/iommu/qcom,iommu.txt
index 059139abce35..ba0b77889f02 100644
--- a/Documentation/devicetree/bindings/iommu/qcom,iommu.txt
+++ b/Documentation/devicetree/bindings/iommu/qcom,iommu.txt
@@ -46,6 +46,7 @@ to non-secure vs secure interrupt line.
                       for routing of context bank irq's to secure vs non-
                       secure lines.  (Ie. if the iommu contains secure
                       context banks)
+- qcom,ctx-num     : The number associated to the context bank
** Examples:
diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
index c31e7bc4ccbe..504ca69adc5a 100644
--- a/drivers/iommu/qcom_iommu.c
+++ b/drivers/iommu/qcom_iommu.c
@@ -585,7 +585,8 @@ static int qcom_iommu_of_xlate(struct device *dev, struct 
of_phandle_args *args)
         * index into qcom_iommu->ctxs:
         */
        if (WARN_ON(asid < 1) ||
-           WARN_ON(asid > qcom_iommu->num_ctxs))
+           WARN_ON(asid > qcom_iommu->num_ctxs) ||
+           WARN_ON(qcom_iommu->ctxs[asid - 1] == NULL))
                return -EINVAL;
if (!fwspec->iommu_priv) {
@@ -693,7 +694,8 @@ static int qcom_iommu_sec_ptbl_init(struct device *dev)
static int get_asid(const struct device_node *np)
  {
-       u32 reg;
+       u32 reg, val;
+       int asid;
/* read the "reg" property directly to get the relative address
         * of the context bank, and calculate the asid from that:
@@ -701,7 +703,16 @@ static int get_asid(const struct device_node *np)
        if (of_property_read_u32_index(np, "reg", 0, &reg))
                return -ENODEV;
- return reg / 0x1000; /* context banks are 0x1000 apart */
+       /* Context banks are 0x1000 apart but, in some cases, the ASID
+        * number doesn't match to this logic and needs to be passed
+        * from the DT configuration explicitly.
+        */
+       if (of_property_read_u32(np, "qcom,ctx-num", &val))
+               asid = reg / 0x1000;
+       else
+               asid = val;
+
+       return asid;
  }
static int qcom_iommu_ctx_probe(struct platform_device *pdev)

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to