linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Mark Brown <broonie@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] arm64: Generate cpucaps.h
Date: Thu, 13 May 2021 12:51:39 +0100	[thread overview]
Message-ID: <20210513115139.GC10886@C02TD0UTHF1T.local> (raw)
In-Reply-To: <9abba2a3-ce01-1cbe-5db3-97575219efad@arm.com>

On Thu, May 13, 2021 at 10:35:04AM +0530, Anshuman Khandual wrote:
> 
> On 4/28/21 5:42 PM, Mark Brown wrote:
> > The arm64 code allocates an internal constant to every CPU feature it can
> > detect, distinct from the public hwcap numbers we use to expose some
> > features to userspace. Currently this is maintained manually which is an
> > irritating source of conflicts when working on new features, to avoid this
> > replace the header with a simple text file listing the names we've assigned
> > and sort it to minimise conflicts.
> > 
> > As part of doing this we also do the Kbuild hookup required to hook up
> > an arch tools directory and to generate header files in there.
> > 
> > This will result in a renumbering and reordering of the existing constants,
> > since they are all internal only the values should not be important. The
> > reordering will impact the order in which some steps in enumeration handle
> > features but the algorithm is not intended to depend on this and I haven't
> > seen any issues when testing. Due to the UAO cpucap having been removed in
> > the past we end up with ARM64_NCAPS being 1 smaller than it was before.
> > 
> > Signed-off-by: Mark Brown <broonie@kernel.org>
> > ---
> >  arch/arm64/Makefile              |  3 ++
> >  arch/arm64/include/asm/Kbuild    |  2 +
> >  arch/arm64/include/asm/cpucaps.h | 74 --------------------------------
> >  arch/arm64/tools/Makefile        | 22 ++++++++++
> >  arch/arm64/tools/cpucaps         | 65 ++++++++++++++++++++++++++++
> >  arch/arm64/tools/gen-cpucaps.awk | 40 +++++++++++++++++
> >  6 files changed, 132 insertions(+), 74 deletions(-)
> >  delete mode 100644 arch/arm64/include/asm/cpucaps.h
> >  create mode 100644 arch/arm64/tools/Makefile
> >  create mode 100644 arch/arm64/tools/cpucaps
> >  create mode 100755 arch/arm64/tools/gen-cpucaps.awk
> > 
> > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> > index 7ef44478560d..b52481f0605d 100644
> > --- a/arch/arm64/Makefile
> > +++ b/arch/arm64/Makefile
> > @@ -175,6 +175,9 @@ vdso_install:
> >  	$(if $(CONFIG_COMPAT_VDSO), \
> >  		$(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso32 $@)
> >  
> > +archprepare:
> > +	$(Q)$(MAKE) $(build)=arch/arm64/tools kapi
> 
> Small nit. So going forward this new directory (arch/arm64/tools/), will
> be used to hold similar scripts that generate header or similar things ?
> At first the directory name 'tool' was bit confusing but seems like other
> archs (arm, m68k, mips, powerpc, s390, sh) have this directory as well.

Yup; there's more stuff we might want to put here in future, e.g.

* Generation of (or consistency-checks) for the kernel HWCAP glue
* Generation of the sysreg headers
* Generation of the insn code
* Generation of metadata (or consistency checks) for
  RELIABLE_STACKTRACE, LIVEPATCH, etc

[...]

> > +BEGIN {
> > +	print "#ifndef __ASM_CPUCAPS_H"
> > +	print "#define __ASM_CPUCAPS_H"
> > +	print ""
> > +	print "/* Generated file - do not edit */"
> > +	cap_num = 0
> > +	print ""
> > +}
> > +
> > +/^[vA-Z0-9_]+$/ {
> 
> Small nit. Should this be length restricted at the least ?
> Like each CAP entries should not exceed a certain length.

I don't think we should enforce a length limit in the script, as we'd
have to select an arbitrary limit, and given we should catch exceedingly
long names during review, I think it's more likely to be a hindrance
than a help (e.g. if we ever bump up against that limit).

> > +	printf("#define ARM64_%-30s\t%d\n", $0, cap_num++)
> > +	next
> > +}
> > +
> > +END {
> > +	printf("#define ARM64_NCAPS\t\t\t\t%d\n", cap_num)
> > +	print ""
> > +	print "#endif"
> 
> Small nit. Should it be print "#endif /* __ASM_CPUCAPS_H */" instead
> in order to be complete.

Agreed; that would be nice.

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-05-13 11:53 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-28 12:12 [PATCH] arm64: Generate cpucaps.h Mark Brown
2021-04-29 12:32 ` Catalin Marinas
2021-05-04 11:43 ` Mark Rutland
2021-05-10 12:55 ` Catalin Marinas
2021-05-13  5:05 ` Anshuman Khandual
2021-05-13 11:51   ` Mark Rutland [this message]
2021-05-13 12:45   ` Mark Brown
2021-05-13 11:30 ` Geert Uytterhoeven
2021-05-13 13:03   ` Mark Brown
2021-09-21 18:08 ` dann frazier
2021-09-21 18:35   ` Mark Brown
2021-09-21 21:09     ` Suzuki K Poulose
2021-09-22 13:45       ` dann frazier

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=20210513115139.GC10886@C02TD0UTHF1T.local \
    --to=mark.rutland@arm.com \
    --cc=anshuman.khandual@arm.com \
    --cc=broonie@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=suzuki.poulose@arm.com \
    --cc=will@kernel.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).