RE: [PATCH v2] drm/xe/hdcp: Add NULL check for media_gt in intel_hdcp_gsc_check_status()

[Gustavo Sousa <gustavo.sousa@intel.com>]

Gustavo Sousa <gustavo.sousa@intel.com> writes:

> "Lin, Shuicheng" <shuicheng.lin@intel.com> writes:
>
>> On Thu, Apr 16, 2026 12:38 PM Matthew Roper wrote:
>>> On Thu, Apr 16, 2026 at 03:17:19PM -0300, Gustavo Sousa wrote:
>>> > When media GT is disabled via configfs, there is no allocation for
>>> > media_gt, which is kept as NULL.  In such scenario,
>>> > intel_hdcp_gsc_check_status() results in a kernel pagefault error due
>>> > to &gt->uc.gsc being evaluated as an invalid memory address.
>>> >
>>> > Fix that by introducing a NULL check on media_gt and bailing out early
>>> > if so.
>>> >
>>> > While at it, also drop the NULL check for gsc, since it can't be NULL
>>> > if media_gt is not NULL.
>>> >
>>> > v2:
>>> >   - Get address for gsc only after checking that gt is not NULL.
>>> >     (Shuicheng)
>>> >   - Drop the NULL check for gsc. (Shuicheng)
>>> >
>>> > Cc: Shuicheng Lin <shuicheng.lin@intel.com>
>>> > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>>> 
>>> We should probably also have
>>> 
>>> Fixes: 4af50beb4e0f ("drm/xe: Use gsc_proxy_init_done to check proxy
>>> status")
>>> Cc: <stable@vger.kernel.org> # v6.10+
>>> 
>>> too?  Or I guess we could make the argument that there don't seem to be any
>>> real devices without the media GT, so it only became possible to trigger this
>>> once we added configfs support for manually disabling GTs?
>>> In that case,
>>> 
>>> Fixes: 7abd69278bb5 ("drm/xe/configfs: Add attribute to disable GT types")
>>> Cc: <stable@vger.kernel.org> # v6.19+
>>> 
>>> minimizes the number of kernel versions this would be backported to.
>>
>> Is it possible that we have future platform that does not have media gt, and be supported with old kernel?
>> For that case, it should be supported with dkms, so this Fixes tag should not affect. Is it right?
>> So, I prefer this Fixes tag.
>>
>> LGTM.
>> Reviewed-by: Shuicheng Lin <shuicheng.lin@intel.com>
>
> Yeah, I guess it is safer to use the first proposed Fixes tag.  I'll use
> that one when applying.

Pushed to drm-xe-next. Thanks Shuicheng and Matt for the review.

--
Gustavo Sousa

>
> --
> Gustavo Sousa
>
>>
>>> 
>>> 
>>> Anyway, the change itself is
>>> 
>>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
>>> 
>>> > ---
>>> > Changes in v2:
>>> > - EDITME: describe what is new in this series revision.
>>> > - EDITME: use bulletpoints and terse descriptions.
>>> > - Link to v1:
>>> > https://patch.msgid.link/20260302-check-for-null-media_gt-in-intel_hdc
>>> > p_gsc_check_status-v1-1-163d5f826b30@intel.com
>>> > ---
>>> >  drivers/gpu/drm/xe/display/xe_hdcp_gsc.c | 12 ++++++++++--
>>> >  1 file changed, 10 insertions(+), 2 deletions(-)
>>> >
>>> > diff --git a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c
>>> > b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c
>>> > index 29c72aa4b0d2..33494b86205d 100644
>>> > --- a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c
>>> > +++ b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c
>>> > @@ -37,9 +37,17 @@ static bool intel_hdcp_gsc_check_status(struct
>>> drm_device *drm)
>>> >  	struct xe_device *xe = to_xe_device(drm);
>>> >  	struct xe_tile *tile = xe_device_get_root_tile(xe);
>>> >  	struct xe_gt *gt = tile->media_gt;
>>> > -	struct xe_gsc *gsc = &gt->uc.gsc;
>>> > +	struct xe_gsc *gsc;
>>> > +
>>> > +	if (!gt) {
>>> > +		drm_dbg_kms(&xe->drm,
>>> > +			    "not checking GSC status for HDCP2.x: media GT not
>>> present or disabled\n");
>>> > +		return false;
>>> > +	}
>>> > +
>>> > +	gsc = &gt->uc.gsc;
>>> >
>>> > -	if (!gsc || !xe_uc_fw_is_available(&gsc->fw)) {
>>> > +	if (!xe_uc_fw_is_available(&gsc->fw)) {
>>> >  		drm_dbg_kms(&xe->drm,
>>> >  			    "GSC Components not ready for HDCP2.x\n");
>>> >  		return false;
>>> >
>>> > ---
>>> > base-commit: 9ac7dca8d82a4e6d9806ac6c991f0777338833bb
>>> > change-id:
>>> > 20260302-check-for-null-media_gt-in-intel_hdcp_gsc_check_status-
>>> 071898
>>> > b8a402
>>> >
>>> > Best regards,
>>> > --
>>> > Gustavo Sousa <gustavo.sousa@intel.com>
>>> >
>>> 
>>> --
>>> Matt Roper
>>> Graphics Software Engineer
>>> Linux GPU Platform Enablement
>>> Intel Corporation