On Wed, Jun 30, 2021 at 05:19:57PM +0300, Nikos Nikoleris wrote: > On 30/06/2021 15:23, Andrew Jones wrote: ... > > It also looks derived from kvm-unit-tests:arm/Makefile.common. So why not > > just modify that instead? Possibly creating a new make target in order to > > accommodate any differences. > > > > I was trying to completely get rid of > herdtools:litmus/libdir/_aarch64/kvm.rules and the cfg files because they > are replicating much of kvm-unit-tests:arm/Makefile.common and > kvm-unit-tests:Makefile. If I understand correctly in your proposal we will > generate them automatically? Either generate or add lines to kvm-unit-tests Makefiles which can be used when building a litmus7 specific build target, e.g. 'make litmus7' > > > > > > > > 2) Add kvm-unit-tests as a git submodule to [1] to get access to the > > > > generated .cfg files and to build the litmus tests for kvm-unit-tests. > > > > A litmus7 command will invoke the kvm-unit-tests build (using > > > > make -C). > > > > > > That's possible but it doesn't solve the biggest problem which is figuring > > > out what is the command(s) we need to run to link an elf and subsequently > > > generate a flat file. > > > > Shouldn't all those commands be in the Makefiles that will be used as part > > of the 'make -C <kut-submodule-dir>' call? > > > > Without making any changes to the existing kvm-unit-tests build system 'make > -C <kut-submodule-dir>' will build all objects and flat files for the > existing tests. Or am I missing something? You can modify kvm-unit-tests Makefiles if necessary. And/or add a new one, e.g. arm/Makefile.litmus7 > > > > > > > > 3) Create an additional unittests.cfg file (e.g. litmus7-tests.cfg) for > > > > kvm-unit-tests that allows easily running all the litmus7 tests. > > > > (That should also allow 'make standalone' to work for litmus7 tests.) > > > > > > This is a good point I can have a look at how we could add this. > > > > > > > 4) Like patch 3/3 already does, document the litmus7 stuff in > > > > kvm-unit-tests, so people understand the purpose of the --litmus7 > > > > configure switch and also to inform them of the ability to run > > > > additional tests and how (by using [1]). > > > > > > > > > > Overall it would be great if we could piggyback on the build system of > > > kvm-unit-tests rather than try to re-generate (part of) it. This is what the > > > patch 2/3 tries to do. This is not solving the problem in a way that is > > > specific to litmus7 and allows for adding more source files to the all-tests > > > list. > > > > Patch 2/3 only adds the ability to add a new dir to look at. It leaves > > everything else up to litmus7 build code to duplicate what it needs and > > also manual commands to populate that new directory. I'd rather we have a > > more coherent solution. > > > > We want to build litmus7 tests as kvm-unit-tests. We can look at this two > > ways: 1) we want to add litmus7 tests to kvm-unit-tests or 2) we want to > > build litmus7 tests for kvm-unit-tests. > > > This patch series is going for (1), but without actually committing the > > tests to kvm-unit-tests. I'm arguing we should do (2), especially since > > the litmus7 build code already appears to need to know about > > kvm-unit-tests. > > > > I was more looking to add support for external tests similar to the idea of > having external modules to the linux kernel. The implementation might not be > ideal; I didn't want the changes to be very intrusive but if that's the > problem then I am happy to make changes. I don't expect the changes to be too intrusive and they can probably be mostly isolated into their own Makefile.litmus7 file if necessary. > > As for assumptions, the external dir needs to have at least a .c file for > the code of the test, a minimal Makefile specifying tests and object files > and as you pointed out a unittests.cfg. > > If we ignore litmus7 for a bit, the general question is, I have a test > (e.g., kvm-unit-test:arm/selftest.c) which is not part of the tree and not > in the build system, how can I build the corresponding flat or efi file > based on it? > > I think your proposal, is more in line with what we do now. At least, we > would automatically generate the config files. kvm-unit-tests generates > config files, which litmus7 would use as input to generate a Makefile. I > wanted to move away from that but we can live with it. Also note that if we > added a --litmus7 configure switch, it would be very specific to way we do > things in litmus7. Let's see what it looks like. I'm having trouble imagining how this repo combination will work, but I'm optimistic that it's doable without too many changes to either repo. And most the specific to litmus7 stuff should still be in the litmus7 repo, even if we need kvm-unit-tests to be aware of some build differences and therefore provide the configure switch. > > > I think we should only need to modify the build scripts of litmus7 to > > use/build kvm-unit-tests as a submodule and to somehow build litmus7 > > tests with it. Also, litmus7 test running could be done from the > > litmus7 build repo or kvm-unit-tests standalone tests could built for > > litmus7 tests and installed elsewhere. > > > > A final note on patch 2/3. Why not just override the TEST_DIR config > > variable with a different directory? (If it doesn't work for some > > reason, then we could hopefully fix that.) > > > > External (litmus7) tests target an arch (could be arm64, could be x86), and > there are many variables inside $(TEST_DIR)/ that we would need. If I am > building for arm64, I need arm/flat.lds and information that currently > resides in arm/Makefile.arm64: > > bits = 64 > ldarch = elf64-littleaarch64 > > arch_LDFLAGS = -pie -n > CFLAGS += -mstrict-align > > and if I change TEST_DIR to point to ~/litmus7_tests I will need to > replicate all that somewhere else. > > I am not saying that it would be impossible to override TEST_DIR but it > doesn't make things simple. It'd probably be safe to change the build scripts to use something like $ARCH/Makefile.$ARCH and $ARCH/flat.lds, but we'll need a arm64 -> arm link. Overriding TEST_DIR may not be easier, but I think it's worth experimenting with it, since I feel like it would be cleaner than the "add an arbitrary directory" configure switch. Thanks, drew
