From: Dave Hansen <dave.hansen@intel.com>
To: "Chang S. Bae" <chang.seok.bae@intel.com>,
linux-kernel@vger.kernel.org, x86@kernel.org,
linux-pm@vger.kernel.org
Cc: tglx@linutronix.de, dave.hansen@linux.intel.com,
peterz@infradead.org, bp@alien8.de, rafael@kernel.org,
ravi.v.shankar@intel.com
Subject: Re: [PATCH v3 1/3] x86/fpu: Make XCR0 accessors immune to unwanted compiler reordering
Date: Fri, 1 Apr 2022 11:16:19 -0700 [thread overview]
Message-ID: <1cb5ccc3-a84d-06da-a8e8-d9d3d8418d25@intel.com> (raw)
In-Reply-To: <ee82ed75-03f5-1962-6888-11491c9569c8@intel.com>
On 4/1/22 10:58, Dave Hansen wrote:
> On 3/24/22 19:22, Chang S. Bae wrote:
>> Some old GCC versions (4.9.x and 5.x) have an issue that incorrectly
>> reordering volatile asm statements with each other [1]. While this bug was
>> fixed on later versions (8.1, 7.3, and 6.5), and the kernel's current XCR0
>> read/write do not appear to be impacted, it is preventive to ensure them on
>> the program order.
> I thought you *actually* saw xgetbv() be reordered, though. Was that on
> a buggy compiler?
Actually, reading the gcc bug[1] a bit more, it says:
> However, correctness here depends on the compiler honouring the
> ordering of volatile asm statements with respect to other volatile
> code, and this patch fixes that.
In your case, there was presumably no volatile code, just a
fpu_state_size_dynamic() call. The compiler thought the xgetbv() was
safe to reorder, and did so.
So, I'm not quite sure how that bug is relevant. It just dealt with
multiple "asm volatile" statements that don't have memory clobbers. We
only have one "asm volatile" in play. Adding the memory clobber should
make that bug totally irrelevant.
[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82602
next prev parent reply other threads:[~2022-04-01 18:16 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-25 2:22 [PATCH v3 0/3] x86/fpu: Make AMX state ready for CPU idle Chang S. Bae
2022-03-25 2:22 ` [PATCH v3 1/3] x86/fpu: Make XCR0 accessors immune to unwanted compiler reordering Chang S. Bae
2022-04-01 17:58 ` Dave Hansen
2022-04-01 18:16 ` Dave Hansen [this message]
2022-04-01 22:14 ` Chang S. Bae
2022-03-25 2:22 ` [PATCH v3 2/3] x86/fpu: Add a helper to prepare AMX state for low-power CPU idle Chang S. Bae
2022-04-03 16:37 ` Thomas Gleixner
2022-04-05 0:24 ` Chang S. Bae
2022-03-25 2:22 ` [PATCH v3 3/3] intel_idle: Add a new flag to initialize the AMX state Chang S. Bae
2022-03-29 17:42 ` Rafael J. Wysocki
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=1cb5ccc3-a84d-06da-a8e8-d9d3d8418d25@intel.com \
--to=dave.hansen@intel.com \
--cc=bp@alien8.de \
--cc=chang.seok.bae@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=rafael@kernel.org \
--cc=ravi.v.shankar@intel.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
all inboxes | Powered by JetHome®