ACK: [SRU][F:linux-bluefield][PATCH v2 1/1] UBUNTU: SAUCE: bluefield_edac: Add SMC support
Kleber Souza
kleber.souza at canonical.com
Thu Mar 4 10:19:01 UTC 2021
On 20.02.21 06:42, Shravan Kumar Ramani wrote:
> BugLink: https://bugs.launchpad.net/ubuntu/+source/linux-bluefield/+bug/1916318
>
> This patch adds secure read/write calls to bluefield_edac. The
> ACPI table entry decides whether the secure calls need to be
> used for accessing the EMI registers.
>
> Signed-off-by: Shravan Kumar Ramani <shravankr at nvidia.com>
With the same comments as Stefan's:
Acked-by: Kleber Sacilotto de Souza <kleber.souza at canonical.com>
> ---
> drivers/edac/bluefield_edac.c | 175 ++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 158 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/edac/bluefield_edac.c b/drivers/edac/bluefield_edac.c
> index e4736eb..87ffdc1 100644
> --- a/drivers/edac/bluefield_edac.c
> +++ b/drivers/edac/bluefield_edac.c
> @@ -1,8 +1,9 @@
> -// SPDX-License-Identifier: GPL-2.0
> +// SPDX-License-Identifier: GPL-2.0 or BSD-3-Clause
> +
> /*
> * Bluefield-specific EDAC driver.
> *
> - * Copyright (c) 2019 Mellanox Technologies.
> + * Copyright (c) 2020 NVIDIA Corporation.
> */
>
> #include <linux/acpi.h>
> @@ -12,6 +13,7 @@
> #include <linux/io.h>
> #include <linux/module.h>
> #include <linux/platform_device.h>
> +#include <linux/version.h>
>
> #include "edac_module.h"
>
> @@ -47,6 +49,18 @@
> #define MLXBF_EDAC_MAX_DIMM_PER_MC 2
> #define MLXBF_EDAC_ERROR_GRAIN 8
>
> +#define MLNX_WRITE_REG_32 (0x82000009)
> +#define MLNX_READ_REG_32 (0x8200000A)
> +#define MLNX_WRITE_REG_64 (0x8200000B)
> +#define MLNX_READ_REG_64 (0x8200000C)
> +#define MLNX_SIP_SVC_UID (0x8200ff01)
> +#define MLNX_SIP_SVC_VERSION (0x8200ff03)
> +
> +#define SMCCC_ACCESS_VIOLATION (-4)
> +
> +#define MLNX_EDAC_SVC_REQ_MAJOR 0
> +#define MLNX_EDAC_SVC_MIN_MINOR 3
> +
> /*
> * Request MLNX_SIP_GET_DIMM_INFO
> *
> @@ -72,9 +86,12 @@
> #define MLXBF_DIMM_INFO__PACKAGE_X GENMASK_ULL(31, 24)
>
> struct bluefield_edac_priv {
> + struct device *dev;
> int dimm_ranks[MLXBF_EDAC_MAX_DIMM_PER_MC];
> void __iomem *emi_base;
> int dimm_per_mc;
> + bool svc_sreg_support;
> + uint32_t sreg_tbl_edac;
> };
>
> static u64 smc_call1(u64 smc_op, u64 smc_arg)
> @@ -86,6 +103,73 @@ static u64 smc_call1(u64 smc_op, u64 smc_arg)
> return res.a0;
> }
>
> +static int secure_readl(void __iomem *addr, uint32_t *result, uint32_t sreg_tbl)
> +{
> + struct arm_smccc_res res;
> + int status;
> +
> + arm_smccc_smc(MLNX_READ_REG_32, sreg_tbl, (uintptr_t) addr,
> + 0, 0, 0, 0, 0, &res);
> +
> + status = res.a0;
> +
> + switch (status) {
> + case SMCCC_RET_NOT_SUPPORTED:
> + case SMCCC_ACCESS_VIOLATION:
> + return -1;
> + default:
> + *result = (uint32_t)res.a1;
> + return 0;
> + }
> +
> +}
> +
> +static int secure_writel(void __iomem *addr, uint32_t data, uint32_t sreg_tbl)
> +{
> + struct arm_smccc_res res;
> + int status;
> +
> + arm_smccc_smc(MLNX_WRITE_REG_32, sreg_tbl, data, (uintptr_t) addr,
> + 0, 0, 0, 0, &res);
> +
> + status = res.a0;
> +
> + switch (status) {
> + case SMCCC_RET_NOT_SUPPORTED:
> + case SMCCC_ACCESS_VIOLATION:
> + return -1;
> + default:
> + return 0;
> + }
> +
> +}
> +
> +static int edac_readl(void __iomem *addr, uint32_t *result,
> + bool sreg_support, uint32_t sreg_tbl)
> +{
> + int err = 0;
> +
> + if (sreg_support)
> + err = secure_readl(addr, result, sreg_tbl);
> + else
> + *result = readl(addr);
> +
> + return err;
> +}
> +
> +static int edac_writel(void __iomem *addr, uint32_t data,
> + bool sreg_support, uint32_t sreg_tbl)
> +{
> + int err = 0;
> +
> + if (sreg_support)
> + err = secure_writel(addr, data, sreg_tbl);
> + else
> + writel(data, addr);
> +
> + return err;
> +}
> +
> /*
> * Gather the ECC information from the External Memory Interface registers
> * and report it to the edac handler.
> @@ -99,7 +183,7 @@ static void bluefield_gather_report_ecc(struct mem_ctl_info *mci,
> u32 ecc_latch_select, dram_syndrom, serr, derr, syndrom;
> enum hw_event_mc_err_type ecc_type;
> u64 ecc_dimm_addr;
> - int ecc_dimm;
> + int ecc_dimm, err;
>
> ecc_type = is_single_ecc ? HW_EVENT_ERR_CORRECTED :
> HW_EVENT_ERR_UNCORRECTED;
> @@ -109,14 +193,22 @@ static void bluefield_gather_report_ecc(struct mem_ctl_info *mci,
> * registers with information about the last ECC error occurrence.
> */
> ecc_latch_select = MLXBF_ECC_LATCH_SEL__START;
> - writel(ecc_latch_select, priv->emi_base + MLXBF_ECC_LATCH_SEL);
> + err = edac_writel(priv->emi_base + MLXBF_ECC_LATCH_SEL,
> + ecc_latch_select, priv->svc_sreg_support,
> + priv->sreg_tbl_edac);
> + if (err)
> + dev_err(priv->dev, "ECC latch select write failed.\n");
>
> /*
> * Verify that the ECC reported info in the registers is of the
> * same type as the one asked to report. If not, just report the
> * error without the detailed information.
> */
> - dram_syndrom = readl(priv->emi_base + MLXBF_SYNDROM);
> + err = edac_readl(priv->emi_base + MLXBF_SYNDROM, &dram_syndrom,
> + priv->svc_sreg_support, priv->sreg_tbl_edac);
> + if (err)
> + dev_err(priv->dev, "DRAM syndrom read failed.\n");
> +
> serr = FIELD_GET(MLXBF_SYNDROM__SERR, dram_syndrom);
> derr = FIELD_GET(MLXBF_SYNDROM__DERR, dram_syndrom);
> syndrom = FIELD_GET(MLXBF_SYNDROM__SYN, dram_syndrom);
> @@ -127,13 +219,24 @@ static void bluefield_gather_report_ecc(struct mem_ctl_info *mci,
> return;
> }
>
> - dram_additional_info = readl(priv->emi_base + MLXBF_ADD_INFO);
> + err = edac_readl(priv->emi_base + MLXBF_ADD_INFO, &dram_additional_info,
> + priv->svc_sreg_support, priv->sreg_tbl_edac);
> + if (err)
> + dev_err(priv->dev, "DRAM additional info read failed.\n");
> +
> err_prank = FIELD_GET(MLXBF_ADD_INFO__ERR_PRANK, dram_additional_info);
>
> ecc_dimm = (err_prank >= 2 && priv->dimm_ranks[0] <= 2) ? 1 : 0;
>
> - edea0 = readl(priv->emi_base + MLXBF_ERR_ADDR_0);
> - edea1 = readl(priv->emi_base + MLXBF_ERR_ADDR_1);
> + err = edac_readl(priv->emi_base + MLXBF_ERR_ADDR_0, &edea0,
> + priv->svc_sreg_support, priv->sreg_tbl_edac);
> + if (err)
> + dev_err(priv->dev, "Error addr 0 read failed.\n");
> +
> + err = edac_readl(priv->emi_base + MLXBF_ERR_ADDR_1, &edea1,
> + priv->svc_sreg_support, priv->sreg_tbl_edac);
> + if (err)
> + dev_err(priv->dev, "Error addr 1 read failed.\n");
>
> ecc_dimm_addr = ((u64)edea1 << 32) | edea0;
>
> @@ -147,6 +250,7 @@ static void bluefield_edac_check(struct mem_ctl_info *mci)
> {
> struct bluefield_edac_priv *priv = mci->pvt_info;
> u32 ecc_count, single_error_count, double_error_count, ecc_error = 0;
> + int err;
>
> /*
> * The memory controller might not be initialized by the firmware
> @@ -155,7 +259,11 @@ static void bluefield_edac_check(struct mem_ctl_info *mci)
> if (mci->edac_cap == EDAC_FLAG_NONE)
> return;
>
> - ecc_count = readl(priv->emi_base + MLXBF_ECC_CNT);
> + err = edac_readl(priv->emi_base + MLXBF_ECC_CNT, &ecc_count,
> + priv->svc_sreg_support, priv->sreg_tbl_edac);
> + if (err)
> + dev_err(priv->dev, "ECC count read failed.\n");
> +
> single_error_count = FIELD_GET(MLXBF_ECC_CNT__SERR_CNT, ecc_count);
> double_error_count = FIELD_GET(MLXBF_ECC_CNT__DERR_CNT, ecc_count);
>
> @@ -172,8 +280,12 @@ static void bluefield_edac_check(struct mem_ctl_info *mci)
> }
>
> /* Write to clear reported errors. */
> - if (ecc_count)
> - writel(ecc_error, priv->emi_base + MLXBF_ECC_ERR);
> + if (ecc_count) {
> + err = edac_writel(priv->emi_base + MLXBF_ECC_ERR, ecc_error,
> + priv->svc_sreg_support, priv->sreg_tbl_edac);
> + if (err)
> + dev_err(priv->dev, "ECC Error write failed.\n");
> + }
> }
>
> /* Initialize the DIMMs information for the given memory controller. */
> @@ -244,6 +356,7 @@ static int bluefield_edac_mc_probe(struct platform_device *pdev)
> struct bluefield_edac_priv *priv;
> struct device *dev = &pdev->dev;
> struct edac_mc_layer layers[1];
> + struct arm_smccc_res res;
> struct mem_ctl_info *mci;
> struct resource *emi_res;
> unsigned int mc_idx, dimm_count;
> @@ -280,12 +393,40 @@ static int bluefield_edac_mc_probe(struct platform_device *pdev)
>
> priv = mci->pvt_info;
>
> + /*
> + * ACPI indicates whether we use SMCs to access registers or not.
> + * If sreg_tbl_perf is not present, just assume we're not using SMCs.
> + */
> + if (device_property_read_u32(dev,
> + "sec_reg_block", &priv->sreg_tbl_edac)) {
> + priv->svc_sreg_support = false;
> + } else {
> + /*
> + * Check service version to see if we actually do support the
> + * needed SMCs. If we have the calls we need, mark support for
> + * them in the pmc struct.
> + */
> + arm_smccc_smc(MLNX_SIP_SVC_VERSION, 0, 0, 0, 0, 0, 0, 0, &res);
> + if (res.a0 == MLNX_EDAC_SVC_REQ_MAJOR &&
> + res.a1 >= MLNX_EDAC_SVC_MIN_MINOR)
> + priv->svc_sreg_support = true;
> + else {
> + dev_err(dev, "Required SMCs are not supported.\n");
> + ret = -EINVAL;
> + goto err;
> + }
> + }
> +
> priv->dimm_per_mc = dimm_count;
> - priv->emi_base = devm_ioremap_resource(dev, emi_res);
> - if (IS_ERR(priv->emi_base)) {
> - dev_err(dev, "failed to map EMI IO resource\n");
> - ret = PTR_ERR(priv->emi_base);
> - goto err;
> + if (!priv->svc_sreg_support) {
> + priv->emi_base = devm_ioremap_resource(dev, emi_res);
> + if (IS_ERR(priv->emi_base)) {
> + dev_err(dev, "failed to map EMI IO resource\n");
> + ret = PTR_ERR(priv->emi_base);
> + goto err;
> + }
> + } else {
> + priv->emi_base = (void __iomem *) emi_res->start;
> }
>
> mci->pdev = dev;
> @@ -353,4 +494,4 @@ module_platform_driver(bluefield_edac_mc_driver);
>
> MODULE_DESCRIPTION("Mellanox BlueField memory edac driver");
> MODULE_AUTHOR("Mellanox Technologies");
> -MODULE_LICENSE("GPL v2");
> +MODULE_LICENSE("Dual BSD/GPL");
>
More information about the kernel-team
mailing list