All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Nick Desaulniers <ndesaulniers@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Nathan Chancellor <nathan@kernel.org>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
	kvm@vger.kernel.org, llvm@lists.linux.dev,
	linux-kernel@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	syzbot+6cde2282daa792c49ab8@syzkaller.appspotmail.com
Subject: Re: [PATCH 1/5] Kconfig: Add option for asm goto w/ tied outputs to workaround clang-13 bug
Date: Tue, 1 Feb 2022 20:56:20 +0000	[thread overview]
Message-ID: <YfmedCAn8pK//I2R@google.com> (raw)
In-Reply-To: <CAKwvOd=9nwR7z7wn50SU=mf5AywFLd95ZMH-EbYdHfbeHVvq1A@mail.gmail.com>

On Tue, Feb 01, 2022, Nick Desaulniers wrote:
> On Mon, Jan 31, 2022 at 5:08 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > Add a config option to guard (future) usage of asm_volatile_goto() that
> > includes "tied outputs", i.e. "+" constraints that specify both an input
> > and output parameter.  clang-13 has a bug[1] that causes compilation of
> > such inline asm to fail, and KVM wants to use a "+m" constraint to
> > implement a uaccess form of CMPXCHG[2].  E.g. the test code fails with
> >
> >   <stdin>:1:29: error: invalid operand in inline asm: '.long (${1:l}) - .'
> >   int foo(int *x) { asm goto (".long (%l[bar]) - .\n": "+m"(*x) ::: bar); return *x; bar: return 0; }
> >                             ^
> >   <stdin>:1:29: error: unknown token in expression
> >   <inline asm>:1:9: note: instantiated into assembly here
> >           .long () - .
> >                  ^
> >   2 errors generated.
> >
> > on clang-13, but passes on gcc (with appropriate asm goto support).  The
> > bug is fixed in clang-14, but won't be backported to clang-13 as the
> > changes are too invasive/risky.
> 
> LGTM.
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> 
> If you're going to respin the series, consider adding a comment in the
> source along the lines of:
> ```
> clang-14 and gcc-11 fixed this.
> ```
> or w/e. This helps us find (via grep) and remove cruft when the
> minimum supported compiler versions are updated.

Will do, a new version is definitely needed.

> Note: gcc-10 had a bug with the symbolic references to labels when
> using tied constraints.
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98096
> 
> Both compilers had bugs here, and it may be worth mentioning that in
> the commit message.

Is this wording accurate?

  gcc also had a similar bug[3], fixed in gcc-11, where gcc failed to
  account for its behavior of assigning two numbers to tied outputs (one
  for input, one for output) when evaluating symbolic references. 

  reply	other threads:[~2022-02-01 20:56 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-01  1:08 [PATCH 0/5] x86: uaccess CMPXCHG + KVM bug fixes Sean Christopherson
2022-02-01  1:08 ` [PATCH 1/5] Kconfig: Add option for asm goto w/ tied outputs to workaround clang-13 bug Sean Christopherson
2022-02-01 20:16   ` Nick Desaulniers
2022-02-01 20:56     ` Sean Christopherson [this message]
2022-02-01 21:15       ` Nick Desaulniers
2022-02-01  1:08 ` [PATCH 2/5] x86/uaccess: Implement macros for CMPXCHG on user addresses Sean Christopherson
2022-02-01  1:08 ` [PATCH 3/5] KVM: x86: Use __try_cmpxchg_user() to update guest PTE A/D bits Sean Christopherson
2022-02-01  7:01   ` kernel test robot
2022-02-01  7:01     ` kernel test robot
2022-02-01 19:44     ` Sean Christopherson
2022-02-01 19:44       ` Sean Christopherson
2022-02-01 19:53       ` Nick Desaulniers
2022-02-01 19:53         ` Nick Desaulniers
2022-02-01 13:25   ` kernel test robot
2022-02-01 13:25     ` kernel test robot
2022-02-01  1:08 ` [PATCH 4/5] KVM: x86: Use __try_cmpxchg_user() to emulate atomic accesses Sean Christopherson
2022-02-01  9:25   ` kernel test robot
2022-02-01  9:25     ` kernel test robot
2022-02-01  1:08 ` [PATCH 5/5] KVM: x86: Bail to userspace if emulation of atomic user access faults Sean Christopherson
2022-02-01 17:09 ` [PATCH 0/5] x86: uaccess CMPXCHG + KVM bug fixes Tadeusz Struk

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=YfmedCAn8pK//I2R@google.com \
    --to=seanjc@google.com \
    --cc=jmattson@google.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=syzbot+6cde2282daa792c49ab8@syzkaller.appspotmail.com \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.