Claude review: drm/xe/xe_hw_error: Add support for PVC SoC errors

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Correctable SoC errors not counted:**

> +	if (hw_err == HARDWARE_ERROR_CORRECTABLE) {
> +		xe_mmio_write32(mmio, SOC_GLOBAL_ERR_STAT_REG(master, hw_err), REG_GENMASK(31, 0));
> +		xe_mmio_write32(mmio, SOC_LOCAL_ERR_STAT_REG(master, hw_err), REG_GENMASK(31, 0));
> +		xe_mmio_write32(mmio, SOC_GLOBAL_ERR_STAT_REG(slave, hw_err), REG_GENMASK(31, 0));
> +		xe_mmio_write32(mmio, SOC_LOCAL_ERR_STAT_REG(slave, hw_err), REG_GENMASK(31, 0));
> +		goto unmask_gsysevtctl;
> +	}

For correctable SoC errors, the status registers are cleared (written with all-ones) without reading them or incrementing any counter. This means the `soc-internal` correctable error counter will always remain 0, regardless of how many correctable SoC errors occur. Is this intentional? It seems like a gap -- for GT errors in `gt_hw_error_handler`, correctable errors DO get counted, but SoC correctable errors are silently discarded.

**Magic value for GSYSEVTCTL unmask:**

> +unmask_gsysevtctl:
> +	for (i = 0; i < XE_SOC_NUM_IEH; i++)
> +		xe_mmio_write32(mmio, SOC_GSYSEVTCTL_REG(master, slave, i),
> +				(HARDWARE_ERROR_MAX << 1) + 1);

`(HARDWARE_ERROR_MAX << 1) + 1` evaluates to `(3 << 1) + 1 = 7 = 0b111`. This presumably enables all three error severity levels. The expression relies on `HARDWARE_ERROR_MAX` being 3 and on the register bit layout matching. A named constant or comment explaining the bit layout would reduce the chance of breakage if `HARDWARE_ERROR_MAX` is ever changed.

**Missing nonfatal local error handling in slave IEH:**

> +	if (hw_err == HARDWARE_ERROR_FATAL) {
> +		for_each_set_bit(regbit, &slave_local_errstat, XE_RAS_REG_SIZE)
> +			log_soc_error(tile, pvc_slave_local_fatal_err_reg, severity,
> +				      regbit, error_id);
> +	}

`soc_slave_ieh_handler` is called for both NONFATAL and FATAL hw_err (correctable was filtered out earlier). For NONFATAL, the slave local error register is read and cleared, but its bits are not processed or logged. There's no `pvc_slave_local_nonfatal_err_reg` array. If the hardware can report nonfatal slave local errors, they would be silently dropped without counting or logging.

**`strcmp` for filtering "Undefined" errors:**

> +	name = reg_info[err_bit];
> +
> +	if (strcmp(name, "Undefined")) {

Using `strcmp` against a string literal to filter out placeholder entries works but is fragile. A more robust approach would be to use NULL for undefined entries and check `if (name)`. The current approach also means every bit that IS set in the register and mapped to "Undefined" triggers a `strcmp` call, which is minor overhead but unnecessary.

---
Generated by Claude Code Patch Reviewer