Re: [PATCH v8 16/29] arm64: Share arm64 headers with s390
From: Will Deacon
Date: Wed Sep 30 2026 - 04:19:12 EST
On Wed, Sep 30, 2026 at 08:55:49AM +0100, Marc Zyngier wrote:
> On Wed, 30 Sep 2026 08:28:03 +0100,
> Steffen Eiden <seiden@xxxxxxxxxxxxx> wrote:
> >
> > On Tue, Sep 29, 2026 at 06:00:12PM +0100, Catalin Marinas wrote:
> > > On Tue, Sep 29, 2026 at 06:19:20AM +0200, Andreas Grapentin wrote:
> > > > On Sep 28 26, Steffen Eiden wrote:
> > > > > On Mon, Sep 28, 2026 at 05:07:52PM +0100, Catalin Marinas wrote:
> > > > > > On Fri, Sep 18, 2026 at 03:30:53PM +0200, Steffen Eiden wrote:
> > > > > > > +# Enable all code shared to s390
> > > > > > > +KBUILD_CFLAGS += -DARM64_S390_COMMON
> > > > > > > +KBUILD_AFLAGS += -DARM64_S390_COMMON
> > > > > > > +KBUILD_CPPFLAGS += -DARM64_S390_COMMON
> > > > > >
> > > > > > Do we actually need these defines? They seem only to be used as markers
> > > > > > for the awk scripts to extract the definitions. Why do we need the C
> > > > > > preprocessor involved at all? Could we not just have comment markers:
> > > > > >
> > > > > > /* ARM64_S390_COMMON_BEGIN */
> > > > > > ...
> > > > > > /* ARM64_S390_COMMON_END */
> > > > > >
> > > > > No technically we do not need those. They could be useful if we find out
> > > > > that AWK is the wrong tool and move to a C Preprocessor + diff based
> > > > > approach.
> > > > >
> > > > > if the ifdev is not closed the compiler will complain, but an 'arm did
> > > > > not destroy us' verifiaction tool ( I will send one soonish) could do
> > > > > the same.
> > > >
> > > > iirc the last time we discussed this we didn't think that load-bearing
> > > > comments were the right tool here. We also briefly floated the idea of
> > > > using a #pragma region based approach, but in the end we went with the
> > > > #ifdef preprocessor directives instead, as the least invasive
> > > > non-comment marker that was available.
> > >
> > > The downside is that the macro affects the preprocessed code. We need to
> > > ensure they don't leak in uapi headers for example. Not a fan of this
> > > approach but I don't have a better suggestion either. At least we could
> > > write them as:
> > >
> > > #if ARM64_S390_COMMON == 1
> >
> > This is a great idea and an improvement. Thanks.
> >
> > @Marc: Shall I sent a v9 of this series or do you want to pick v8 and I
> > send this as an improvement once picked?
>
> Please send a v9. That's invasive enough that it warrants it.
>
> > > It would also be useful for the awk scripts look for the paired #endif
> > > rather than relying on the comment at the end of the line. Otherwise
> > > it's not really different from just sticking to comments.
> > >
> >
> > Are you suggesting that the AWK scripts should track the #if% and #endif
> > in the code and warn or reject if the marked endif for the shared code
> > does not match pairing with the starting #if? That makes sense yes. I'll
> > investigate and would add this as an improvement later on.
>
> I think the suggestion is to teach the script about the #if/#endif
> nesting, and therefore to not require the trailing comment on #endif
> (i.e. the script acts as a normal cpp).
>
> It should only be a matter of counting the #{if,ifdef}s and #endif
> past the ARM64_S390_COMMON guard.
Yeah, and the check on the comment anchor could still remain as a rough
consistency check.
Will