2022-11-09 14:52:51

by Sumit Gupta

[permalink] [raw]
Subject: [Patch 1/4] soc/tegra: cbb: use correct master_id mask for cbb noc in Tegra194

In Tegra194 SoC, master_id bit range is different between
cluster NOC and CBB NOC. Currently same bit range is used
which results in wrong master_id value. Due to this,
illegal accesses from the CCPLEX master do not result in a
crash as expected. Fix this by using the correct range for
the CBB NOC.
Finally, it is only necessary to extract the master_id when
the erd_mask_inband_err flag is set because when this is not
set, a crash is always triggered.

Fixes: b71344221466 ("soc/tegra: cbb: Add CBB 1.0 driver for Tegra194")
Fixes: fc2f151d2314 ("soc/tegra: cbb: Add driver for Tegra234 CBB 2.0")
Signed-off-by: Sumit Gupta <[email protected]>
---
drivers/soc/tegra/cbb/tegra194-cbb.c | 14 +++++++-------
drivers/soc/tegra/cbb/tegra234-cbb.c | 13 ++++++-------
2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/soc/tegra/cbb/tegra194-cbb.c b/drivers/soc/tegra/cbb/tegra194-cbb.c
index 1ae0bd9a1ac1..2e952c6f7c9e 100644
--- a/drivers/soc/tegra/cbb/tegra194-cbb.c
+++ b/drivers/soc/tegra/cbb/tegra194-cbb.c
@@ -102,8 +102,6 @@
#define CLUSTER_NOC_VQC GENMASK(17, 16)
#define CLUSTER_NOC_MSTR_ID GENMASK(21, 18)

-#define USRBITS_MSTR_ID GENMASK(21, 18)
-
#define CBB_ERR_OPC GENMASK(4, 1)
#define CBB_ERR_ERRCODE GENMASK(10, 8)
#define CBB_ERR_LEN1 GENMASK(27, 16)
@@ -2038,15 +2036,17 @@ static irqreturn_t tegra194_cbb_err_isr(int irq, void *data)
smp_processor_id(), priv->noc->name, priv->res->start,
irq);

- mstr_id = FIELD_GET(USRBITS_MSTR_ID, priv->errlog5) - 1;
is_fatal = print_errlog(NULL, priv, status);

/*
- * If illegal request is from CCPLEX(0x1)
- * initiator then call BUG() to crash system.
+ * If illegal request is from CCPLEX(0x1) initiator
+ * and error is fatal then call BUG() to crash system.
*/
- if ((mstr_id == 0x1) && priv->noc->erd_mask_inband_err)
- is_inband_err = 1;
+ if (priv->noc->erd_mask_inband_err) {
+ mstr_id = FIELD_GET(CBB_NOC_MSTR_ID, priv->errlog5);
+ if (mstr_id == 0x1)
+ is_inband_err = 1;
+ }
}
}

diff --git a/drivers/soc/tegra/cbb/tegra234-cbb.c b/drivers/soc/tegra/cbb/tegra234-cbb.c
index 3528f9e15d5c..654c3d164606 100644
--- a/drivers/soc/tegra/cbb/tegra234-cbb.c
+++ b/drivers/soc/tegra/cbb/tegra234-cbb.c
@@ -92,7 +92,6 @@ struct tegra234_slave_lookup {
struct tegra234_cbb_fabric {
const char *name;
phys_addr_t off_mask_erd;
- bool erd_mask_inband_err;
const char * const *master_id;
unsigned int notifier_offset;
const struct tegra_cbb_error *errors;
@@ -525,14 +524,14 @@ static irqreturn_t tegra234_cbb_isr(int irq, void *data)
if (err)
goto unlock;

- mstr_id = FIELD_GET(USRBITS_MSTR_ID, priv->mn_user_bits);
-
/*
- * If illegal request is from CCPLEX(id:0x1) master then call BUG() to
- * crash system.
+ * If illegal request is from CCPLEX(id:0x1) master then call WARN()
*/
- if ((mstr_id == 0x1) && priv->fabric->off_mask_erd)
- is_inband_err = 1;
+ if (priv->fabric->off_mask_erd) {
+ mstr_id = FIELD_GET(USRBITS_MSTR_ID, priv->mn_user_bits);
+ if (mstr_id == 0x1)
+ is_inband_err = 1;
+ }
}
}

--
2.17.1



2022-11-09 14:53:00

by Sumit Gupta

[permalink] [raw]
Subject: [Patch 4/4] soc/tegra: cbb: check firewall before enabling error reporting

To enable error reporting for a fabric to CCPLEX, we need to write
its register for enabling error interrupt to CCPLEX during boot and
later clear the error status register after error occurs. If a fabric's
registers are protected and not accessible from CCPLEX, then accessing
the registers will cause CBB firewall error.
Add support to check whether write access from CCPLEX to the registers
of a fabric is not blocked by it's firewall before enabling error reporting
to CCPLEX for that fabric.

Fixes: fc2f151d2314 ("soc/tegra: cbb: Add driver for Tegra234 CBB 2.0")
Signed-off-by: Sumit Gupta <[email protected]>
---
drivers/soc/tegra/cbb/tegra234-cbb.c | 83 +++++++++++++++++++++++++++-
1 file changed, 81 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/tegra/cbb/tegra234-cbb.c b/drivers/soc/tegra/cbb/tegra234-cbb.c
index 0fab9e21d677..f33d094e5ea6 100644
--- a/drivers/soc/tegra/cbb/tegra234-cbb.c
+++ b/drivers/soc/tegra/cbb/tegra234-cbb.c
@@ -72,6 +72,11 @@

#define REQ_SOCKET_ID GENMASK(27, 24)

+#define CCPLEX_MSTRID 0x1
+#define FIREWALL_APERTURE_SZ 0x10000
+/* Write firewall check enable */
+#define WEN 0x20000
+
enum tegra234_cbb_fabric_ids {
CBB_FAB_ID,
SCE_FAB_ID,
@@ -92,6 +97,9 @@ struct tegra234_slave_lookup {
struct tegra234_cbb_fabric {
const char *name;
phys_addr_t off_mask_erd;
+ phys_addr_t firewall_base;
+ unsigned int firewall_ctl;
+ unsigned int firewall_wr_ctl;
const char * const *master_id;
unsigned int notifier_offset;
const struct tegra_cbb_error *errors;
@@ -129,6 +137,44 @@ static inline struct tegra234_cbb *to_tegra234_cbb(struct tegra_cbb *cbb)
static LIST_HEAD(cbb_list);
static DEFINE_SPINLOCK(cbb_lock);

+static bool
+tegra234_cbb_write_access_allowed(struct platform_device *pdev, struct tegra234_cbb *cbb)
+{
+ u32 val;
+
+ if (!cbb->fabric->firewall_base ||
+ !cbb->fabric->firewall_ctl ||
+ !cbb->fabric->firewall_wr_ctl) {
+ dev_info(&pdev->dev, "SoC data missing for firewall\n");
+ return false;
+ }
+
+ if ((cbb->fabric->firewall_ctl > FIREWALL_APERTURE_SZ) ||
+ (cbb->fabric->firewall_wr_ctl > FIREWALL_APERTURE_SZ)) {
+ dev_err(&pdev->dev, "wrong firewall offset value\n");
+ return false;
+ }
+
+ val = readl(cbb->regs + cbb->fabric->firewall_base + cbb->fabric->firewall_ctl);
+ /*
+ * If the firewall check feature for allowing or blocking the
+ * write accesses through the firewall of a fabric is disabled
+ * then CCPLEX can write to the registers of that fabric.
+ */
+ if (!(val & WEN))
+ return true;
+
+ /*
+ * If the firewall check is enabled then check whether CCPLEX
+ * has write access to the fabric's error notifier registers
+ */
+ val = readl(cbb->regs + cbb->fabric->firewall_base + cbb->fabric->firewall_wr_ctl);
+ if (val & (BIT(CCPLEX_MSTRID)))
+ return true;
+
+ return false;
+}
+
static void tegra234_cbb_fault_enable(struct tegra_cbb *cbb)
{
struct tegra234_cbb *priv = to_tegra234_cbb(cbb);
@@ -551,7 +597,7 @@ static irqreturn_t tegra234_cbb_isr(int irq, void *data)
*/
if (priv->fabric->off_mask_erd) {
mstr_id = FIELD_GET(USRBITS_MSTR_ID, priv->mn_user_bits);
- if (mstr_id == 0x1)
+ if (mstr_id == CCPLEX_MSTRID)
is_inband_err = 1;
}
}
@@ -665,6 +711,9 @@ static const struct tegra234_cbb_fabric tegra234_aon_fabric = {
.errors = tegra234_cbb_errors,
.max_errors = ARRAY_SIZE(tegra234_cbb_errors),
.notifier_offset = 0x17000,
+ .firewall_base = 0x30000,
+ .firewall_ctl = 0x8d0,
+ .firewall_wr_ctl = 0x8c8,
};

static const struct tegra234_slave_lookup tegra234_bpmp_slave_map[] = {
@@ -683,6 +732,9 @@ static const struct tegra234_cbb_fabric tegra234_bpmp_fabric = {
.errors = tegra234_cbb_errors,
.max_errors = ARRAY_SIZE(tegra234_cbb_errors),
.notifier_offset = 0x19000,
+ .firewall_base = 0x30000,
+ .firewall_ctl = 0x8f0,
+ .firewall_wr_ctl = 0x8e8,
};

static const struct tegra234_slave_lookup tegra234_cbb_slave_map[] = {
@@ -757,7 +809,10 @@ static const struct tegra234_cbb_fabric tegra234_cbb_fabric = {
.errors = tegra234_cbb_errors,
.max_errors = ARRAY_SIZE(tegra234_cbb_errors),
.notifier_offset = 0x60000,
- .off_mask_erd = 0x3a004
+ .off_mask_erd = 0x3a004,
+ .firewall_base = 0x10000,
+ .firewall_ctl = 0x23f0,
+ .firewall_wr_ctl = 0x23e8,
};

static const struct tegra234_slave_lookup tegra234_common_slave_map[] = {
@@ -777,6 +832,9 @@ static const struct tegra234_cbb_fabric tegra234_dce_fabric = {
.errors = tegra234_cbb_errors,
.max_errors = ARRAY_SIZE(tegra234_cbb_errors),
.notifier_offset = 0x19000,
+ .firewall_base = 0x30000,
+ .firewall_ctl = 0x290,
+ .firewall_wr_ctl = 0x288,
};

static const struct tegra234_cbb_fabric tegra234_rce_fabric = {
@@ -787,6 +845,9 @@ static const struct tegra234_cbb_fabric tegra234_rce_fabric = {
.errors = tegra234_cbb_errors,
.max_errors = ARRAY_SIZE(tegra234_cbb_errors),
.notifier_offset = 0x19000,
+ .firewall_base = 0x30000,
+ .firewall_ctl = 0x290,
+ .firewall_wr_ctl = 0x288,
};

static const struct tegra234_cbb_fabric tegra234_sce_fabric = {
@@ -797,6 +858,9 @@ static const struct tegra234_cbb_fabric tegra234_sce_fabric = {
.errors = tegra234_cbb_errors,
.max_errors = ARRAY_SIZE(tegra234_cbb_errors),
.notifier_offset = 0x19000,
+ .firewall_base = 0x30000,
+ .firewall_ctl = 0x290,
+ .firewall_wr_ctl = 0x288,
};

static const char * const tegra241_master_id[] = {
@@ -979,6 +1043,9 @@ static const struct tegra234_cbb_fabric tegra241_cbb_fabric = {
.max_errors = ARRAY_SIZE(tegra241_cbb_errors),
.notifier_offset = 0x60000,
.off_mask_erd = 0x40004,
+ .firewall_base = 0x20000,
+ .firewall_ctl = 0x2370,
+ .firewall_wr_ctl = 0x2368,
};

static const struct tegra234_slave_lookup tegra241_bpmp_slave_map[] = {
@@ -1000,6 +1067,9 @@ static const struct tegra234_cbb_fabric tegra241_bpmp_fabric = {
.errors = tegra241_cbb_errors,
.max_errors = ARRAY_SIZE(tegra241_cbb_errors),
.notifier_offset = 0x19000,
+ .firewall_base = 0x30000,
+ .firewall_ctl = 0x8f0,
+ .firewall_wr_ctl = 0x8e8,
};

static const struct of_device_id tegra234_cbb_dt_ids[] = {
@@ -1084,6 +1154,15 @@ static int tegra234_cbb_probe(struct platform_device *pdev)

platform_set_drvdata(pdev, cbb);

+ /*
+ * Don't enable error reporting for a Fabric if write to it's registers
+ * is blocked by CBB firewall.
+ */
+ if (!tegra234_cbb_write_access_allowed(pdev, cbb)) {
+ dev_info(&pdev->dev, "error reporting not enabled due to firewall\n");
+ return 0;
+ }
+
spin_lock_irqsave(&cbb_lock, flags);
list_add(&cbb->base.node, &cbb_list);
spin_unlock_irqrestore(&cbb_lock, flags);
--
2.17.1


2022-11-09 16:36:57

by Sumit Gupta

[permalink] [raw]
Subject: [Patch 2/4] soc/tegra: cbb: update slave maps for Tegra234

Updating the slave map for fabrics and using the same maps for
DCE, RCE and SCE as they all are a replica in Tegra234.

Signed-off-by: Sumit Gupta <[email protected]>
---
drivers/soc/tegra/cbb/tegra234-cbb.c | 34 +++++++++++-----------------
1 file changed, 13 insertions(+), 21 deletions(-)

diff --git a/drivers/soc/tegra/cbb/tegra234-cbb.c b/drivers/soc/tegra/cbb/tegra234-cbb.c
index 654c3d164606..04e12d9fdea5 100644
--- a/drivers/soc/tegra/cbb/tegra234-cbb.c
+++ b/drivers/soc/tegra/cbb/tegra234-cbb.c
@@ -516,7 +516,7 @@ static irqreturn_t tegra234_cbb_isr(int irq, void *data)
u32 status = tegra_cbb_get_status(cbb);

if (status && (irq == priv->sec_irq)) {
- tegra_cbb_print_err(NULL, "CPU:%d, Error: %s@%llx, irq=%d\n",
+ tegra_cbb_print_err(NULL, "CPU:%d, Error: %s@0x%llx, irq=%d\n",
smp_processor_id(), priv->fabric->name,
priv->res->start, irq);

@@ -732,48 +732,35 @@ static const struct tegra234_cbb_fabric tegra234_cbb_fabric = {
.off_mask_erd = 0x3a004
};

-static const struct tegra234_slave_lookup tegra234_dce_slave_map[] = {
+static const struct tegra234_slave_lookup tegra234_common_slave_map[] = {
{ "AXI2APB", 0x00000 },
{ "AST0", 0x15000 },
{ "AST1", 0x16000 },
+ { "CBB", 0x17000 },
+ { "RSVD", 0x00000 },
{ "CPU", 0x18000 },
};

static const struct tegra234_cbb_fabric tegra234_dce_fabric = {
.name = "dce-fabric",
.master_id = tegra234_master_id,
- .slave_map = tegra234_dce_slave_map,
+ .slave_map = tegra234_common_slave_map,
.errors = tegra234_cbb_errors,
.notifier_offset = 0x19000,
};

-static const struct tegra234_slave_lookup tegra234_rce_slave_map[] = {
- { "AXI2APB", 0x00000 },
- { "AST0", 0x15000 },
- { "AST1", 0x16000 },
- { "CPU", 0x18000 },
-};
-
static const struct tegra234_cbb_fabric tegra234_rce_fabric = {
.name = "rce-fabric",
.master_id = tegra234_master_id,
- .slave_map = tegra234_rce_slave_map,
+ .slave_map = tegra234_common_slave_map,
.errors = tegra234_cbb_errors,
.notifier_offset = 0x19000,
};

-static const struct tegra234_slave_lookup tegra234_sce_slave_map[] = {
- { "AXI2APB", 0x00000 },
- { "AST0", 0x15000 },
- { "AST1", 0x16000 },
- { "CBB", 0x17000 },
- { "CPU", 0x18000 },
-};
-
static const struct tegra234_cbb_fabric tegra234_sce_fabric = {
.name = "sce-fabric",
.master_id = tegra234_master_id,
- .slave_map = tegra234_sce_slave_map,
+ .slave_map = tegra234_common_slave_map,
.errors = tegra234_cbb_errors,
.notifier_offset = 0x19000,
};
@@ -888,7 +875,7 @@ static const struct tegra_cbb_error tegra241_cbb_errors[] = {
};

static const struct tegra234_slave_lookup tegra241_cbb_slave_map[] = {
- { "CCPLEX", 0x50000 },
+ { "RSVD", 0x00000 },
{ "PCIE_C8", 0x51000 },
{ "PCIE_C9", 0x52000 },
{ "RSVD", 0x00000 },
@@ -941,8 +928,12 @@ static const struct tegra234_slave_lookup tegra241_cbb_slave_map[] = {
{ "PCIE_C3", 0x58000 },
{ "PCIE_C0", 0x59000 },
{ "PCIE_C1", 0x5a000 },
+ { "CCPLEX", 0x50000 },
{ "AXI2APB_29", 0x85000 },
{ "AXI2APB_30", 0x86000 },
+ { "CBB_CENTRAL", 0x00000 },
+ { "AXI2APB_31", 0x8E000 },
+ { "AXI2APB_32", 0x8F000 },
};

static const struct tegra234_cbb_fabric tegra241_cbb_fabric = {
@@ -955,6 +946,7 @@ static const struct tegra234_cbb_fabric tegra241_cbb_fabric = {
};

static const struct tegra234_slave_lookup tegra241_bpmp_slave_map[] = {
+ { "RSVD", 0x00000 },
{ "RSVD", 0x00000 },
{ "RSVD", 0x00000 },
{ "CBB", 0x15000 },
--
2.17.1


2022-11-11 15:00:25

by Thierry Reding

[permalink] [raw]
Subject: Re: [Patch 1/4] soc/tegra: cbb: use correct master_id mask for cbb noc in Tegra194

From: Thierry Reding <[email protected]>

On Wed, 9 Nov 2022 19:27:14 +0530, Sumit Gupta wrote:
> In Tegra194 SoC, master_id bit range is different between
> cluster NOC and CBB NOC. Currently same bit range is used
> which results in wrong master_id value. Due to this,
> illegal accesses from the CCPLEX master do not result in a
> crash as expected. Fix this by using the correct range for
> the CBB NOC.
> Finally, it is only necessary to extract the master_id when
> the erd_mask_inband_err flag is set because when this is not
> set, a crash is always triggered.
>
> [...]

Applied, thanks!

[1/4] soc/tegra: cbb: use correct master_id mask for cbb noc in Tegra194
(no commit info)
[2/4] soc/tegra: cbb: update slave maps for Tegra234
(no commit info)
[3/4] soc/tegra: cbb: add checks for potential out of bound errors
(no commit info)
[4/4] soc/tegra: cbb: check firewall before enabling error reporting
(no commit info)

Best regards,
--
Thierry Reding <[email protected]>