[Intel-xe] [PATCH 05/11] drm/xe: Support GSC hardware error reporting for PVC.
Aravind Iddamsetty
aravind.iddamsetty at linux.intel.com
Wed Oct 11 07:18:07 UTC 2023
On 27/09/23 17:16, Himal Prasad Ghimiray wrote:
> Add support to report GSC hw errors and counter update in case
> of correctable errors.
>
> Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray at intel.com>
> ---
> drivers/gpu/drm/xe/regs/xe_tile_error_regs.h | 8 ++
> drivers/gpu/drm/xe/xe_hw_error.c | 96 +++++++++++++++++++-
> drivers/gpu/drm/xe/xe_hw_error.h | 16 ++++
> 3 files changed, 117 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/regs/xe_tile_error_regs.h b/drivers/gpu/drm/xe/regs/xe_tile_error_regs.h
> index db78d6687213..fa16eaf9436b 100644
> --- a/drivers/gpu/drm/xe/regs/xe_tile_error_regs.h
> +++ b/drivers/gpu/drm/xe/regs/xe_tile_error_regs.h
> @@ -12,4 +12,12 @@
> #define DEV_ERR_STAT_REG(x) XE_REG(_PICK_EVEN((x), \
> _DEV_ERR_STAT_CORRECTABLE, \
> _DEV_ERR_STAT_NONFATAL))
> +
> +#define PVC_GSC_HECI1_BASE 0x00284000
> +#define PVC_GSC_HECI2_BASE 0x00285000
please maintain the order of register definition as per address in this file.
> +#define _GSC_HEC_CORR_ERR_STATUS 0x128
> +#define _GSC_HEC_UNCOR_ERR_STATUS 0x118
> +#define GSC_HEC_ERR_STAT_REG(base, x) XE_REG(_PICK_EVEN((x), \
> + (base) + _GSC_HEC_CORR_ERR_STATUS, \
> + (base) + _GSC_HEC_UNCOR_ERR_STATUS))
> #endif
> diff --git a/drivers/gpu/drm/xe/xe_hw_error.c b/drivers/gpu/drm/xe/xe_hw_error.c
> index 9595e3369656..eb76b8e6a338 100644
> --- a/drivers/gpu/drm/xe/xe_hw_error.c
> +++ b/drivers/gpu/drm/xe/xe_hw_error.c
> @@ -183,6 +183,28 @@ static const struct err_msg_cntr_pair err_stat_gt_correctable_vectr_reg[] = {
> [2 ... 3] = {"L3BANK", XE_GT_HW_ERR_L3BANK_CORR},
> };
>
> +static const struct err_msg_cntr_pair gsc_nonfatal_err_reg[] = {
> + [0] = {"MinuteIA Unexpected Shutdown", XE_GSC_HW_ERR_MIA_SHUTDOWN_UNCOR},
> + [1] = {"MinuteIA Internal Error", XE_GSC_HW_ERR_MIA_INTERNAL_UNCOR},
> + [2] = {"Double bit error on SRAM", XE_GSC_HW_ERR_SRAM_UNCOR},
> + [3] = {"WDT 2nd Timeout", XE_GSC_HW_ERR_WDG_UNCOR},
> + [4] = {"ROM has a parity error", XE_GSC_HW_ERR_ROM_PARITY_UNCOR},
> + [5] = {"Ucode has a parity error", XE_GSC_HW_ERR_UCODE_PARITY_UNCOR},
> + [6] = {"Errors Reported to and Detected by FW", XE_GSC_HW_ERR_FW_UNCOR},
> + [7] = {"Glitch is detected on voltage rail", XE_GSC_HW_ERR_VLT_GLITCH_UNCOR},
> + [8] = {"Fuse Pull Error", XE_GSC_HW_ERR_FUSE_PULL_UNCOR},
> + [9] = {"Fuse CRC Check Failed on Fuse Pull", XE_GSC_HW_ERR_FUSE_CRC_UNCOR},
> + [10] = {"Self Mbist Failed", XE_GSC_HW_ERR_SELF_MBIST_UNCOR},
> + [11] = {"AON RF has parity error", XE_GSC_HW_ERR_AON_RF_PARITY_UNCOR},
> + [12 ... 31] = {"Undefined", XE_GSC_HW_ERR_UNKNOWN_UNCOR},
> +};
> +
> +static const struct err_msg_cntr_pair gsc_correctable_err_reg[] = {
> + [0] = {"Single bit error on SRAM", XE_GSC_HW_ERR_SRAM_CORR},
> + [1] = {"Errors Reported to FW and Detected by FW", XE_GSC_HW_ERR_FW_CORR},
> + [2 ... 31] = {"Undefined", XE_GSC_HW_ERR_UNKNOWN_CORR},
> +};
> +
> static void xe_assign_hw_err_regs(struct xe_device *xe)
> {
> const struct err_msg_cntr_pair **dev_err_stat = xe->hw_err_regs.dev_err_stat;
> @@ -344,6 +366,71 @@ xe_gt_hw_error_handler(struct xe_gt *gt, const enum hardware_error hw_err)
> xe_gt_hw_error_status_reg_handler(gt, hw_err);
> }
>
> +static void
> +xe_gsc_hw_error_handler(struct xe_tile *tile, const enum hardware_error hw_err)
> +{
> + const char *hw_err_str = hardware_error_type_to_str(hw_err);
> + const struct err_msg_cntr_pair *errstat;
> + struct xe_gt *mmio;
better name this as gt, as it really holds a struct xe_gt and not a struct mmio in tile
> + unsigned long errsrc;
> + const char *errmsg;
> + u32 indx;
> + u32 errbit;
> + u32 base;
> +
> + if ((tile_to_xe(tile)->info.platform != XE_PVC))
> + return;
> +
> + /* GSC errors are valid only on root tile and for NONFATAL and
> + * CORRECTABLE type.For non root tiles or FATAL type it should
> + * be categorized as undefined GSC HARDWARE ERROR
> + */
> + base = PVC_GSC_HECI1_BASE;
> +
> + if (tile->id || hw_err == HARDWARE_ERROR_FATAL) {
> + drm_err_ratelimited(&tile_to_xe(tile)->drm, HW_ERR
> + "Undefined GSC %s error on tile%d\n", hw_err_str, tile->id);
> + return;
> + }
> +
> + lockdep_assert_held(&tile_to_xe(tile)->irq.lock);
> + if (hw_err == HARDWARE_ERROR_CORRECTABLE)
> + errstat = gsc_correctable_err_reg;
> + else
> + errstat = gsc_nonfatal_err_reg;
let's have one common design of initializing all error registers at once during driver load.
> +
> + mmio = tile->primary_gt;
> + errsrc = xe_mmio_read32(mmio, GSC_HEC_ERR_STAT_REG(base, hw_err));
> + if (!errsrc) {
> + drm_err_ratelimited(&tile_to_xe(tile)->drm, HW_ERR
> + "GSC detected GSC_HEC_ERR_STAT_REG_%s blank!\n", hw_err_str);
> + goto clear_reg;
> + }
> +
> + drm_info(&tile_to_xe(tile)->drm, HW_ERR
> + "GSC_HEC_ERR_STAT_REG_%s=0x%08lx\n", hw_err_str, errsrc);
> +
> + for_each_set_bit(errbit, &errsrc, 32) {
> + errmsg = errstat[errbit].errmsg;
> + indx = errstat[errbit].cntr_indx;
> +
> + if (hw_err == HARDWARE_ERROR_CORRECTABLE) {
> + drm_warn(&tile_to_xe(tile)->drm,
> + HW_ERR "GSC detected %s %s error, bit[%d] is set\n",
> + errmsg, hw_err_str, errbit);
the message should have common convention , like "TileN reported XXX" for all the errors we log
> +
> + } else {
> + drm_err_ratelimited(&tile_to_xe(tile)->drm,
> + HW_ERR "GSC detected %s %s error, bit[%d] is set\n",
> + errmsg, hw_err_str, errbit);
> + }
may be you want to skip FW_ERR here, see below.
> + tile->errors.count[indx]++;
> + }
> +
> +clear_reg:
> + xe_mmio_write32(mmio, GSC_HEC_ERR_STAT_REG(base, hw_err), errsrc);
> +}
> +
> static void
> xe_hw_error_source_handler(struct xe_tile *tile, const enum hardware_error hw_err)
> {
> @@ -385,10 +472,13 @@ xe_hw_error_source_handler(struct xe_tile *tile, const enum hardware_error hw_er
> HW_ERR "TILE%d detected %s %s error, bit[%d] is set\n",
> tile->id, errmsg, hw_err_str, errbit);
> tile->errors.count[indx]++;
> - }
>
> - if (errsrc & REG_BIT(0))
> - xe_gt_hw_error_handler(tile->primary_gt, hw_err);
> + if (errbit == 0)
> + xe_gt_hw_error_handler(tile->primary_gt, hw_err);
> +
> + if (errbit == 8)
> + xe_gsc_hw_error_handler(tile, hw_err);
please define what 0 and 8 are.
> + }
>
> xe_mmio_write32(mmio, DEV_ERR_STAT_REG(hw_err), errsrc);
> unlock:
> diff --git a/drivers/gpu/drm/xe/xe_hw_error.h b/drivers/gpu/drm/xe/xe_hw_error.h
> index 2812407dd4bf..155722a0af4c 100644
> --- a/drivers/gpu/drm/xe/xe_hw_error.h
> +++ b/drivers/gpu/drm/xe/xe_hw_error.h
> @@ -48,6 +48,22 @@ enum xe_tile_hw_errors {
> XE_TILE_HW_ERR_GSC_CORR,
> XE_TILE_HW_ERR_SOC_CORR,
> XE_TILE_HW_ERR_UNKNOWN_CORR,
> + XE_GSC_HW_ERR_SRAM_CORR,
> + XE_GSC_HW_ERR_FW_CORR,
I don't think counting FW_CORR is correct as the acutal count is maintained and reported by CSC via HECI
> + XE_GSC_HW_ERR_UNKNOWN_CORR,
> + XE_GSC_HW_ERR_MIA_SHUTDOWN_UNCOR,
> + XE_GSC_HW_ERR_MIA_INTERNAL_UNCOR,
> + XE_GSC_HW_ERR_SRAM_UNCOR,
> + XE_GSC_HW_ERR_WDG_UNCOR,
> + XE_GSC_HW_ERR_ROM_PARITY_UNCOR,
> + XE_GSC_HW_ERR_UCODE_PARITY_UNCOR,
> + XE_GSC_HW_ERR_FW_UNCOR,
> + XE_GSC_HW_ERR_VLT_GLITCH_UNCOR,
> + XE_GSC_HW_ERR_FUSE_PULL_UNCOR,
> + XE_GSC_HW_ERR_FUSE_CRC_UNCOR,
> + XE_GSC_HW_ERR_SELF_MBIST_UNCOR,
> + XE_GSC_HW_ERR_AON_RF_PARITY_UNCOR,
we shall maintain uniform naming,
everywhere it is mentioned as NONFATAL, here is just UNCOR
> + XE_GSC_HW_ERR_UNKNOWN_UNCOR,
> XE_TILE_HW_ERROR_MAX,
> };
Thanks,
Aravind,
>
More information about the Intel-xe
mailing list