Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] drm/xe/survivability: missing boot survivability log when pcode probe fails - [Low] drm/xe/survivability: missing static on survivability_mode_required() --
commit 76e10b908f7904219e63d606ae1f0f79dc234cdb Author: Alan Previn <[email protected]> drm/xe/survivability: broadcast dmesg when engaging survivability This commit renames xe_survivability_mode_is_requested() to xe_survivability_mode_is_required() and moves boot survivability logging from enable_boot_survivability_mode() into survivability_mode_required() when log_on_action is true. > diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c > index 7e6027f49a4ca..bb0ccae7f13a8 100644 > --- a/drivers/gpu/drm/xe/xe_device.c > +++ b/drivers/gpu/drm/xe/xe_device.c > @@ -727,7 +727,7 @@ int xe_device_probe_early(struct xe_device *xe) > * the driver will be blocked > */ > err = xe_pcode_probe_early(xe); > - if (err || xe_survivability_mode_is_requested(xe)) { > + if (err || xe_survivability_mode_is_required(xe)) { > int save_err = err; > > /* > diff --git a/drivers/gpu/drm/xe/xe_survivability_mode.c > b/drivers/gpu/drm/xe/xe_survivability_mode.c > index 45d44ebe288b4..50af47ac58cc7 100644 > --- a/drivers/gpu/drm/xe/xe_survivability_mode.c > +++ b/drivers/gpu/drm/xe/xe_survivability_mode.c > @@ -363,11 +363,6 @@ static int enable_boot_survivability_mode(struct pci_dev > *pdev) > if (ret) > goto failed; > > - if (check_boot_failure(xe)) > - xe_log_err_fatal(pdev, SURVIVABILITY, 0, "Boot Mode > enabled!\n"); > - else > - xe_log_info(pdev, SURVIVABILITY, "Boot Mode enabled!\n"); > - > return 0; > > failed: > @@ -389,17 +384,7 @@ bool xe_survivability_mode_is_boot_enabled(struct > xe_device *xe) > return survivability->mode && survivability->type == > XE_SURVIVABILITY_TYPE_BOOT; > } > > -/** > - * xe_survivability_mode_is_requested - check if it's possible to enable > survivability > - * mode that was requested by firmware or > userspace > - * @xe: xe device instance > - * > - * This function reads configfs and boot status from Pcode. > - * > - * Return: true if platform support is available and boot status indicates > - * failure or if survivability mode is requested, false otherwise. > - */ > -bool xe_survivability_mode_is_requested(struct xe_device *xe) > +bool survivability_mode_required(struct xe_device *xe, bool log_on_action) [Severity: Low] Should survivability_mode_required() be marked static? It is only called within drivers/gpu/drm/xe/xe_survivability_mode.c and has no prototype in drivers/gpu/drm/xe/xe_survivability_mode.h, which will trigger a -Wmissing-prototypes compiler warning (and fail the build when CONFIG_DRM_XE_WERROR or CONFIG_WERROR is enabled). > { > struct xe_survivability *survivability = &xe->survivability; > struct xe_mmio *mmio = xe_root_tile_mmio(xe); > @@ -412,13 +397,36 @@ bool xe_survivability_mode_is_requested(struct > xe_device *xe) > > survivability_mode = xe_configfs_get_survivability_mode(pdev); > /* Enable survivability mode if set via configfs */ > - if (survivability_mode) > + if (survivability_mode) { > + if (log_on_action) > + xe_log_info(xe, SURVIVABILITY, "Boot mode > requested.\n"); > return true; > + } > > data = xe_mmio_read32(mmio, PCODE_SCRATCH(0)); > survivability->boot_status = REG_FIELD_GET(BOOT_STATUS, data); > > - return check_boot_failure(xe); > + if (check_boot_failure(xe)) { > + if (log_on_action) > + xe_log_err_fatal(pdev, SURVIVABILITY, 0, "Boot Mode > required!\n"); > + return true; > + } > + return false; > +} > + > +/** > + * xe_survivability_mode_is_required - check if survivability mode is > required based on > + * state of firmware or userspace > request. > + * @xe: xe device instance > + * > + * This function reads configfs and boot status from Pcode. > + * > + * Return: true if platform support is available and boot status indicates > + * failure or if survivability mode is requested, false otherwise. > + */ > +bool xe_survivability_mode_is_required(struct xe_device *xe) > +{ > + return survivability_mode_required(xe, true); > } > > /** > @@ -468,7 +476,7 @@ int xe_survivability_mode_boot_enable(struct xe_device > *xe) > struct xe_survivability *survivability = &xe->survivability; > struct pci_dev *pdev = to_pci_dev(xe->drm.dev); > > - if (!xe_survivability_mode_is_requested(xe)) > + if (!survivability_mode_required(xe, false)) [Severity: Medium] Does this suppress all boot survivability dmesg messages when xe_pcode_probe_early() returns an error? In drivers/gpu/drm/xe/xe_device.c, xe_device_probe_early() checks: err = xe_pcode_probe_early(xe); if (err || xe_survivability_mode_is_required(xe)) { int save_err = err; err = xe_survivability_mode_boot_enable(xe); When pcode initialization fails (err != 0), the || operator short-circuits and skips calling xe_survivability_mode_is_required(xe). Execution then enters xe_survivability_mode_boot_enable(xe), which calls survivability_mode_required(xe, false) with log_on_action set to false. Because the "Boot Mode enabled!" messages were also removed from enable_boot_survivability_mode(), won't entering boot survivability mode on a pcode probe failure result in no survivability message being logged at all? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
