[PATCH] drm/xe/xe_ras: Use xe_log_info to avoid passing zero as error code

[PATCH] drm/xe/xe_ras: Use xe_log_info to avoid passing zero as error code

[Ethan Tidmore]

xe_log_err() expects an error code, but in the case 0 branch of
handle_page_offline(), 'ret' is zero. Passing zero to a function
expecting an error code is incorrect.

Switch to xe_log_info() instead so that an error code is not
expected and zero is not passed.

Fixes: 12b427cf8bb0 ("drm/xe/xe_ras: Handle page offline requests for device memory ecc errors")
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
 drivers/gpu/drm/xe/xe_ras.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
index db3cbc186a51..7942a2a48887 100644
--- a/drivers/gpu/drm/xe/xe_ras.c
+++ b/drivers/gpu/drm/xe/xe_ras.c
@@ -304,9 +304,9 @@ static int handle_page_offline(struct xe_device *xe, u64 page_address, bool noti
 	switch (ret) {
 	case 0:
 		action = XE_RAS_PAGE_ACTION_REMOVE;
-		xe_log_err(xe, DEVICE_MEMORY, ret,
-			   "Poison detected at physical address 0x%llx, page soft-offlined\n",
-			   page_address);
+		xe_log_info(xe, DEVICE_MEMORY,
+			    "Poison detected at physical address 0x%llx, page soft-offlined\n",
+			    page_address);
 		break;
 	case -EIO:
 		xe_log_err(xe, DEVICE_MEMORY, ret,
-- 
Thanks,
ET
https://github.com/sponsors/ethantidmore

Re: [PATCH] drm/xe/xe_ras: Use xe_log_info to avoid passing zero as error code

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/xe/xe_ras: unintended severity downgrade in handle_page_offline()
--

commit 43f2f57f0adc520b740507efd524d932280d6544
Author: Ethan Tidmore <ethantidmore06@gmail.com>

drm/xe/xe_ras: Use xe_log_info to avoid passing zero as error code

This commit replaces xe_log_err() with xe_log_info() in the case 0 branch
of handle_page_offline() in drivers/gpu/drm/xe/xe_ras.c to avoid passing
a zero return value to xe_log_err().

> diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
> index 2c01c183d02b1..df157d521d640 100644
> --- a/drivers/gpu/drm/xe/xe_ras.c
> +++ b/drivers/gpu/drm/xe/xe_ras.c
> @@ -346,9 +346,9 @@ static int handle_page_offline(struct xe_device *xe, u64 page_address, bool noti
>  	switch (ret) {
>  	case 0:
>  		action = XE_RAS_PAGE_ACTION_REMOVE;
> -		xe_log_err(xe, DEVICE_MEMORY, ret,
> -			   "Poison detected at physical address 0x%llx, page soft-offlined\n",
> -			   page_address);
> +		xe_log_info(xe, DEVICE_MEMORY,
> +			    "Poison detected at physical address 0x%llx, page soft-offlined\n",
> +			    page_address);

[Severity: Medium]
Does switching to xe_log_info() here unintentionally downgrade the severity
of a hardware memory poison event when xe_ttm_vram_handle_addr_fault()
returns 0 in handle_page_offline()?

In drivers/gpu/drm/xe/xe_log.h, xe_log_err() explicitly documents 0 as a
valid argument when no negative errno applies, and emits the log entry with
CPER_SEV_RECOVERABLE severity:

    * @err: negative errno for the failing operation, or 0 if not applicable
    ...
    * The log entry will be emitted with @CPER_SEV_RECOVERABLE severity.
    */
    #define xe_log_err(any, TAG, err, fmt, args...) \
            xe_log_comp_recoverable((any), TAG, ERR_PTR(err), 0, fmt, ##args)

Meanwhile, xe_log_info() still passes 0 to ERR_PTR() via xe_log_err_info(),
while downgrading the severity to CPER_SEV_INFORMATIONAL:

    #define xe_log_err_info(any, TAG, err, fmt, args...) \
            xe_log_comp_info((any), TAG, ERR_PTR(err), 0, fmt, ##args)
    ...
    #define xe_log_info(any, TAG, fmt, args...) \
            xe_log_err_info((any), TAG, 0, fmt, ##args)

In drivers/gpu/drm/xe/xe_log.c, log_emit_dmesg() handles ERR_PTR(0) (which is
NULL) in the final else branch by omitting the (%pe) output, while using
cper_sev to select pci_err() vs pci_info() and whether to include the
*ERROR* and [Hardware Error] prefixes:

    if (IS_ERR(data))
            log_dmesg_printf(pdev, cper_sev, "SIGID=%u %s(%pe) %s%s%s%pV",
                             sigid, sev_prefix, data, hwe_prefix,
                             loc_prefix, comp_prefix, vaf);
    else if (data && len)
            log_dmesg_printf(pdev, cper_sev, "SIGID=%u %s(%*phN) %s%s%s%pV",
                             sigid, sev_prefix, (int)len, data, hwe_prefix,
                             loc_prefix, comp_prefix, vaf);
    else
            log_dmesg_printf(pdev, cper_sev, "SIGID=%u %s%s%s%s%pV",
                             sigid, sev_prefix, hwe_prefix,
                             loc_prefix, comp_prefix, vaf);

In addition, send_page_offline_cmd() in drivers/gpu/drm/xe/xe_ras.c, which
runs immediately after this switch on the case 0 path, already passes a
literal 0 to xe_log_err():

    if (action == XE_RAS_PAGE_ACTION_OFFLINE)
            xe_log_err(xe, DEVICE_MEMORY, 0, "Requesting firmware to offline page 0x%llx\n",
                       page_address);
    else
            xe_log_err(xe, DEVICE_MEMORY, 0, "Requesting firmware to remove page 0x%llx from queue\n",
                       page_address);

If this change was intended to silence a static checker warning on passing
the variable ret to ERR_PTR(ret) when ret is known to be 0, would passing a
literal 0 to xe_log_err() instead preserve the CPER_SEV_RECOVERABLE severity?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101225252.556741-1-ethantidmore06@gmail.com?part=1

✗ LGCI.VerificationFailed: failure for drm/xe/xe_ras: Use xe_log_info to avoid passing zero as error code

[Patchwork]

== Series Details ==

Series: drm/xe/xe_ras: Use xe_log_info to avoid passing zero as error code
URL   : https://patchwork.freedesktop.org/series/177448/
State : failure

== Summary ==

Series author address 'ethantidmore06@gmail.com' is not on the allowlist, which prevents CI from being automatically triggered.
If you want CI to run for this series, ask Patchwork project owners to click 'retest' on the series in Patchwork.
Exception occurred during validation, bailing out!
Build URL: http://intel-gfx-ci-public.igk.intel.com:8080/job/xe_pw_trigger/1331034/ (on master)