From: Linus Torvalds <torvalds@linux-foundation.org>
To: Uros Bizjak <ubizjak@gmail.com>
Cc: x86@kernel.org, linux-kernel@vger.kernel.org,
Andy Lutomirski <luto@kernel.org>, Ingo Molnar <mingo@kernel.org>,
Nadav Amit <namit@vmware.com>, Brian Gerst <brgerst@gmail.com>,
Denys Vlasenko <dvlasenk@redhat.com>,
"H . Peter Anvin" <hpa@zytor.com>,
Peter Zijlstra <peterz@infradead.org>,
Thomas Gleixner <tglx@linutronix.de>,
Borislav Petkov <bp@alien8.de>,
Josh Poimboeuf <jpoimboe@redhat.com>
Subject: Re: [PATCH 4/4] x86/percpu: Use C for percpu read/write accessors
Date: Sun, 8 Oct 2023 13:13:54 -0700 [thread overview]
Message-ID: <CAHk-=whq=+LNHmsde8LaF4pdvKxqKt5GxW+Tq+U35_aDcV0ADg@mail.gmail.com> (raw)
In-Reply-To: <CAFULd4YWjxoSTyCtMN0OzKgHtshMQOuMH1Z0n_OaWKVnUjy2iA@mail.gmail.com>
On Sun, 8 Oct 2023 at 12:18, Uros Bizjak <ubizjak@gmail.com> wrote:
>
> Let me see what happens here. I have changed *only* raw_cpu_read_8,
> but the GP fault is reported in cpu_init_exception_handling, which
> uses this_cpu_ptr. Please note that all per-cpu initializations go
> through existing {this|raw}_cpu_write.
I think it's an ordering issue, and I think you may hit some issue
with loading TR od the GDT or whatever.
For example, we have this
set_tss_desc(cpu, &get_cpu_entry_area(cpu)->tss.x86_tss);
followed by
asm volatile("ltr %w0"::"q" (GDT_ENTRY_TSS*8));
in native_load_tr_desc(), and I think we might want to add a "memory"
clobber to it to make sure it is serialized with any stores to the GDT
entries in question.
I don't think *that* particular thing is the issue (because you kept
the writes as-is and still hit things), but I think it's an example of
some lazy inline asm constraints that could possibly cause problems if
the ordering changes.
And yes, this code ends up depending on things like
CONFIG_PARAVIRT_XXL for whether it uses the native TR loading or uses
some paravirt version, so config options can make a difference.
Again: I don't think it's that "ltr" instruction. I'm calling it out
just as a "that function does some funky things", and the load TR is
*one* of the funky things, and it looks like it could be the same type
of thing that then causes issues.
Things like CONFIG_SMP might also matter, because the percpu setup is
different. On UP, the *segment* use goes away, but I think the whole
"use inline asm vs regular memory ops" remains (admittedly I did *not*
verify that, I might be speaking out of my *ss).
Your dump does end up being close to a %gs access:
0: 4a 03 04 ed 40 19 15 add -0x7aeae6c0(,%r13,8),%rax
7: 85
8: 48 89 c7 mov %rax,%rdi
b: e8 9c bb ff ff call 0xffffffffffffbbac
10: 48 c7 c0 10 73 02 00 mov $0x27310,%rax
17: 48 ba 00 00 00 00 00 movabs $0xdffffc0000000000,%rdx
1e: fc ff df
21: 48 c1 e8 03 shr $0x3,%rax
25:* 80 3c 10 00 cmpb $0x0,(%rax,%rdx,1) <-- trapping instruction
29: 0f 85 21 05 00 00 jne 0x550
2f: 65 48 8b 05 45 26 f6 mov %gs:0x7ef62645(%rip),%rax # 0x7ef6267c
36: 7e
37: 48 8d 7b 24 lea 0x24(%rbx),%rdi
but I don't know what the "call" before is, so I wasn't able to match
it up with any obvious code in there.
Linus
next prev parent reply other threads:[~2023-10-08 20:23 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-04 14:49 [PATCH 0/4] x86/percpu: Use segment qualifiers Uros Bizjak
2023-10-04 14:49 ` [PATCH 1/4] x86/percpu: Update arch/x86/include/asm/percpu.h to the current tip Uros Bizjak
2023-10-04 14:49 ` [PATCH 2/4] x86/percpu: Enable named address spaces with known compiler version Uros Bizjak
2023-10-05 7:20 ` [tip: x86/percpu] " tip-bot2 for Uros Bizjak
2023-10-04 14:49 ` [PATCH 3/4] x86/percpu: Use compiler segment prefix qualifier Uros Bizjak
2023-10-05 7:20 ` [tip: x86/percpu] " tip-bot2 for Nadav Amit
2023-10-04 14:49 ` [PATCH 4/4] x86/percpu: Use C for percpu read/write accessors Uros Bizjak
2023-10-04 16:37 ` Ingo Molnar
2023-10-04 16:40 ` Ingo Molnar
2023-10-04 19:23 ` [PATCH v2 " Uros Bizjak
2023-10-04 19:42 ` Linus Torvalds
2023-10-04 20:07 ` Uros Bizjak
2023-10-04 20:12 ` Linus Torvalds
2023-10-04 20:19 ` Linus Torvalds
2023-10-04 20:22 ` Uros Bizjak
2023-10-05 7:06 ` Ingo Molnar
2023-10-05 7:40 ` Uros Bizjak
2023-10-05 7:20 ` [tip: x86/percpu] " tip-bot2 for Uros Bizjak
2023-10-08 17:59 ` [PATCH 4/4] " Linus Torvalds
2023-10-08 19:17 ` Uros Bizjak
2023-10-08 20:13 ` Linus Torvalds [this message]
2023-10-08 20:48 ` Linus Torvalds
2023-10-08 21:41 ` Uros Bizjak
2023-10-09 11:41 ` Ingo Molnar
2023-10-09 11:51 ` Ingo Molnar
2023-10-09 12:00 ` Uros Bizjak
2023-10-09 12:20 ` Ingo Molnar
2023-10-09 12:21 ` Nadav Amit
2023-10-09 12:42 ` Uros Bizjak
2023-10-09 12:53 ` Nadav Amit
2023-10-09 12:27 ` Uros Bizjak
2023-10-09 14:35 ` Uros Bizjak
2024-04-10 11:11 ` Andrey Konovalov
2024-04-10 11:21 ` Uros Bizjak
2024-04-10 11:24 ` Andrey Konovalov
2023-10-09 11:42 ` Ingo Molnar
2023-10-10 6:37 ` Uros Bizjak
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='CAHk-=whq=+LNHmsde8LaF4pdvKxqKt5GxW+Tq+U35_aDcV0ADg@mail.gmail.com' \
--to=torvalds@linux-foundation.org \
--cc=bp@alien8.de \
--cc=brgerst@gmail.com \
--cc=dvlasenk@redhat.com \
--cc=hpa@zytor.com \
--cc=jpoimboe@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mingo@kernel.org \
--cc=namit@vmware.com \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=ubizjak@gmail.com \
--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).