xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	"Wei Liu" <wl@xen.org>, "Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [PATCH v2 5/6] x86: guard against straight-line speculation past RET
Date: Tue, 13 Oct 2020 15:34:57 +0200	[thread overview]
Message-ID: <f5be4c67-95ad-dd98-cd24-b925da3ef519@suse.com> (raw)
In-Reply-To: <447525bc-662d-ff52-6b73-e6e1a61767ec@citrix.com>

On 13.10.2020 14:00, Andrew Cooper wrote:
> On 28/09/2020 13:31, Jan Beulich wrote:
>> --- a/xen/Makefile
>> +++ b/xen/Makefile
>> @@ -145,7 +145,15 @@ t2 = $(call as-insn,$(CC) -I$(BASEDIR)/i
>>  # https://bugs.llvm.org/show_bug.cgi?id=36110
>>  t3 = $(call as-insn,$(CC),".macro FOO;.endm"$(close); asm volatile $(open)".macro FOO;.endm",-no-integrated-as)
>>  
>> -CLANG_FLAGS += $(call or,$(t1),$(t2),$(t3))
>> +# Check whether \(text) escaping in macro bodies is supported.
>> +t4 = $(call as-insn,$(CC),".macro m ret:req; \\(ret) $$\\ret; .endm; m 8",,-no-integrated-as)
>> +
>> +# Check whether macros can override insn mnemonics in inline assembly.
>> +t5 = $(call as-insn,$(CC),".macro ret; .error; .endm; .macro retq; .error; .endm",-no-integrated-as)
>> +
>> +acc1 := $(call or,$(t1),$(t2),$(t3),$(t4))
>> +
>> +CLANG_FLAGS += $(call or,$(acc1),$(t5))
> 
> I'm not happy taking this until there is toolchain support visible in
> the future.
> 
> We *cannot* rule out the use of IAS forever more, because there are
> features far more important than ret speculation which depend on it.

So what do you suggest? We can't have both, afaics, so we need to
pick either being able to use the integrated assembler or being
able to guard RET.

>> --- a/xen/include/asm-x86/asm-defns.h
>> +++ b/xen/include/asm-x86/asm-defns.h
>> @@ -50,3 +50,22 @@
>>  .macro INDIRECT_JMP arg:req
>>      INDIRECT_BRANCH jmp \arg
>>  .endm
>> +
>> +/*
>> + * To guard against speculation past RET, insert a breakpoint insn
>> + * immediately after them.
>> + */
>> +.macro ret operand:vararg
>> +    ret$ \operand
>> +.endm
>> +.macro retq operand:vararg
>> +    ret$ \operand
>> +.endm
>> +.macro ret$ operand:vararg
>> +    .purgem ret
>> +    ret \operand
> 
> You're substituting retq for ret, which defeats the purpose of unwrapping.

I'm afraid I don't understand the "defeats" aspect.

> I will repeat my previous feedback.  Do away with this
> wrapping/unwrapping and just use a raw .byte.  Its simpler and faster.

Well, I could now also repeat my prior response to your prior
feedback, but since iirc you didn't reply back then I don't
expect you would now. Instead I'll - once again - give in despite
my believe that this is the cleaner approach, and that in cases
like this one - when there are pros and cons to either approach -
it should be the author's choice rather than the reviewer's.

Jan


  reply	other threads:[~2020-10-13 13:35 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-28 12:28 [PATCH v2 0/6] x86: some assembler macro rework Jan Beulich
2020-09-28 12:29 ` [PATCH v2 1/6] x86: replace __ASM_{CL,ST}AC Jan Beulich
2020-10-13 11:04   ` Andrew Cooper
2020-10-13 11:21     ` Jan Beulich
2020-09-28 12:30 ` [PATCH v2 2/6] x86: reduce CET-SS related #ifdef-ary Jan Beulich
2020-09-28 12:37   ` Jan Beulich
2020-10-13 11:40     ` Andrew Cooper
2020-10-13 13:24       ` Jan Beulich
2020-10-13 11:20   ` Andrew Cooper
2020-10-13 11:26     ` Jan Beulich
2020-09-28 12:30 ` [PATCH v2 3/6] x86: drop ASM_{CL,ST}AC Jan Beulich
2020-09-28 12:30 ` [PATCH v2 4/6] x86: fold indirect_thunk_asm.h into asm-defns.h Jan Beulich
2020-09-28 12:31 ` [PATCH v2 5/6] x86: guard against straight-line speculation past RET Jan Beulich
2020-10-08 16:28   ` Roger Pau Monné
2020-10-13 10:33     ` Jan Beulich
2020-10-13 12:00   ` Andrew Cooper
2020-10-13 13:34     ` Jan Beulich [this message]
2020-09-28 12:32 ` [PATCH v2 6/6] x86: limit amount of INT3 in IND_THUNK_* Jan Beulich
2020-10-08 16:35   ` Roger Pau Monné
2020-10-13 12:58   ` Andrew Cooper
2020-09-28 12:34 ` [PATCH v2 0/6] x86: some assembler macro rework Jan Beulich

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=f5be4c67-95ad-dd98-cd24-b925da3ef519@suse.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=roger.pau@citrix.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.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).