https://bugs.freedesktop.org/show_bug.cgi?id=101387

Michel Dänzer michel@daenzer.net changed:

What |Removed |Added ---------------------------------------------------------------------------- Attachment #131890|text/x-log |text/plain mime type| |

https://bugs.freedesktop.org/show_bug.cgi?id=101387

--- Comment #2 from Michel Dänzer michel@daenzer.net --- Looks like amdgpu_atombios_crtc_powergate_init scribbles over the stack.

(In reply to Carlo Caione from comment #1)

If I recompile the kernel forcing CONFIG_CC_STACKPROTECTOR_NONE == none I don't have the panic anymore (of course) but I can still see the corruption at boot (and weird artifacts on plymouth).

What kind of artifacts in plymouth? Does e.g. X work correctly?

https://bugs.freedesktop.org/show_bug.cgi?id=101387

--- Comment #4 from Michel Dänzer michel@daenzer.net --- (In reply to Carlo Caione from comment #3)

...

What kind of artifacts in plymouth?

https://pasteboard.co/hWu18yPj0.jpg

That could be a plymouth bug. I've noticed that it doesn't seem to call drmModeCrtcSetGamma to make sure the display CLUTs are initialized to appropriate values.

https://bugs.freedesktop.org/show_bug.cgi?id=101387

--- Comment #6 from Carlo Caione carlo@caione.org --- Uhm, probably I have found something.

In amdgpu_atombios_crtc_powergate_init() we are declaring

ENABLE_DISP_POWER_GATING_PARAMETERS_V2_1 args;

so that args is basically a 32byte struct. We are passing down this struct to amdgpu_atom_execute_table() casting it to (uint32_t *). This address is then assigned to (uint32_t *) ectx.ps in amdgpu_atom_execute_table_locked().

At a certain point during the execution of the code in the table with index = 75, atom_put_dst() is called with argument ATOM_ARG_PS and index == 1. So we are doing:

ctx->ps[idx] = cpu_to_le32(val);

but being idx == 1, we are accessing over the boundaries of args, so triggering the stack corruption.

Is this analysis correct and if it is how can we fix this?

https://bugs.freedesktop.org/show_bug.cgi?id=101387

--- Comment #8 from Carlo Caione carlo@caione.org --- Just a better description of what's going on and a couple of questions.

When amdgpu_atombios_crtc_powergate_init() is called this triggers the parsing of the command table with index == 13 (>> execute C5C0 (len 589, WS 0, PS 0)). As already reported the parameter space used (struct ENABLE_DISP_POWER_GATING_PARAMETERS_V2_1) is 32 bytes wide.

During the execution of this table several CALL_TABLE (op == 82) are executed.

In particular we first just to table with index == 78 (>> execute F166 (len 588, WS 0, PS 8)), then to table with index == 51 (>> execute F446 (len 465, WS 4, PS 4)) and finally to table with index == 75 (>> execute F6CC (len 1330, WS 4, PS 0)) before finally reaching the EOT for table 13.

During the execution of table 75 a MOVE_PS is executed with a destination index == 1, accessing ctx->ps[idx] and causing the stack corruption.

So either the atombios code is wrong or the atombios interpreter in the kernel is doing something wrong.

I also have a couple of questions / observations:

1) Table 75 has WS == 4 and PS == 0 and looking at the opcodes in the table I basically have only *_WS opcodes (MOVE_WS, TEST_WS, ADD_WS, etc...) and just two *_PS instructions (MOVE_PS and OR_PS) that (guess what) are the instructions causing the stack corruption. My guess here is that the opcodes *_PS in the atombios are wrong and they should actually be *_WS opcodes.

2) Don't we need to allocate the size of the ps allocation struct for the command table we are going to execute after a CALL_TABLE matching the ps size in the table header? IIUC the code in the kernel, when we are jumping to a different table ctx->ps is not being reallocated.

3) Could the point at (2) also be a problem in our case? Assuming that ps read from the table header has something to do with the size of the parameter space (guessing here) Table 13 has PS == 0, while table 75 has PS == 4 whereas both are using the same ctx->ps.

https://bugs.freedesktop.org/show_bug.cgi?id=101387

--- Comment #10 from Alex Deucher alexdeucher@gmail.com --- Created attachment 131982 --> https://bugs.freedesktop.org/attachment.cgi?id=131982&action=edit possible fix

This patch should fix the issue.

https://bugs.freedesktop.org/show_bug.cgi?id=101387

--- Comment #12 from Carlo Caione carlo@caione.org --- Also (in case it is useful for the corruption issue):

00:01.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI] Device 9874 (rev ca) ... 03:00.0 Display controller: Advanced Micro Devices, Inc. [AMD/ATI] Device 699f (rev ff)

# echo 1 > /sys/bus/pci/devices/0000:00:01.0/rom # cat /sys/bus/pci/devices/0000:00:01.0/rom > /tmp/vbios.rom cat: /sys/bus/pci/devices/0000:00:01.0/rom: Input/output error

...

# echo 1 > /sys/bus/pci/devices/0000:03:00.0/rom # cat /sys/bus/pci/devices/0000:03:00.0/rom > /tmp/vbios.rom cat: /sys/bus/pci/devices/0000:03:00.0/rom: Input/output error

https://bugs.freedesktop.org/show_bug.cgi?id=101387

--- Comment #14 from Carlo Caione carlo@caione.org ---

not likely related.

Alright, any hint where to start looking? As I said before it looks like the visual corruption (this https://pasteboard.co/qrC9mh4p.jpg) at boot starts at

[drm] amdgpu atom DIG backlight initialized [drm] AMDGPU Display Connectors [drm] Connector 0: [drm] eDP-1 [drm] HPD1 [drm] DDC: 0x4868 0x4868 0x4869 0x4869 0x486a 0x486a 0x486b 0x486b [drm] Encoders: [drm] LCD1: INTERNAL_UNIPHY [drm] Connector 1: [drm] HDMI-A-1 [drm] HPD2 [drm] DDC: 0x486c 0x486c 0x486d 0x486d 0x486e 0x486e 0x486f 0x486f [drm] Encoders: [drm] DFP1: INTERNAL_UNIPHY [drm] amdgpu atom DIG backlight initialized [drm] AMDGPU Display Connectors [drm] Connector 0: [drm] eDP-1 [drm] HPD1 [drm] DDC: 0x4868 0x4868 0x4869 0x4869 0x486a 0x486a 0x486b 0x486b [drm] Encoders: [drm] LCD1: INTERNAL_UNIPHY [drm] Connector 1: [drm] HDMI-A-1 [drm] HPD2 [drm] DDC: 0x486c 0x486c 0x486d 0x486d 0x486e 0x486e 0x486f 0x486f [drm] Encoders: [drm] DFP1: INTERNAL_UNIPHY

Fortunately it disappears after ~2 seconds when plymouth kicks in. My impression is that the visual corruption is triggered again when interpreting some atombios code, not really easy to debug.

https://bugs.freedesktop.org/show_bug.cgi?id=101387

--- Comment #16 from Carlo Caione carlo@caione.org --- I see. The problem is that the garbage lines result in a very bad experience for the user. Do you have any idea how to fix / mitigate that?

https://bugs.freedesktop.org/show_bug.cgi?id=101387

--- Comment #18 from Alex Deucher alexdeucher@gmail.com --- The VCE regression is fixed. If you want to try those patches, they are available here: https://cgit.freedesktop.org/~agd5f/linux/log/?h=drm-next-4.13-wip

https://bugs.freedesktop.org/show_bug.cgi?id=101387

--- Comment #20 from Carlo Caione carlo@caione.org ---

That could be a plymouth bug. I've noticed that it doesn't seem to call drmModeCrtcSetGamma to make sure the display CLUTs are initialized to appropriate values.

Can you expand on this a little bit?

I have (by chance TBH) fixed plymouth basically reading the CLUTs using drmModeCrtcGetGamma() when plymouthd starts and writing them back using drmModeCrtcSetGamma() before drmModeSetCrtc() is called when the reboot splash has to be displayed.

But it's not clear to me why this is needed at all since in theory I'm writing back the same CLUTs values read by drmModeCrtcGetGamma() but at the same time if I do not do that, I have image corruption.

Any hint about this?

https://bugs.freedesktop.org/show_bug.cgi?id=101387

--- Comment #22 from Carlo Caione carlo@caione.org ---

Plymouth shouldn't get the values with drmModeCrtcGetGamma — there's no guarantee that the CLUT contains suitable values at that point (e.g. the hardware could currently be programmed for a different colour bit depth in the first place).

Ok, but at this point I wonder (sorry, not really familiar with libdrm) why the value reported by drmModeCrtcGetGamma are not coherent with the values currently programmed in hardware. Is this a common thing?

If __immediately__ before drmModeSetCrtc I do drmModeCrtcGetGamma + drmModeCrtcSetGamma with the values retrieved, everything works fine with the correct colors, so the CLUTs returned by drmModeCrtcGetGamma actually contain suitable / correct values but apparently these are different from what is programmed in the hardware.

https://bugs.freedesktop.org/show_bug.cgi?id=101387

--- Comment #24 from Carlo Caione carlo@caione.org ---

Sounds like there is also a bug somewhere in the kernel (driver), but even if that was fixed, it still wouldn't solve the plymouth issue in all cases.

Yeah, so what should be the right way of proceeding here? I image something like: - Retrieve the gamma size from the kernel with drmModeGetCrtc() - Build a linear ramp using the correct size - Set it with drmModeCrtcSetGamma() - Call drmModeSetCrtc()

https://bugs.freedesktop.org/show_bug.cgi?id=101387

Martin Peres martin.peres@free.fr changed:

What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution|--- |MOVED

--- Comment #26 from Martin Peres martin.peres@free.fr --- -- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/drm/amd/issues/190.