[PATCH] drm/xe/guc: Make creation of SLPC debugfs files conditional
Upadhyay, Tejas
tejas.upadhyay at intel.com
Fri May 16 05:42:43 UTC 2025
> -----Original Message-----
> From: Bhatia, Aradhya <aradhya.bhatia at intel.com>
> Sent: 16 May 2025 10:38
> To: Summers, Stuart <stuart.summers at intel.com>; Upadhyay, Tejas
> <tejas.upadhyay at intel.com>; Roper, Matthew D
> <matthew.d.roper at intel.com>
> Cc: intel-xe at lists.freedesktop.org; Ghimiray, Himal Prasad
> <himal.prasad.ghimiray at intel.com>
> Subject: Re: [PATCH] drm/xe/guc: Make creation of SLPC debugfs files
> conditional
>
> Hi Stuart,
>
> Thank you for reviewing the patch.
>
> On 16-05-2025 00:07, Summers, Stuart wrote:
> > On Thu, 2025-05-15 at 23:06 +0530, Aradhya Bhatia wrote:
> >> Hi Tejas,
> >>
> >> Thank you for reviewing the patch.
> >>
> >> On 15-05-2025 18:46, Upadhyay, Tejas wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Bhatia, Aradhya <aradhya.bhatia at intel.com>
> >>>> Sent: 15 May 2025 15:19
> >>>> To: Roper, Matthew D <matthew.d.roper at intel.com>
> >>>> Cc: Intel XE List <intel-xe at lists.freedesktop.org>; Upadhyay, Tejas
> >>>> <tejas.upadhyay at intel.com>; Ghimiray, Himal Prasad
> >>>> <himal.prasad.ghimiray at intel.com>; Bhatia, Aradhya
> >>>> <aradhya.bhatia at intel.com>
> >>>> Subject: [PATCH] drm/xe/guc: Make creation of SLPC debugfs files
> >>>> conditional
> >>>>
> >>>> Platforms that do not support SLPC are exempted from the GuC PC
> >>>> support.
> >>>> The GuC PC does not get initialized, and neither do its BOs get
> >>>> created.
> >>>>
> >>>> This causes a problem because the GuC PC debugfs file is still
> >>>> being created.
> >>>> Whenever the file is attempted to read, it causes a NULL pointer
> >>>> dereference on the supposed BO of the GuC PC.
> >>>>
> >>>> So, make the creation of SLPC debugfs files conditional to when
> >>>> SLPC features are supported.
> >>>>
> >>>> Suggested-by: Matt Roper <matthew.d.roper at intel.com>
> >>>> Signed-off-by: Aradhya Bhatia <aradhya.bhatia at intel.com>
> >>>> ---
> >>>> drivers/gpu/drm/xe/xe_guc_debugfs.c | 17 ++++++++++++++---
> >>>> 1 file changed, 14 insertions(+), 3 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/xe/xe_guc_debugfs.c
> >>>> b/drivers/gpu/drm/xe/xe_guc_debugfs.c
> >>>> index f33013f8a0f3..0b102ab46c4d 100644
> >>>> --- a/drivers/gpu/drm/xe/xe_guc_debugfs.c
> >>>> +++ b/drivers/gpu/drm/xe/xe_guc_debugfs.c
> >>>> @@ -113,23 +113,34 @@ static const struct drm_info_list
> >>>> vf_safe_debugfs_list[] = {
> >>>> { "guc_ctb", .show = guc_debugfs_show, .data = guc_ctb },
> >>>> };
> >>>>
> >>>> +/* For GuC debugfs files that require the SLPC support */ static
> >>>> const
> >>>> +struct drm_info_list slpc_debugfs_list[] = {
> >>>> + { "guc_pc", .show = guc_debugfs_show, .data = guc_pc },
> >>>> };
> >>>> +
> >>>> /* everything else should be added here */ static const struct
> >>>> drm_info_list pf_only_debugfs_list[] = {
> >>>> { "guc_log", .show = guc_debugfs_show, .data = guc_log },
> >>>> { "guc_log_dmesg", .show = guc_debugfs_show, .data =
> >>>> guc_log_dmesg },
> >>>> - { "guc_pc", .show = guc_debugfs_show, .data = guc_pc },
> >>>
> >>> You need fixes tag here it seems. With that,
> >>
> >> About this, there is a slight complication.
> >>
> >> The original patch that introduced this issue is this commit,
> >>
> >> aaab5404b16f ("drm/xe: Introduce GuC PC debugfs") [0]
> >>
> >> This is the patch that introduced GuC PC debugfs file but didn't
> >> account for the case when "skip_guc_pc" is set.
> >>
> >>
> >> But, the guc debugfs has had other commits _after_ the above patch,
> >> which refactor in various ways how guc debugfs is being handled in Xe
> >> driver.
> >>
> >>
> >> e15826bb3c2c ("drm/xe/guc: Refactor GuC debugfs initialization") [1]
> >> 387444984d7b ("drm/xe/guc: Don't expose GuC privileged debugfs files
> >> if
> >> VF") [2]
> >>
> >> Since my patch is based after patches [1], and [2] (which are not
> >> "fixes", and hence were not backported to any previous kernel
> >> version), my patch will not be "backport-able" to the original patch
> >> that needs the fix [0].
> >>
> >>
> >> So, what should be done in this situation?
> >
> > I'd just add the fixes to the original patch. The refactors will have
> > to take that into consideration if these get merged down the road.
>
> Please help me understand, what do you meany by "if these get merged
> down the road".
>
> drm-xe-next(-fixes) (the branch this patch is based on top of) has both those
> refactoring patches merged.
>
> However, the v6.15-rc6 (and drm-xe-fixes) do not.
>
> Please correct me if I am wrong, but the only way I understand that the fix
> patch can be merged _before_ the refactoring patches, is to go through drm-
> xe-fixes.
>
> Is that what you meant?
I think what Stuart's meaning is that just add fixes to original commit, the commit which first introduced this debugfs entry.
Tejas
>
> >
> > Reviewed-by: Stuart Summers <stuart.summers at intel.com>
> >
> > Thanks,
> > Stuart
> >
> >>
> >>
> >> Regards
> >> Aradhya
> >>
> >>
> >> [0]:
> >> https://lore.kernel.org/all/20250114232443.1135355-1-rodrigo.vivi@int
> >> el.com/
> >>
> >> [1]:
> >> https://lore.kernel.org/all/20250403142635.1821-2-michal.wajdeczko@in
> >> tel.com/
> >>
> >> [2]:
> >> https://lore.kernel.org/all/20250403142635.1821-3-michal.wajdeczko@in
> >> tel.com/
> >>
> >>
> >>
> >>> Reviewed-by: Tejas Upadhyay <tejas.upadhyay at intel.com>
> >>>
> >>> Tejas
> >>>> };
> >>>>
> >>>> void xe_guc_debugfs_register(struct xe_guc *guc, struct dentry
> >>>> *parent) {
> >>>> - struct drm_minor *minor = guc_to_xe(guc)->drm.primary;
> >>>> + struct xe_device *xe = guc_to_xe(guc);
> >>>> + struct drm_minor *minor = xe->drm.primary;
> >>>>
> >>>> drm_debugfs_create_files(vf_safe_debugfs_list,
> >>>>
> >>>> ARRAY_SIZE(vf_safe_debugfs_list),
> >>>> parent, minor);
> >>>>
> >>>> - if (!IS_SRIOV_VF(guc_to_xe(guc)))
> >>>> + if (!IS_SRIOV_VF(xe)) {
> >>>> drm_debugfs_create_files(pf_only_debugfs_list,
> >>>>
> >>>> ARRAY_SIZE(pf_only_debugfs_list),
> >>>> parent, minor);
> >>>> +
> >>>> + if (!xe->info.skip_guc_pc)
> >>>> + drm_debugfs_create_files(slpc_debugfs_lis
> >>>> t,
> >>>> +
> >>>> ARRAY_SIZE(slpc_debugfs_list),
> >>>> + parent, minor);
> >>>> + }
> >>>> }
> >>>>
> >>>> base-commit: 3d6670fab64cb00b5e6ed80d2517147db533faf1
> >>>> --
> >>>> 2.43.0
> >>>
> >>
> >
More information about the Intel-xe
mailing list