Re: [PATCH v10 1/4] drm/xe/xe_ras: Do not Review, compilation only.

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Hardware/firmware error payload structures use non-portable C bitfields and host-native integer types instead of explicitly sized little-endian types.
- [Low] The newly added function `handle_soc_internal_errors` is marked as `__maybe_unused`, has no callers, and contains incomplete PUNIT error handling.
--

--- Patch [1]: [PATCH v10 1/4] drm/xe/xe_ras: Do not Review, compilation only. ---
commit 8b53e475734bf697402ee545f94eb93bf4f28f2b
Author: Riana Tauro <riana.tauro@intel.com>

drm/xe/xe_ras: Do not Review, compilation only.
    
DO NOT REVIEW. COMPILATION ONLY
This patch is from https://patchwork.freedesktop.org/series/160482/
Added only for Compilation.

> diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
> index 74d5016d9ffe7..3d39bb95e94d6 100644
> --- a/drivers/gpu/drm/xe/xe_ras.c
> +++ b/drivers/gpu/drm/xe/xe_ras.c
[ ... ]
> @@ -193,6 +194,48 @@ static void ras_usp_aer_init(struct xe_device *xe)
>  	dev_dbg(&usp->dev, "Uncorrectable Internal Errors downgraded and unmasked\n");
>  }
>  
> +static __maybe_unused enum xe_ras_recovery_action
> +handle_soc_internal_errors(struct xe_device *xe,
> +			   struct xe_ras_error_array *arr)

[Severity: Low]
Does this introduce dead, incomplete code into the tree?

This function is marked __maybe_unused, has no callers, and contains an
incomplete PUNIT error handling TODO below.

> +{
> +	struct xe_ras_soc_error *info = (void *)arr->details;
> +	struct xe_ras_soc_error_source *source = &info->source;

[Severity: Medium]
Can this cause misinterpretation of firmware error messages on big-endian
architectures?

This code reads firmware values via the bitfield struct without endianness
conversion. The hardware/firmware error payload structures are defined using
non-portable C bitfields and host-native integer types instead of explicitly
sized little-endian types.

Using C bitfields inside __packed structs for hardware/firmware interface
definitions relies on compiler-specific bit-packing rules. Explicitly sized
little-endian types (e.g., __le32) and bitwise macros (like FIELD_GET())
should probably be used to parse these hardware structures.

> +	struct xe_ras_error_class *counter = &arr->counter;
> +
> +	if (source->csc) {
> +		struct xe_ras_csc_error *csc_error = (void *)info->details;
[ ... ]
> diff --git a/drivers/gpu/drm/xe/xe_ras_types.h b/drivers/gpu/drm/xe/xe_ras_types.h
> index 6688e11f57a86..3c165096ea20c 100644
> --- a/drivers/gpu/drm/xe/xe_ras_types.h
> +++ b/drivers/gpu/drm/xe/xe_ras_types.h
[ ... ]
> +/**
> + * struct xe_ras_soc_error_source - Source of SoC error
> + */
> +struct xe_ras_soc_error_source {
> +	/** @csc: CSC */
> +	u32 csc:1;
> +	/** @ieh: IEH (Integrated Error Handler) */
> +	u32 ieh:1;
> +	/** @reserved: Reserved for future use */
> +	u32 reserved:30;
> +} __packed;

[Severity: Medium]
Will this bitfield layout work correctly across different architectures?

This struct defines hardware/firmware error source flags using brittle C
bitfields. As noted above, relying on compiler-specific bit-packing inside
__packed structs for hardware interfaces can lead to issues.

[ ... ]
> +/**
> + * struct xe_ras_ieh_error - SoC IEH (Integrated Error Handler) error details
> + */
> +struct xe_ras_ieh_error {
> +	/** @ieh_instance: IEH instance */
> +	u32 ieh_instance:2;
> +	/** @reserved: Reserved for future use */
> +	u32 reserved:30;
> +	/** @global_error_status: Global error status */
> +	u32 global_error_status;
> +	/** @local_error_status: Local error status */
> +	u32 local_error_status;
> +	/** @gerr_mask: Global error mask */
> +	u32 gerr_mask;
> +	/** @info: Additional information */
> +	u32 info[10];
> +} __packed;

[Severity: Medium]
Could this bitfield layout be problematic for similar reasons?

This struct defines hardware/firmware IEH error layout using C bitfields and
host-native types instead of explicitly sized little-endian types.

>  #endif

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713083939.251872-6-mallesh.koujalagi@intel.com?part=1