[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+rthh9Lorjyeuq3O50c7ABeuX0j+vKh9ZFe96+gHKc76oMfeg@mail.gmail.com>
Date: Thu, 5 Oct 2017 15:01:48 +0200
From: Mathias Krause <minipli@...glemail.com>
To: Michael Matz <matz@...e.de>
Cc: Borislav Petkov <bp@...en8.de>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
"H. Peter Anvin" <hpa@...or.com>, x86-ml <x86@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] x86/alternatives: Fix alt_max_short macro to really be a max()
On 5 October 2017 at 14:52, Michael Matz <matz@...e.de> wrote:
> On Thu, 5 Oct 2017, Borislav Petkov wrote:
>> On Thu, Oct 05, 2017 at 02:35:33PM +0200, Mathias Krause wrote:
>> > Note the "<"! ...comment is wrong, though the implementation works!
>>
>> I know, I realized that when I looked at alternative-asm.h. Wanted to
>> double-check it with Micha first.
>
> Yeah, for bit-twiddling the result of arithmetic would need to be
> booleanized first, or alternatively the boolean operators be used in the
> first place. So if '<' works then that's better in this context.
Ack!
> (In a different context, or in the same one there definitely was a problem
> with using '<', but I can't remember the details, it's too long ago we
> discussed about this; maybe it even was a problem only with some binutils
> versions. So I'd suggest using the more obvious way until problems
> reoccur, and then document why exactly using relational ops was a problem
> ;-) )
Might be because the "true" value of gas' < operator is non-obvious
with being -1. But, well, if you don't know, I don't know either ;)
Anyways, the alt_max_short() macro in alternative.h is plain wrong
while the one in alternative-asm.h works and is used in a few places,
even with varying lengths of the alternatives (e.g.
arch/x86/entry/vdso/vdso32/system_call.S,
arch/x86/lib/{clear_page_64.S,memcpy_64.S,memset_64.S}), therefore
proves to be functional.
Thanks,
Mathias
Powered by blists - more mailing lists