qemu_v8: recent QEMU requires libfdt 1.4.2 or higher #156
Conversation
Ubuntu 1.4.04 LTS, 16.04 LTS and even 17.04 provide the libfdt in version 1.4.0. This version is too old for QEMU that requires libfdt in version 1.4.2 or higher. This change insures DTC package is locally clone so that QEMU builds over it rather than using the libfdt support and tools from the system. Suggested-by: Joakim Bech <joakim.bech@linaro.org> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
qemu_v8.mk
Outdated
| @@ -65,7 +65,11 @@ arm-tf-clean: | |||
| # QEMU | |||
| ################################################################################ | |||
| qemu: | |||
| cd $(QEMU_PATH); ./configure --target-list=aarch64-softmmu\ | |||
| cd $(QEMU_PATH); \ | |||
| dpkg --compare-versions 1.4.2 le \ | |||
There was a problem hiding this comment.
Do we even need to compare etc? It seems like QEMU defaults to the locally checked out version if there is any, otherwise it will use the hosts version. So, I guess line 71 only is sufficient.
There was a problem hiding this comment.
Ok, always rely on the cloned dtc package.
|
Also, with this change (working), you could also remove the lines doing the same in .travis.yml. |
|
By the way, I wonder if the same change should be required on qemu_virt (default.mk/default_stable.mk). These currently rely on old qemu and we should upgrade it some day. |
Always clone the latest dtc. Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
Update .travis.yml accordingly.
Change commit title to "qemu_v8: recent QEMU requires recent libfdt". Update also the qemu (QEMU/ARMv7 setup). Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
Yeah, I think that is better safe than sorry. Anyhow, for the current patches. |
| @@ -65,7 +65,9 @@ arm-tf-clean: | |||
| # QEMU | |||
| ################################################################################ | |||
| qemu: | |||
| cd $(QEMU_PATH); ./configure --target-list=aarch64-softmmu\ | |||
| cd $(QEMU_PATH); \ | |||
| git submodule update --init dtc && \ | |||
There was a problem hiding this comment.
I'd rather avoid changing the source tree in the build step, if possible. Could this be done before, such as in manifest.xml?
There was a problem hiding this comment.
I'm not sure if that can be done by repo. I some other cases in the past, I've added a check for the existence of a file created by some command to avoid doing the same operation over and over again. Having that said, by doing so it would still be a step/check done in the build step.
There was a problem hiding this comment.
@jbech-linaro
I'll probably agree with Jerome,
It could be done by adding default attribute to the manifest (based on info from manifest-format.txt)
<default sync-s="true"/>
And to sync git reps + all submodules:
repo sync --fetch-submodules
There was a problem hiding this comment.
Tested: adding sync-s="true" to the qemu project only seems to do the job through command repo sync. I.e, in qemu_v8.xml:
- <project remote="qemu" path="qemu" name="qemu.git" />
+ <project remote="qemu" path="qemu" name="qemu.git" sync-s="true" />There was a problem hiding this comment.
Doesn't seem that optimal to have it there. Another option would be to write a simple "init" script, but I'm not super keen on doing that. Just running the make is quite nice when you have got the code. What about an "init" target to the make file? The "qemu" target would depend on that for example?
There was a problem hiding this comment.
This also works, it's a trade-off. I think your proposal is better than having sync-s="true", since with your proposal, it's only a symlink that needs to be created if you clean a lot.
Odd, that you are in favor of using repo and I'm not :)
Related, I think there is something fishy with the QEMU-v8 setup in general. Meanwhile doing other stuff today I've built QEMU-v8 in the background on different computers with different manifests (latest, old_stable, 2.5.0-rc1 etc, with and without Liangs ARM-TF rebase) and it behaves very weird. It hangs almost immediately on both computers. Having that said, I have had some test runs working. So I'm staring to wonder if there is an issue showing up from time to time. On my side it seems to fail more often. Probably time for gdb ... or printfs.
There was a problem hiding this comment.
A have the same issues as @jbech-linaro: with this DTC issue resoved, optee/qemu_v8 tests are failing. Hangs during optee inits or hangs when exercising xtest.
edited for info: using the qemu version from tag optee-2.4.0 (qemu commit 5fe2339e6b09...), i can run optee and its tests for the qemu_armv8 setup.
There was a problem hiding this comment.
qemu already creates the sub-directory ./dtc. The linkfile directive above may fail.
You are right. Everything seems to work fine with this simple change instead:
<!-- QEMU -->
<project remote="qemu" path="qemu" name="qemu.git" />
+ <project remote="qemu" path="qemu/dtc" name="dtc.git" />There was a problem hiding this comment.
<!-- QEMU -->
- <project remote="qemu" path="qemu" name="qemu.git" />
+ <project remote="qemu" path="dtc" name="dtc.git" />
+ <project remote="qemu" path="qemu" name="qemu.git">
+ <linkfile src="../dtc" dest="qemu/dtc" />
+ </project>This will probably brake qemu git local rep, at least if someone decide to update dtc submodule manually within qemu rep. After syncing this manifest file, have you tried to do this? I'm curious what will happen
git submodule update --init dtc
There was a problem hiding this comment.
@igoropaniuk yeah ignore my first proposal, it's broken because repo sync errors out due to qemu/dtc exists already.
FYI, I tried what you are asking with my second proposal (#156 (comment)) and it's alright:
$ cd eqmu
$ git submodule update --init dtc
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Submodule path 'dtc': checked out '558cd81bdd432769b59bff01240c44f82cfb1a9d'
Troll! :D |
I think I've managed to git bisect to the QEMU commit where it starts failing. I've been booting up QEMU and starting xtest between 5-10 times before marking it either as good or bad and I ended up with this: $ git bisect bad
e75449a346bf558296966a44277bfd93412c6da6 is the first bad commit
commit e75449a346bf558296966a44277bfd93412c6da6
Author: Emilio G. Cota <cota@braap.org>
Date: Fri Apr 28 14:59:23 2017 -0400
target/aarch64: optimize indirect branches
Measurements:
[Baseline performance is that before applying this and the previous commit]
- NBench, aarch64-softmmu. Host: Intel i7-4790K @ 4.00GHz
...
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index ab61d96099..860e279658 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -11367,8 +11367,7 @@ void gen_intermediate_code_a64(ARMCPU *cpu, TranslationBlock *tb)
gen_a64_set_pc_im(dc->pc);
/* fall through */
case DISAS_JUMP:
- /* indicate that the hash table must be used to find the next TB */
- tcg_gen_exit_tb(0);
+ tcg_gen_lookup_and_goto_ptr(cpu_pc);
break;
case DISAS_TB_JUMP:
case DISAS_EXC: The patch contains just a oneliner, but without out understanding of QEMU it is hard to say anything about it. I've also played with running latest and just revert that particular patch and since I did that I simply cannot repeat issue (just hangs otherwise). Maybe @pm215 would like to have a look or know what it could be about? To reproduce, simply checkout a QEMU v8 OP-TEE environment (and the DTC fix that this PR is about is also needed) and run xtest. @jforissier , If this indeed is the bad commit I found, then I think we should use the commit prior to that in our manifest when tagging, i.e, this one: Finally it would be good if someone else could test it, to see if you agree with me here. It was a bit tricky to rebase since it was working a few times even with the bad patch. Not often, but it happened. |
|
While bisecting and playing with the qemu history, I went to another faulty commit:
I think there are 2 unrelated issues. One that prevent input console from being functional, and one that makes the xtest hanging at some place. The faulty commit found by @jbech-linaro (e75449a346bf5 "target/aarch64: optimize indirect branches", committed 5th of June) is not far from the one I found, but on another branch (it is hard to follow the history through all these merges). And.... reverting both the commit found by Joakim (e75449a) and the one pointed above (7e6478e7d4f) from the latest merged head (82d76dc7fc19a) make the optee xtest functional again. Don't ask me why. Yet, here is the most recent merge commit I found (from the 7th of June) over which I successfully ran the optee env/test:
|
|
I'm told that commit e75449a346bf55 was indeed an upstream regression, but it should also now be fixed upstream, by commit 8da54b2507c. I'm not sure why commit 7e6478e7d4 (the fcntl one) would cause problems unless you're actually triggering the new asserts that it adds... |
From my end user view, the effect seems that the console prints the log but fails the get input characters. And that reverting it restores input console. I cannot tell more. |
|
Replaced by OP-TEE/manifest#57. |
|
On 4 July 2017 at 13:05, Jérôme Forissier ***@***.***> wrote:
Closed #156.
Does that mean you've resolved the issues with regressions with
recent QEMU, or is that still an outstanding thing we need to
look into on our side?
thanks
-- PMM
|
No, I'm closing this because QEMU regressions is not the topic of this PR. @jbech-linaro or @etienne-lms would you please open a separate issue? Thanks. |
@pm215 , I guess we still need to do some additional testing. The issue itself here was about adding a newer libfdt version to our QEMU setup and when we did that we faced this other issue when running latest from upstream. So one issue became two. I'll have a look at the commits you mentioned and see if I can come to any conclusion. |
Yes, will do that if needed. |
Ubuntu 1.4.04 LTS, 16.04 LTS and even 17.04 provide the libfdt in
version 1.4.0. This version is too old for QEMU that requires libfdt
in version 1.4.2 or higher.
This change insures DTC package is locally clone so that QEMU builds
over it rather than using the libfdt support and tools from the system.
Suggested-by: Joakim Bech joakim.bech@linaro.org
Signed-off-by: Etienne Carriere etienne.carriere@linaro.org