Re: [PATCH v5 20/33] drm/xe/survivability: Report 'boot status' using SIGID
| Newsgroups | org.freedesktop.lists.intel-xe |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [High] Potential infinite loop in `populate_survivability_info()` due to unvalidated hardware-provided linked list. -- --- Patch [20]: [PATCH v5 20/33] drm/xe/survivability: Report 'boot status' using SIGID --- commit 777c3378750db72921a668da05900b90d4fd8d8e Author: Michal Wajdeczko <[email protected]> drm/xe/survivability: Report 'boot status' using SIGID This commit updates the driver to report 'boot status' and other scratch register details using the xe_log_info() macros. It also adds a helper to decode the boot status code and changes the pdev parameter to xe. > diff --git a/drivers/gpu/drm/xe/xe_survivability_mode.c b/drivers/gpu/drm/xe/xe_survivability_mode.c > index 4c506027fa94c..0ace10ae500af 100644 > --- a/drivers/gpu/drm/xe/xe_survivability_mode.c > +++ b/drivers/gpu/drm/xe/xe_survivability_mode.c [ ... ] > @@ -172,18 +173,32 @@ static void populate_survivability_info(struct xe_device *xe) > } > } [Severity: High] This is a pre-existing issue, but is there a risk of an infinite loop in populate_survivability_info() if the hardware returns a cyclic linked list? Looking at the loop that traverses the aux info registers just above this point in the function: if (reg_value & AUXINFO_SUPPORT) { for (id = REG_FIELD_GET(AUXINFO_REG_OFFSET, reg_value); id >= AUX_INFO0 && id < MAX_SCRATCH_REG; id = REG_FIELD_GET(AUXINFO_HISTORY_OFFSET, info[id])) set_survivability_info(mmio, info, id); } If the PCODE firmware returns a value in AUXINFO_HISTORY_OFFSET that points to the current register index or creates a cycle among the registers, could this loop run indefinitely without a limit to the iteration count? An infinite loop here might result in a kernel thread hang during device probe. > -static void log_survivability_info(struct pci_dev *pdev) > +static const char *boot_status_str(u8 boot_status) [ ... ] -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=20