Re: [PATCH] drm/mst: Fix NULL pointer dereference at drm_dp_add_payload_part2
Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
for once, to make this easily accessible to everyone.
Hmm, from here it looks like the patch now that it was reviewed more
that a week ago is still not even in -next. Is there a reason?
I know, we are in the merge window. But at the same time this is a fix
(that already lingered on the lists for way too long before it was
reviewed) for a regression in a somewhat recent kernel, so it in Linus
own words should be "expedited"[1].
Or are we again just missing a right person for the job in the CC?
Adding Dave and Sima just in case.
Ciao, Thorsten
[1]
https://lore.kernel.org/all/CAHk-=wis_qqy4odnynnki5b7qhosmxtoj1jxo5wmb6sruwq...@mail.gmail.com/
On 12.05.24 18:11, Limonciello, Mario wrote:
> On 5/10/2024 4:24 AM, Jani Nikula wrote:
>> On Fri, 10 May 2024, "Lin, Wayne" wrote:
>>>> -Original Message-
>>>> From: Limonciello, Mario
>>>> Sent: Friday, May 10, 2024 3:18 AM
>>>> To: Linux regressions mailing list ;
>>>> Wentland, Harry
>>>> ; Lin, Wayne
>>>> Cc: ly...@redhat.com; imre.d...@intel.com; Leon Weiß
>>>> >>> bochum.de>; sta...@vger.kernel.org; dri-de...@lists.freedesktop.org;
>>>> amd-
>>>> g...@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
>>>> Subject: Re: [PATCH] drm/mst: Fix NULL pointer dereference at
>>>> drm_dp_add_payload_part2
>>>>
>>>> On 5/9/2024 07:43, Linux regression tracking (Thorsten Leemhuis) wrote:
>>>>> On 18.04.24 21:43, Harry Wentland wrote:
>>>>>> On 2024-03-07 01:29, Wayne Lin wrote:
>>>>>>> [Why]
>>>>>>> Commit:
>>>>>>> - commit 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload
>>>>>>> allocation/removement") accidently overwrite the commit
>>>>>>> - commit 54d217406afe ("drm: use mgr->dev in drm_dbg_kms in
>>>>>>> drm_dp_add_payload_part2") which cause regression.
>>>>>>>
>>>>>>> [How]
>>>>>>> Recover the original NULL fix and remove the unnecessary input
>>>>>>> parameter 'state' for drm_dp_add_payload_part2().
>>>>>>>
>>>>>>> Fixes: 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload
>>>>>>> allocation/removement")
>>>>>>> Reported-by: Leon Weiß
>>>>>>> Link:
>>>>>>> https://lore.kernel.org/r/38c253ea42072cc825dc969ac4e6b9b600371cc8.c
>>>>>>> a...@ruhr-uni-bochum.de/
>>>>>>> Cc: ly...@redhat.com
>>>>>>> Cc: imre.d...@intel.com
>>>>>>> Cc: sta...@vger.kernel.org
>>>>>>> Cc: regressi...@lists.linux.dev
>>>>>>> Signed-off-by: Wayne Lin
>>>>>>
>>>>>> I haven't been deep in MST code in a while but this all looks pretty
>>>>>> straightforward and good.
>>>>>>
>>>>>> Reviewed-by: Harry Wentland
>>>>>
>>>>> Hmmm, that was three weeks ago, but it seems since then nothing
>>>>> happened to fix the linked regression through this or some other
>>>>> patch. Is there a reason? The build failure report from the CI maybe?
>>>>
>>>> It touches files outside of amd but only has an ack from AMD. I
>>>> think we
>>>> /probably/ want an ack from i915 and nouveau to take it through.
>>>
>>> Thanks, Mario!
>>>
>>> Hi Thorsten,
>>> Yeah, like what Mario said. Would also like to have ack from i915 and
>>> nouveau.
>>
>> It usually works better if you Cc the folks you want an ack from! ;)
>>
>> Acked-by: Jani Nikula
>>
>
> Thanks! Can someone with commit permissions take this to drm-misc?
>
>
>
Re: [PATCH] drm/mst: Fix NULL pointer dereference at drm_dp_add_payload_part2
On 18.04.24 21:43, Harry Wentland wrote:
> On 2024-03-07 01:29, Wayne Lin wrote:
>> [Why]
>> Commit:
>> - commit 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload
>> allocation/removement")
>> accidently overwrite the commit
>> - commit 54d217406afe ("drm: use mgr->dev in drm_dbg_kms in
>> drm_dp_add_payload_part2")
>> which cause regression.
>>
>> [How]
>> Recover the original NULL fix and remove the unnecessary input parameter
>> 'state' for
>> drm_dp_add_payload_part2().
>>
>> Fixes: 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload
>> allocation/removement")
>> Reported-by: Leon Weiß
>> Link:
>> https://lore.kernel.org/r/38c253ea42072cc825dc969ac4e6b9b600371cc8.ca...@ruhr-uni-bochum.de/
>> Cc: ly...@redhat.com
>> Cc: imre.d...@intel.com
>> Cc: sta...@vger.kernel.org
>> Cc: regressi...@lists.linux.dev
>> Signed-off-by: Wayne Lin
>
> I haven't been deep in MST code in a while but this all looks
> pretty straightforward and good.
>
> Reviewed-by: Harry Wentland
Hmmm, that was three weeks ago, but it seems since then nothing happened
to fix the linked regression through this or some other patch. Is there
a reason? The build failure report from the CI maybe?
Wayne Lin, do you know what's up?
Ciao, Thorsten
>> ---
>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 +-
>> drivers/gpu/drm/display/drm_dp_mst_topology.c | 4 +---
>> drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +-
>> drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +-
>> include/drm/display/drm_dp_mst_helper.h | 1 -
>> 5 files changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
>> index c27063305a13..2c36f3d00ca2 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
>> @@ -363,7 +363,7 @@ void dm_helpers_dp_mst_send_payload_allocation(
>> mst_state = to_drm_dp_mst_topology_state(mst_mgr->base.state);
>> new_payload = drm_atomic_get_mst_payload_state(mst_state,
>> aconnector->mst_output_port);
>>
>> -ret = drm_dp_add_payload_part2(mst_mgr, mst_state->base.state,
>> new_payload);
>> +ret = drm_dp_add_payload_part2(mst_mgr, new_payload);
>>
>> if (ret) {
>> amdgpu_dm_set_mst_status(>mst_status,
>> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> b/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> index 03d528209426..95fd18f24e94 100644
>> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> @@ -3421,7 +3421,6 @@ EXPORT_SYMBOL(drm_dp_remove_payload_part2);
>> /**
>> * drm_dp_add_payload_part2() - Execute payload update part 2
>> * @mgr: Manager to use.
>> - * @state: The global atomic state
>> * @payload: The payload to update
>> *
>> * If @payload was successfully assigned a starting time slot by
>> drm_dp_add_payload_part1(), this
>> @@ -3430,14 +3429,13 @@ EXPORT_SYMBOL(drm_dp_remove_payload_part2);
>> * Returns: 0 on success, negative error code on failure.
>> */
>> int drm_dp_add_payload_part2(struct drm_dp_mst_topology_mgr *mgr,
>> - struct drm_atomic_state *state,
>> struct drm_dp_mst_atomic_payload *payload)
>> {
>> int ret = 0;
>>
>> /* Skip failed payloads */
>> if (payload->payload_allocation_status !=
>> DRM_DP_MST_PAYLOAD_ALLOCATION_DFP) {
>> -drm_dbg_kms(state->dev, "Part 1 of payload creation for %s
>> failed, skipping part 2\n",
>> +drm_dbg_kms(mgr->dev, "Part 1 of payload creation for %s
>> failed, skipping part 2\n",
>> payload->port->connector->name);
>> return -EIO;
>> }
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> index 53aec023ce92..2fba66aec038 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> @@ -1160,7 +1160,7 @@ static void intel_mst_enable_dp(struct
>> intel_atomic_state *state,
>> if (first_mst_stream)
>> intel_ddi_wait_for_fec_status(encoder, pipe_config, true);
>>
>> -drm_dp_add_payload_part2(_dp->mst_mgr, >base,
>> +drm_dp_add_payload_part2(_dp->mst_mgr,
>> drm_atomic_get_mst_payload_state(mst_state,
>> connector->port));
>>
>> if (DISPLAY_VER(dev_priv) >= 12)
>> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
>> b/drivers/gpu/drm/nouveau/dispnv50/disp.c
>> index 0c3d88ad0b0e..88728a0b2c25 100644
>> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
>> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
>> @@ -915,7 +915,7 @@ nv50_msto_cleanup(struct drm_atomic_state *state,
>> msto->disabled = false;
>>
Re: [REGRESSION] external monitor+Dell dock in 6.8
[Adding a few folks and list while dropping the stable list, as this is
unrelated to it]
On 31.03.24 07:59, Andrei Gaponenko wrote:
>
> I noticed a regression with the mailine kernel pre-compiled by EPEL.
> I have just tried linux-6.9-rc1.tar.gz from kernel.org, and it still
> misbehaves.
>
> The default setup: a laptop is connected to a dock, Dell WD22TB4, via
> a USB-C cable. The dock is connected to an external monitor via a
> Display Port cable. With a "good" kernel everything works. With a
> "broken" kernel, the external monitor is still correctly identified by
> the system, and is shown as enabled in plasma systemsettings. The
> system also behaves like the monitor is working, for example, one can
> move the mouse pointer off the laptop screen. However the external
> monitor screen stays black, and it eventually goes to sleep.
Just a quick heads up to ensure people are aware of it:
Imre Deak, turns out this is caused by a patch of yours: 55eaef16417448
("drm/i915/dp_mst: Handle the Synaptics HBlank expansion quirk"). Andrei
Gaponenko meanwhile filed a ticket about it here:
https://gitlab.freedesktop.org/drm/intel/-/issues/10637
Ciao, Thorsten
> Everything worked with EPEL mainline kernels up to and including
> kernel-ml-6.7.9-1.el9.elrepo.x86_64
>
> The breakage is observed in
>
> kernel-ml-6.8.1-1.el9.elrepo.x86_64
> kernel-ml-6.8.2-1.el9.elrepo.x86_64
> linux-6.9-rc1.tar.gz from kernel.org (with olddefconfig)
>
> Other tests: using an HDMI cable instead of the Display Port cable
> between the monitor and the dock does not change things, black screen
> with the newer kernels.
>
> Using a small HDMI-to-USB-C adapter instead of the dock results in a
> working system, even with the newer kernels. So the breakage appears
> to be specific to the Dell WD22TB4 dock.
>
> Operating System: AlmaLinux 9.3 (Shamrock Pampas Cat)
>
> uname -mi: x86_64 x86_64
>
> Laptop: Dell Precision 5470/02RK6V
>
> lsusb |grep dock
> Bus 003 Device 007: ID 413c:b06e Dell Computer Corp. Dell dock
> Bus 003 Device 008: ID 413c:b06f Dell Computer Corp. Dell dock
> Bus 003 Device 006: ID 0bda:5413 Realtek Semiconductor Corp. Dell dock
> Bus 003 Device 005: ID 0bda:5487 Realtek Semiconductor Corp. Dell dock
> Bus 002 Device 004: ID 0bda:0413 Realtek Semiconductor Corp. Dell dock
> Bus 002 Device 003: ID 0bda:0487 Realtek Semiconductor Corp. Dell dock
>
> dmesg and kernel config are attached to
> https://bugzilla.kernel.org/show_bug.cgi?id=218663
>
> #regzbot introduced: v6.7.9..v6.8.1
P.S.:
#regzbot duplicate: https://bugzilla.kernel.org/show_bug.cgi?id=218663
#regzbot duplicate: https://gitlab.freedesktop.org/drm/intel/-/issues/10637
#regzbot title: drm/i915/dp_mst: external monitor on Dell dock broke
Re: [PATCH] Fix divide-by-zero on DP unplug with nouveau
On 11.03.24 17:09, Imre Deak wrote: > On Sat, Feb 10, 2024 at 09:24:59PM +, Chris Bainbridge wrote: > Sorry for the delay. Happens, thx for looking onto this! >> The following trace occurs when using nouveau and unplugging a DP MST >> adaptor: > [...] >> +if (bpp_x16 == 0) >> +return 0; > > Could you please move the check to the beginnig of the function and add > a debug message in case bpp_x16 is 0? > > It looks odd that a driver calls this function with a 0 bpp_x16, and > ideally it should be fixed in the driver. However as it's a regression > and we don't have a better idea now: > > Acked-by: Imre Deak Chris: as this went into 6.8, please consider adding a stable-tag to ensure Greg picks this up. Ciao, Thorsten
Re: [REGRESSION] Divide-by-zero on DisplayPort MST unplug with nouveau
On 07.03.24 18:58, Chris Bainbridge wrote: > - Forwarded message from Chris Bainbridge > - > > Date: Sat, 10 Feb 2024 21:24:59 + Hmm, it looks like nobody is looking into this regression. Is there a good reason? Imre, or did you maybe just miss that Chris' regression seems to be caused by a commit of yours? He initally proposed a fix (the forwarded mail that is quoted here) more a month ago already here: https://lore.kernel.org/all/ZcfpqwnkSoiJxeT9@debian.local/ Chris recently filed a ticket, too: https://gitlab.freedesktop.org/drm/misc/kernel/-/issues/36 Mostly silence there as well. :-/ Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) P.S: Chris, sorry, I had missed that you initially proposed the fix a month ago; if I had noticed this earlier I had sent a mail like this one earlier. -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page. > From: Chris Bainbridge > To: dri-de...@lists.freedesktop.org > Cc: ly...@redhat.com, ville.syrj...@linux.intel.com, > stanislav.lisovs...@intel.com, > mrip...@kernel.org, imre.d...@intel.com > Subject: [PATCH] Fix divide-by-zero on DP unplug with nouveau > > The following trace occurs when using nouveau and unplugging a DP MST > adaptor: >> divide error: [#1] PREEMPT SMP PTI > CPU: 7 PID: 2962 Comm: Xorg Not tainted 6.8.0-rc3+ #744 > Hardware name: Razer Blade/DANA_MB, BIOS 01.01 08/31/2018 > RIP: 0010:drm_dp_bw_overhead+0xb4/0x110 [drm_display_helper] > Code: c6 b8 01 00 00 00 75 61 01 c6 41 0f af f3 41 0f af f1 c1 e1 04 48 63 > c7 31 d2 89 ff 48 8b 5d f8 c9 48 0f af f1 48 8d 44 06 ff <48> f7 f7 31 d2 31 > c9 31 f6 31 ff 45 31 c0 45 31 c9 45 31 d2 45 31 > RSP: 0018:b2c5c211fa30 EFLAGS: 00010206 > RAX: RBX: RCX: 00f59b00 > RDX: RSI: RDI: > RBP: b2c5c211fa48 R08: 0001 R09: 0020 > R10: 0004 R11: R12: 00023b4a > R13: 91d37d165800 R14: 91d36fac6d80 R15: 91d34a764010 > FS: 7f4a1ca3fa80() GS:91d6edbc() knlGS: > CS: 0010 DS: ES: CR0: 80050033 > CR2: 559491d49000 CR3: 00011d180002 CR4: 003706f0 > Call Trace: > > ? show_regs+0x6d/0x80 > ? die+0x37/0xa0 > ? do_trap+0xd4/0xf0 > ? do_error_trap+0x71/0xb0 > ? drm_dp_bw_overhead+0xb4/0x110 [drm_display_helper] > ? exc_divide_error+0x3a/0x70 > ? drm_dp_bw_overhead+0xb4/0x110 [drm_display_helper] > ? asm_exc_divide_error+0x1b/0x20 > ? drm_dp_bw_overhead+0xb4/0x110 [drm_display_helper] > ? drm_dp_calc_pbn_mode+0x2e/0x70 [drm_display_helper] > nv50_msto_atomic_check+0xda/0x120 [nouveau] > drm_atomic_helper_check_modeset+0xa87/0xdf0 [drm_kms_helper] > drm_atomic_helper_check+0x19/0xa0 [drm_kms_helper] > nv50_disp_atomic_check+0x13f/0x2f0 [nouveau] > drm_atomic_check_only+0x668/0xb20 [drm] > ? drm_connector_list_iter_next+0x86/0xc0 [drm] > drm_atomic_commit+0x58/0xd0 [drm] > ? __pfx___drm_printfn_info+0x10/0x10 [drm] > drm_atomic_connector_commit_dpms+0xd7/0x100 [drm] > drm_mode_obj_set_property_ioctl+0x1c5/0x450 [drm] > ? __pfx_drm_connector_property_set_ioctl+0x10/0x10 [drm] > drm_connector_property_set_ioctl+0x3b/0x60 [drm] > drm_ioctl_kernel+0xb9/0x120 [drm] > drm_ioctl+0x2d0/0x550 [drm] > ? __pfx_drm_connector_property_set_ioctl+0x10/0x10 [drm] > nouveau_drm_ioctl+0x61/0xc0 [nouveau] > __x64_sys_ioctl+0xa0/0xf0 > do_syscall_64+0x76/0x140 > ? do_syscall_64+0x85/0x140 > ? do_syscall_64+0x85/0x140 > entry_SYSCALL_64_after_hwframe+0x6e/0x76 > RIP: 0033:0x7f4a1cd1a94f > Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00 00 00 48 89 44 > 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05 <41> 89 c0 3d 00 f0 > ff ff 77 1f 48 8b 44 24 18 64 48 2b 04 25 28 00 > RSP: 002b:7ffd2f1df520 EFLAGS: 0246 ORIG_RAX: 0010 > RAX: ffda RBX: 7ffd2f1df5b0 RCX: 7f4a1cd1a94f > RDX: 7ffd2f1df5b0 RSI: c01064ab RDI: 000f > RBP: c01064ab R08: 56347932deb8 R09: 56347a7d99c0 > R10: R11: 0246 R12: 56347938a220 > R13: 000f R14: 563479d9f3f0 R15: > > Modules linked in: rfcomm xt_conntrack nft_chain_nat xt_MASQUERADE nf_nat > nf_conntrack_netlink nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 xfrm_user > xfrm_algo xt_addrtype nft_compat nf_tables nfnetlink br_netfilter bridge stp > llc ccm cmac algif_hash overlay algif_skcipher af_alg bnep binfmt_misc > snd_sof_pci_intel_cnl snd_sof_intel_hda_common snd_soc_hdac_hda snd_sof_pci > snd_sof_xtensa_dsp snd_sof_intel_hda snd_sof snd_sof_utils > snd_soc_acpi_intel_match snd_soc_acpi snd_soc_core snd_compress > snd_sof_intel_hda_mlink
Re: [Intel-gfx] [REGRESSION] HDMI connector detection broken in 6.3 on Intel(R) Celeron(R) N3060 integrated graphics
On 11.08.23 20:10, Mikhail Rudenko wrote: > On 2023-08-11 at 08:45 +02, Thorsten Leemhuis > wrote: >> On 10.08.23 21:33, Mikhail Rudenko wrote: >>> The following is a copy an issue I posted to drm/i915 gitlab [1] two >>> months ago. I repost it to the mailing lists in hope that it will help >>> the right people pay attention to it. >> >> Thx for your report. Wonder why Dmitry (who authored a4e771729a51) or >> Thomas (who committed it) it didn't look into this, but maybe the i915 >> devs didn't forward the report to them. For the record: they did, and Jani mentioned already. Sorry, should have phrased this differently. >> Let's see if these mails help. Just wondering: does reverting >> a4e771729a51 from 6.5-rc5 or drm-tip help as well? > > I've redone my tests with 6.5-rc5, and here are the results: > (1) 6.5-rc5 -> still affected > (2) 6.5-rc5 + revert a4e771729a51 -> not affected > (3) 6.5-rc5 + two patches [1][2] suggested on i915 gitlab by @ideak -> not > affected (!) > > Should we somehow tell regzbot about (3)? That's good to know, thx. But the more important things are: * When will those be merged? They are not yet in next yet afaics, so it might take some time to mainline them, especially at this point of the devel cycle. Imre, could you try to prod the right people so that these are ideally upstreamed rather sooner than later, as they fix a regression? * They if possible ideally should be tagged for backporting to 6.4, as this is a regression from the 6.3 cycle. But yes, let's tell regzbot that fixes are available, too: #regzbot fix: drm/i915: Fix HPD polling, reenabling the output poll work as needed (for the record: that's the second of two patches apparently needed) Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page. >> BTW, there was an earlier report about a problem with a4e771729a51 that >> afaics was never addressed, but it might be unrelated. >> https://lore.kernel.org/all/20230328023129.3596968-1-zhouzong...@kylinos.cn/ > [1] https://patchwork.freedesktop.org/patch/548590/?series=121050=1 > [2] https://patchwork.freedesktop.org/patch/548591/?series=121050=1
