From: "Christian König" <christian.koenig@amd.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Jann Horn <jannh@google.com>,
Harry Wentland <harry.wentland@amd.com>,
Leo Li <sunpeng.li@amd.com>,
amd-gfx@lists.freedesktop.org,
Alex Deucher <alexander.deucher@amd.com>,
"David (ChunMing) Zhou" <David1.Zhou@amd.com>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
"H. Peter Anvin" <hpa@zytor.com>,
the arch/x86 maintainers <x86@kernel.org>,
kernel list <linux-kernel@vger.kernel.org>,
Josh Poimboeuf <jpoimboe@redhat.com>,
Andy Lutomirski <luto@kernel.org>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
mhiramat@kernel.org
Subject: Re: AMD DC graphics display code enables -mhard-float, -msse, -msse2 without any visible FPU state protection
Date: Fri, 10 Apr 2020 16:31:39 +0200 [thread overview]
Message-ID: <56fd0509-ebef-f7b5-3ca1-fc51ca493a4c@amd.com> (raw)
In-Reply-To: <20200409200125.GB2483@worktop.programming.kicks-ass.net>
Am 09.04.20 um 22:01 schrieb Peter Zijlstra:
> On Thu, Apr 09, 2020 at 08:15:57PM +0200, Christian König wrote:
>> Am 09.04.20 um 19:09 schrieb Peter Zijlstra:
>>> On Thu, Apr 09, 2020 at 05:59:56PM +0200, Peter Zijlstra wrote:
>>> [SNIP]
>>>> I'll need another approach, let me consider.
>>> Christian; it says these files are generated, does that generator know
>>> which functions are wholly in FPU context and which are not?
>> Well that "generator" is still a human being :)
>>
>> It's just that the formulae for the calculation come from the hardware team
>> and we are not able to easily transcript them to fixed point calculations.
> Well, if it's a human, can this human respect the kernel coding style a
> bit more :-) Some of that stuff is atrocious.
Yes, I know. That's unfortunately something we still need to work on as
well.
>> We are currently in the process of moving all the stuff which requires
>> floating point into a single C file(s) and then make sure that we only call
>> those within kernel_fpu_begin()/end() blocks.
> Can you make the build system stick all those .o files in a single
> archive? That's the only way I can do call validation; external
> relocatoin records do not contain the section.
Need to double check that with the display team responsible for the
code, but I think that shouldn't be much of a problem.
>> Annotating those function with __fpu or even saying to gcc that all code of
>> those files should go into a special text.fpu segment shouldn't be much of a
>> problem.
> Guess what the __fpu attribute does ;-)
Good to know that my suspicion how this is implemented was correct :)
> With the below patch (which is on to of newer versions of the objtool
> patches send earlier, let me know if you want a full set
Getting a branch somewhere would be perfect.
> ) that only
> converts a few files, but fully converts:
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c
>
> But building it (and this is an absolute pain; when you're reworking
> this, can you pretty please also fix the Makefiles?), we get:
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o: warning: objtool: dcn_validate_bandwidth()+0x34fa: FPU instruction outside of kernel_fpu_{begin,end}()
>
> $ ./scripts/faddr2line defconfig-build/drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o dcn_validate_bandwidth+0x34fa
> dcn_validate_bandwidth+0x34fa/0x57ce:
> dcn_validate_bandwidth at /usr/src/linux-2.6/defconfig-build/../drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:1293 (discriminator 5)
>
> # ./objdump-func.sh defconfig-build/drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o dcn_validate_bandwidth | grep 34fa
> 34fa 50fa: f2 0f 10 b5 60 ff ff movsd -0xa0(%rbp),%xmm6
>
> Which seems to indicate there's still problms with the current code.
Making an educated guess I would say the compiler has no idea that it
shouldn't use instructions which touch fp registers outside of
kernel_fpu_{begin,end}().
Going to talk with the display team about this whole topic internally
once more. Since this discussion already raised attention in our
technical management it shouldn't be to much of a problem to get
manpower to get this fixed properly.
Can we put this new automated check will be behind a configuration flag
initially? Or at least make it a warning and not a hard error.
Thanks,
Christian.
next prev parent reply other threads:[~2020-04-10 14:32 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-02 2:34 AMD DC graphics display code enables -mhard-float, -msse, -msse2 without any visible FPU state protection Jann Horn
2020-04-02 7:33 ` Christian König
2020-04-02 7:56 ` Jann Horn
2020-04-02 9:36 ` Thomas Gleixner
2020-04-02 14:50 ` Jann Horn
2020-04-02 14:13 ` Peter Zijlstra
2020-04-03 5:28 ` Masami Hiramatsu
2020-04-03 11:21 ` Peter Zijlstra
2020-04-04 3:08 ` Masami Hiramatsu
2020-04-04 3:15 ` Randy Dunlap
2020-04-04 8:32 ` Masami Hiramatsu
2020-04-04 14:32 ` Peter Zijlstra
2020-04-05 3:19 ` Masami Hiramatsu
2020-04-06 10:21 ` Peter Zijlstra
2020-04-07 9:50 ` Masami Hiramatsu
2020-04-07 11:15 ` Peter Zijlstra
2020-04-07 15:41 ` Masami Hiramatsu
2020-04-07 15:43 ` [PATCH] x86: insn: Add insn_is_fpu() Masami Hiramatsu
2020-04-07 15:54 ` AMD DC graphics display code enables -mhard-float, -msse, -msse2 without any visible FPU state protection Peter Zijlstra
2020-04-08 0:31 ` Masami Hiramatsu
2020-04-08 16:09 ` [PATCH v2] x86: insn: Add insn_is_fpu() Masami Hiramatsu
2020-04-09 14:32 ` Peter Zijlstra
2020-04-09 14:45 ` Peter Zijlstra
2020-04-10 0:47 ` Masami Hiramatsu
2020-04-10 1:22 ` [PATCH v3] " Masami Hiramatsu
2020-04-15 8:23 ` Masami Hiramatsu
2020-04-15 8:49 ` [PATCH v4] " Masami Hiramatsu
2020-04-04 14:36 ` AMD DC graphics display code enables -mhard-float, -msse, -msse2 without any visible FPU state protection Peter Zijlstra
2020-04-05 3:37 ` Masami Hiramatsu
2020-04-09 15:59 ` Peter Zijlstra
2020-04-09 17:09 ` Peter Zijlstra
2020-04-09 18:15 ` Christian König
2020-04-09 20:01 ` Peter Zijlstra
2020-04-10 14:31 ` Christian König [this message]
2020-04-15 9:16 ` Peter Zijlstra
2020-04-17 20:27 ` Rodrigo Siqueira
2020-04-17 21:56 ` Peter Zijlstra
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56fd0509-ebef-f7b5-3ca1-fc51ca493a4c@amd.com \
--to=christian.koenig@amd.com \
--cc=David1.Zhou@amd.com \
--cc=acme@kernel.org \
--cc=alexander.deucher@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=bp@alien8.de \
--cc=harry.wentland@amd.com \
--cc=hpa@zytor.com \
--cc=jannh@google.com \
--cc=jpoimboe@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mhiramat@kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=sunpeng.li@amd.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).