tools: Add a toplevel Makefile

Hi Borislav.

Some random comments as I really did not look at this part of the patch-set before.

tools/scripts/Makefile.include line 15

Quoted 5 lines

tools/scripts/Makefile.include new file +5 -0

+#
+# Include saner warnings here, which can catch bugs:
+#
+EXTRA_WARNINGS := -Wformat
+EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wformat-security

Two general comments - and I know this is just something you copied...

Why not use the += operator. The below looks like shell script syntax. And we use the += operator in other places - at least in the kernel stuff.

AND WHY ALL THESE SCREAMING CAPITAL LETTERS?
I know Makefiles and scripts are full of them - but that does not help the readability.

For kbuild I generally shifted to use:
- lower-case names for local stuff
- Upper case letters for global stuff - properly prefixed to avoid collisions EXTRA_WARNINGS likely fall into the category global-stuff,
but then a local variable could still be usefull.

From: Borislav Petkov <[email protected]>

Add a Makefile with all the targets under tools/.

Signed-off-by Borislav Petkov <[email protected]>

---
 tools/Makefile |   31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 tools/Makefile

tools/Makefile new file +21 -0

new file mode 100644
index 000000000000..e93b8ec3d250
@@ -0,0 +1,31 @@
+include scripts/Makefile.include
+
+perf firewire lguest usb virtio vm: FORCE
+	$(QUIET_SUBDIR0)$@/ $(QUIET_SUBDIR1)
+
+cpupower: FORCE
+	$(QUIET_SUBDIR0)power/$@/ $(QUIET_SUBDIR1)
+
+turbostat x86_energy_perf_policy: FORCE
+	$(QUIET_SUBDIR0)power/x86/$@/ $(QUIET_SUBDIR1)
+
+selftests: FORCE
+	$(QUIET_SUBDIR0)testing/$@/ $(QUIET_SUBDIR1)
+
+firewire_clean lguest_clean perf_clean vm_clean usb_clean virtio_clean:
+	$(QUIET_SUBDIR0)$(@:_clean=)/ $(QUIET_SUBDIR1) clean
+
+cp_clean:
+	$(QUIET_SUBDIR0)power/cpupower/ $(QUIET_SUBDIR1) clean
+
+turbostat_clean x86_energy_perf_policy_clean:

Things would be much simpler is all tools were adjusted to:
a) the subdir they live in is the make target name
b) each tool has three mandatory targets:
1) default target => build the tool
2) clean
3) install

I can see that with the layout today you cannot distingush between the various targets below power. But the current solutions is not scaleable nor maintainable.

Even if you cannot fix power/* then testing looks easy.

But all that can always to improved and consolidated later.

From: Borislav Petkov <[email protected]>

... and make it the default one so that calling 'make' without arguments in the tools/ directory gives you the possible targets to build along with a short description of what they are.

Signed-off-by Borislav Petkov <[email protected]>

---
 tools/Makefile |   26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

tools/Makefile +20 -0

index e93b8ec3d250..c7ed814e12cf 100644
@@ -1,5 +1,25 @@
 include scripts/Makefile.include
 
+help:
+	@echo 'Possible targets:'
+	@echo ''
+	@echo '  cpupower   - a tool for all things x86 CPU power'
+	@echo '  firewire   - the userspace part of nosy, an IEEE-1394 traffic sniffer'
+	@echo '  lguest     - a minimal 32-bit x86 hypervisor'
+	@echo '  perf       - Linux performance measurement and analysis tool'
+	@echo '  selftests  - various kernel selftests'
+	@echo '  turbostat  - Intel CPU idle stats and freq reporting tool'
+	@echo '  usb        - USB testing tools'
+	@echo '  virtio     - vhost test module'
+	@echo '  vm         - misc vm tools'
+	@echo '  x86_energy_perf_policy - Intel energy policy tool'
+	@echo ''
+	@echo 'Cleaning targets:'
+	@echo ''
+	@echo '  all of the above with the "_clean" string appended cleans'
+	@echo '    the respective build directory.'
+	@echo '  clean: a summary clean target to clean _all_ folders'
+
 perf firewire lguest usb virtio vm: FORCE
 	$(QUIET_SUBDIR0)$@/ $(QUIET_SUBDIR1)
 

From: Borislav Petkov <[email protected]>

Now you can do

$ make tools/<toolname>

from the toplevel kernel directory and have the respective tool built.

If you want to build and install it, do

$ make tools/<toolname>_install

$ make tools/<toolname>_clean

should clean the respective tool directories.

If you want to clean all in tools, simply do

$ make tools/clean

Also, if you want to get what the possible targets are, simply calling

$ make tools/

should give you the short help.

$ make tools/install

installs all tools, of course. Doh.

Cc Sam Ravnborg <[email protected]>

Signed-off-by Borislav Petkov <[email protected]>

---
 Makefile       |    7 +++++++
 tools/Makefile |   25 ++++++++++++++++++++++++-
 2 files changed, 31 insertions(+), 1 deletion(-)

Makefile +0 -0

index 0df3d003a079..e24ce7104f63 100644

Makefile +5 -1

+
+tools/%: FORCE
+	$(Q)$(MAKE) LDFLAGS= MAKEFLAGS= -C $(src)/tools/ $*
+
 
 
-perf firewire lguest usb virtio vm: FORCE
+firewire lguest perf usb virtio vm: FORCE

$(QUIET_SUBDIR0)$@/ $(QUIET_SUBDIR1)

cpupower: FORCE

Makefile +3 -0

@@ -32,6 +42,19 @@ turbostat x86_energy_perf_policy: FORCE
 selftests: FORCE
 	$(QUIET_SUBDIR0)testing/$@/ $(QUIET_SUBDIR1)
 
+firewire_install lguest_install perf_install usb_install virtio_install vm_install:
+	$(QUIET_SUBDIR0)$(@:_install=)/ $(QUIET_SUBDIR1) install
+

Quoted 42 lines · Borislav Petkov

From: Borislav Petkov <[email protected]>

Now you can do

$ make tools/<toolname>

from the toplevel kernel directory and have the respective tool built.

If you want to build and install it, do

$ make tools/<toolname>_install

$ make tools/<toolname>_clean

should clean the respective tool directories.

If you want to clean all in tools, simply do

$ make tools/clean

Also, if you want to get what the possible targets are, simply calling

$ make tools/

should give you the short help.

$ make tools/install

installs all tools, of course. Doh.

Cc Sam Ravnborg <[email protected]>

Signed-off-by Borislav Petkov <[email protected]>

---
 Makefile       |    7 +++++++
 tools/Makefile |   25 ++++++++++++++++++++++++-
 2 files changed, 31 insertions(+), 1 deletion(-)

Makefile +0 -0

index 0df3d003a079..e24ce7104f63 100644

Makefile kernelrelease: · line 1473

Quoted 6 lines

Makefile +3 -0

@@ -1468,6 +1468,13 @@ kernelrelease:
 kernelversion:
 	@echo $(KERNELVERSION)
 
+# Clear a bunch of variables before executing the submake
+tools/: FORCE
+	$(Q)$(MAKE) LDFLAGS= -C $(src)/tools/

I think you need to clear MAKEFLAGS here too?

Quoted 9 lines

Makefile +5 -1

+
+tools/%: FORCE
+	$(Q)$(MAKE) LDFLAGS= MAKEFLAGS= -C $(src)/tools/ $*
+
 
 
-perf firewire lguest usb virtio vm: FORCE
+firewire lguest perf usb virtio vm: FORCE

$(QUIET_SUBDIR0)$@/ $(QUIET_SUBDIR1)

Fold this into 2/4

Quoted 9 lines

cpupower: FORCE

Makefile +3 -0

@@ -32,6 +42,19 @@ turbostat x86_energy_perf_policy: FORCE
 selftests: FORCE
 	$(QUIET_SUBDIR0)testing/$@/ $(QUIET_SUBDIR1)
 
+firewire_install lguest_install perf_install usb_install virtio_install vm_install:
+	$(QUIET_SUBDIR0)$(@:_install=)/ $(QUIET_SUBDIR1) install
+

Makefile turbostat x86_energy_perf_policy: FORCE · line 57

Quoted 10 lines

Makefile +10 -0

+cpupower_install:
+	$(QUIET_SUBDIR0)power/$(@:_install=)/ $(QUIET_SUBDIR1) install
+
+turbostat_install x86_energy_perf_policy_install:
+	$(QUIET_SUBDIR0)power/x86/$(@:_install=)/ $(QUIET_SUBDIR1) install
+
+install: firewire_install lguest_install perf_install usb_install \
+		virtio_install vm_install cpupower_install turbostat_install \
+		x86_energy_perf_policy_install
+

This really belongs in 2/4 or a separate patch.

Sorry for bing picky about these details - but they destroy an otherwise fine patch-set.