linux-kernel - Re: [PATCH 01/12] drm/amdgpu: Write masked value to control register

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <92eebfd3-4234-b3fa-87cc-c48b3deec33f@igalia.com>
Date:   Thu, 14 Jul 2022 16:04:35 -0300
From:   André Almeida <andrealmeid@...lia.com>
To:     Maíra Canal <mairacanal@...eup.net>
Cc:     magalilemes00@...il.com, Xinhui.Pan@....com,
        Dmytro Laktyushkin <Dmytro.Laktyushkin@....com>,
        Nicholas Kazlauskas <nicholas.kazlauskas@....com>,
        Daniel Vetter <daniel@...ll.ch>,
        David Airlie <airlied@...ux.ie>,
        Alex Deucher <alexander.deucher@....com>,
        Rodrigo Siqueira <Rodrigo.Siqueira@....com>,
        Aurabindo Pillai <aurabindo.pillai@....com>,
        Harry Wentland <harry.wentland@....com>,
        tales.aparecida@...il.com, linux-kernel@...r.kernel.org,
        amd-gfx@...ts.freedesktop.org, mwen@...lia.com,
        Isabella Basso <isabbasso@...eup.net>,
        christian.koenig@....com, andrealmeid@...eup.net,
        Leo Li <sunpeng.li@....com>
Subject: Re: [PATCH 01/12] drm/amdgpu: Write masked value to control register

Hi Maíra,

Thank you for your patch,

Às 13:44 de 14/07/22, Maíra Canal escreveu:
> On the dce_v6_0 and dce_v8_0 hpd tear down callback, the tmp variable
> should be written into the control register instead of 0.
> 

Why? I do see that tmp was unused before your patch, but why should we
write it into this register? Did you manage to test this somehow?

> Fixes: b00861b9 ("drm/amd/amdgpu: port of DCE v6 to new headers (v3)")
> Fixes: 2285b91c ("drm/amdgpu/dce8: simplify hpd code")

Checking both commits, I can see that 0 is written at `mmDC_HPD1_CONTROL
+ N` register in _hpd_fini() in them, so if you are trying to fix the
commit that inserted that behavior, I think aren't those ones. For instance:

$ git show 2285b91c

[...]

@@ -479,28 +372,11 @@ static void dce_v8_0_hpd_fini(struct amdgpu_device
*adev)
        list_for_each_entry(connector, &dev->mode_config.connector_list,
head) {
                struct amdgpu_connector *amdgpu_connector =
to_amdgpu_connector(connector);

-               switch (amdgpu_connector->hpd.hpd) {
-               case AMDGPU_HPD_1:
-                       WREG32(mmDC_HPD1_CONTROL, 0);
-                       break;
-               case AMDGPU_HPD_2:
-                       WREG32(mmDC_HPD2_CONTROL, 0);
-                       break;
-               case AMDGPU_HPD_3:
-                       WREG32(mmDC_HPD3_CONTROL, 0);
-                       break;
-               case AMDGPU_HPD_4:
-                       WREG32(mmDC_HPD4_CONTROL, 0);
-                       break;
-               case AMDGPU_HPD_5:
-                       WREG32(mmDC_HPD5_CONTROL, 0);
-                       break;
-               case AMDGPU_HPD_6:
-                       WREG32(mmDC_HPD6_CONTROL, 0);
-                       break;
-               default:
-                       break;
-               }
+               if (amdgpu_connector->hpd.hpd >= adev->mode_info.num_hpd)
+                       continue;
+
+               WREG32(mmDC_HPD1_CONTROL +
hpd_offsets[amdgpu_connector->hpd.hpd], 0);
+

0 was the valued written here before this commit. The commit basically
replaces the switch case with a sum in this snippet.

thanks,
	andré