| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 20 Jan 2022 15:53:23 +0300
From: Dmitry Osipenko <digetx@...il.com>
To: Ashish Mhetre <amhetre@...dia.com>, thierry.reding@...il.com,
jonathanh@...dia.com, linux-tegra@...r.kernel.org,
krzysztof.kozlowski@...onical.com, linux-kernel@...r.kernel.org
Cc: Snikam@...dia.com, vdumpa@...dia.com
Subject: Re: [Patch V2] memory: tegra: Add MC error logging on tegra186 onward
20.01.2022 12:02, Ashish Mhetre пишет:
> Remove static from tegra30_mc_handle_irq and use it as interrupt handler
> for MC interrupts on tegra186, tegra194 and tegra234 to log the errors.
> Add error specific MC status and address register bits and use them on
> tegra186, tegra194 and tegra234.
> Add error logging for generalized carveout interrupt on tegra186, tegra194
> and tegra234.
> Add error logging for route sanity interrupt on tegra194 an tegra234.
> Add register for higher bits of error address and use it on tegra194 and
> tegra234.
>
> Signed-off-by: Ashish Mhetre <amhetre@...dia.com>
> ---
> Changes in v2:
> - Updated patch subject and commit message
> - Removed separate irq handlers
> - Updated tegra30_mc_handle_irq to be used for Tegra186 onwards as well
>
> drivers/memory/tegra/mc.c | 73 ++++++++++++++++++++++++++++++++++-------
> drivers/memory/tegra/mc.h | 16 +++++++++
> drivers/memory/tegra/tegra186.c | 7 ++++
> drivers/memory/tegra/tegra194.c | 5 +++
> drivers/memory/tegra/tegra234.c | 5 +++
> 5 files changed, 94 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
> index bf3abb6..badebe8 100644
> --- a/drivers/memory/tegra/mc.c
> +++ b/drivers/memory/tegra/mc.c
> @@ -508,7 +508,21 @@ int tegra30_mc_probe(struct tegra_mc *mc)
> return 0;
> }
>
> -static irqreturn_t tegra30_mc_handle_irq(int irq, void *data)
> +const struct tegra_mc_ops tegra30_mc_ops = {
> + .probe = tegra30_mc_probe,
> + .handle_irq = tegra30_mc_handle_irq,
> +};
> +#endif
> +
> +#if defined(CONFIG_ARCH_TEGRA_3x_SOC) || \
> + defined(CONFIG_ARCH_TEGRA_114_SOC) || \
> + defined(CONFIG_ARCH_TEGRA_124_SOC) || \
> + defined(CONFIG_ARCH_TEGRA_132_SOC) || \
> + defined(CONFIG_ARCH_TEGRA_210_SOC) || \
> + defined(CONFIG_ARCH_TEGRA_186_SOC) || \
> + defined(CONFIG_ARCH_TEGRA_194_SOC) || \
> + defined(CONFIG_ARCH_TEGRA_234_SOC)
Ifdefs are unnecessary, please remove them. They are okay for
tegra30_mc_ops, which is known to be used only by specific older SoC
versions, not okay in case of newer SoCs.
> +irqreturn_t tegra30_mc_handle_irq(int irq, void *data)
> {
> struct tegra_mc *mc = data;
> unsigned long status;
> @@ -521,23 +535,64 @@ static irqreturn_t tegra30_mc_handle_irq(int irq, void *data)
>
> for_each_set_bit(bit, &status, 32) {
> const char *error = tegra_mc_status_names[bit] ?: "unknown";
> + u32 status_reg = MC_ERR_STATUS, addr_reg = MC_ERR_ADR;
> const char *client = "unknown", *desc;
> const char *direction, *secure;
> phys_addr_t addr = 0;
> + u32 addr_hi_reg = 0;
> unsigned int i;
> char perm[7];
> u8 id, type;
> u32 value;
>
> - value = mc_readl(mc, MC_ERR_STATUS);
> +#if defined(CONFIG_ARCH_TEGRA_186_SOC) || \
> + defined(CONFIG_ARCH_TEGRA_194_SOC) || \
> + defined(CONFIG_ARCH_TEGRA_234_SOC)
Please drop these ifdefs.
> + switch (bit) {
> + case MC_INT_DECERR_VPR:
> + status_reg = MC_ERR_VPR_STATUS;
> + addr_reg = MC_ERR_VPR_ADR;
I mentioned previously that VPR is supported by T124+. Hence ifdefs are
incorrect.
...
> + addr |= mc_readl(mc, addr_reg);
>
> if (value & MC_ERR_STATUS_RW)
> direction = "write";
> @@ -591,9 +646,6 @@ static irqreturn_t tegra30_mc_handle_irq(int irq, void *data)
> break;
> }
>
> - value = mc_readl(mc, MC_ERR_ADR);
Don't change the order of the code, just replace the MC_ERR_ADR here.
> - addr |= value;
> -
> dev_err_ratelimited(mc->dev, "%s: %s%s @%pa: %s (%s%s)\n",
> client, secure, direction, &addr, error,
> desc, perm);
> @@ -604,11 +656,6 @@ static irqreturn_t tegra30_mc_handle_irq(int irq, void *data)
>
> return IRQ_HANDLED;
> }
> -
> -const struct tegra_mc_ops tegra30_mc_ops = {
> - .probe = tegra30_mc_probe,
> - .handle_irq = tegra30_mc_handle_irq,
> -};
> #endif
>
> const char *const tegra_mc_status_names[32] = {
> @@ -622,6 +669,8 @@ const char *const tegra_mc_status_names[32] = {
> [12] = "VPR violation",
> [13] = "Secure carveout violation",
> [16] = "MTS carveout violation",
> + [17] = "Generalized carveout violation",
> + [20] = "Route Sanity error",
> };
>
> const char *const tegra_mc_error_names[8] = {
> diff --git a/drivers/memory/tegra/mc.h b/drivers/memory/tegra/mc.h
> index 062886e..9b1b0dc 100644
> --- a/drivers/memory/tegra/mc.h
> +++ b/drivers/memory/tegra/mc.h
> @@ -44,6 +44,8 @@
> #define MC_TIMING_CONTROL_DBG 0xf8
> #define MC_TIMING_CONTROL 0xfc
>
> +#define MC_INT_DECERR_ROUTE_SANITY BIT(20)
> +#define MC_INT_DECERR_GENERALIZED_CARVEOUT BIT(17)
> #define MC_INT_DECERR_MTS BIT(16)
> #define MC_INT_SECERR_SEC BIT(13)
> #define MC_INT_DECERR_VPR BIT(12)
> @@ -65,6 +67,18 @@
> #define MC_ERR_STATUS_SECURITY BIT(17)
> #define MC_ERR_STATUS_RW BIT(16)
>
> +#define MC_ERR_VPR_STATUS 0x654
> +#define MC_ERR_VPR_ADR 0x658
> +#define MC_ERR_SEC_STATUS 0x67c
> +#define MC_ERR_SEC_ADR 0x680
> +#define MC_ERR_MTS_STATUS 0x9b0
> +#define MC_ERR_MTS_ADR 0x9b4
> +#define MC_ERR_ROUTE_SANITY_STATUS 0x9c0
> +#define MC_ERR_ROUTE_SANITY_ADR 0x9c4
> +#define MC_ERR_GENERALIZED_CARVEOUT_STATUS 0xc00
> +#define MC_ERR_GENERALIZED_CARVEOUT_ADR 0xc04
> +#define MC_ERR_ADR_HI 0x11fc
Please put these regs right after the MC_TIMING_CONTROL. There is no
reason to separate them.
Powered by blists - more mailing lists