Re: [PATCH] x86: Apply standalone prefixes to the following instruction

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Fri, Jul 19, 2019 at 9:25 AM Jan Beulich <JBeulich@suse.com> wrote:
>
> On 19.07.2019 17:01,  H.J. Lu  wrote:
> > On Fri, Jul 19, 2019 at 1:46 AM Jan Beulich <JBeulich@suse.com> wrote:
> >>
> >> On 19.07.2019 00:26,  H.J. Lu  wrote:
> >>> Standalone prefixes should be applied to the following instruction,
> >>> instead of being treated as regular instructions.  An error should be
> >>> issued when a standalone prefix is at the end of source or isn't
> >>> followed by an instruction in the same section.
> >>
> >> Commenting here, because commenting on the actual code fragments is
> >> not easily possible with the patch sent as attachment.
> >>
> >> For one, I don't agree that errors should be issued when switching
> >> sections. Clever assembly programming can easily result in the
> >> actual section later getting resumed, and an appropriate insn being
> >> there.
> >
> > Yes, you will get an error which can be easily fixed.
>
> For both this and ...
>
> >> And then I'm getting the impression that the change here is going
> >> to break things like
> >>
> >> static inline unsigned int find_first_set_bit(unsigned long word)
> >> {
> >>       asm ( "rep; bsf %1,%0" : "=r" (word) : "rm" (word) );
> >>       return (unsigned int)word;
> >> }
> >
> > "rep; bsf" works like "rep bsf".
> >
> >> (quoted from Xen sources), being a backwards compatible
> >> representation of tzcnt. Just like such have shown up in the past,
> >> REP prefixes could easily obtain meaning for other insns going
> >> forward, so tagging individual templates with RepPrefixOk is not
> >> going to help. WBNOINVD is a pretty recent example.
> >>
> >
> > Since adding REP to random instructions may lead to different instructions,
> > RepPrefixOk is used to prevent that.  If one really wants different
> > instructions,
> > ".byte 0xf3" can be used.
>
> ... this you realize that breaking existing code is bad? It doesn't
> matter how "easy" it is to fix such. Taking Xen (again) as the example,
> older trees are supposed to not be touched anymore except for security
> fixes. Now if people upgrade their underlying distros, builds of these
> older trees will suddenly start to fail.
>
> Furthermore, with your ".byte 0xf3" suggestion, what "protection" do
> you achieve when disallowing "rep", but allowing ".byte 0xf3"? Plus
> personally I consider the .byte variant quite a bit worse.

It will discourage using old assembler for new instructions.  Of course,
one can use ".byte" for anything.

> Finally, with a number of changes of mine (including the still pending
> operand size default changes which I'm slowly making progress with)
> you've been demanding that the Linux build not be broken. But just
> like Xen, Linux too uses "rep; bsf".
>

"rep; bsf" works fine since bsf has RepPrefixOk.  If REP should be
allowed for any instructions, shouldn't RepPrefixOk be removed?

-- 
H.J.