Date:   Thu, 24 Feb 2022 21:13:25 +1000
From:   Nicholas Piggin <npiggin@...il.com>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Anders Roxell <anders.roxell@...aro.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        "# 3.4.x" <stable@...r.kernel.org>
Subject: Re: [PATCH 2/3] powerpc: fix build errors

Excerpts from Arnd Bergmann's message of February 24, 2022 8:20 pm:
> On Thu, Feb 24, 2022 at 11:11 AM Nicholas Piggin <npiggin@...il.com> wrote:
>> Excerpts from Arnd Bergmann's message of February 24, 2022 6:55 pm:
>> > On Thu, Feb 24, 2022 at 6:05 AM Nicholas Piggin <npiggin@...il.com> wrote:
>> > We had the same thing on Arm a few years ago when binutils
>> > started enforcing this more strictly, and it does catch actual
>> > bugs. I think annotating individual inline asm statements is
>> > the best choice here, as that documents what the intention is.
>>
>> A few cases where there are differences in privileged instructions
>> (that won't be compiler generated), that will be done anyway.
>>
>> For new instructions added to the ISA though? I think it's ugly and
>> unecesaary. There is no ambiguity about the intention when you see
>> a lharx instruction is there?
>>
>> It would delinate instructions that can't be used on all processors
>> but I don't see  much advantage there, it's not an exhaustive check
>> because we have other restrictions on instructions in the kernel
>> environment. And why would inline asm be special but not the rest
>> of the asm? Would you propose to put these .machine directives
>> everywhere in thousands of lines of asm code in the kernel? I
>> don't know that it's an improvement. And inline asm is a small
>> fraction of instructions.
> 
> Most of the code is fine, as we tend to only build .S files that
> are for the given target CPU,

That's not true on powerpc at least. grep FTR_SECTION.

Not all of them are different ISA, but it's more than just the
CPU_FTR_ARCH ones which only started about POWER7.

> the explicit .machine directives are
> only needed when you have a file that mixes instructions for
> incompatible machines, using a runtime detection.

Right. There are .S files are in that category. And a lot of
it for inline and .S we probably skirt entirely due to using raw 
instruction encoding because of old toolchains (which gets no error 
checking at all) which we really should tidy up and trim.

> 
>> Right that should be caught if you just pass -m<superset> architecture
>> to the assembler that does not include the mtpmr. 32-bit is a lot more
>> complicated than 64s like this though, so it's pssible in some cases
>> you will want more checking and -m<subset> + some .machine directives
>> will work better.
>>
>> Once you add the .machine directive to your inline asm though, you lose
>> *all* such static checking for the instruction. So it's really not a
>> panacea and has its own downsides.
> 
> Again, there should be a minimum number of those .machine directives
> in inline asm as well, which tends to work out fine as long as the
> entire kernel is built with the correct -march= option for the minimum
> supported CPU, and stays away from inline asm that requires a higher
> CPU level.

There's really no advantage to them, and they're ugly and annoying
and if we applied the concept consistently for all asm they would grow 
to a very large number.

The idea they'll give you good static checking just doesn't really
pan out.

Thanks,
Nick

Powered by blists - more mailing lists