Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

qemu_v8: recent QEMU requires libfdt 1.4.2 or higher #156

Closed
wants to merge 4 commits into from
Closed

qemu_v8: recent QEMU requires libfdt 1.4.2 or higher #156

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
382e923
qemu_v8: recent QEMU requires libfdt 1.4.2 or higher
etienne-lms Jul 3, 2017
e569292
[review] qemu_v8: recent QEMU requires libfdt 1.4.2 or higher
etienne-lms Jul 3, 2017
5108bed
[review] qemu_v8: recent QEMU requires libfdt 1.4.2 or higher
etienne-lms Jul 3, 2017
fd4bbfd
[review] qemu_v8: recent QEMU requires libfdt 1.4.2 or higher
etienne-lms Jul 3, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ script:
# in the makefile.
- if [ $REPO_PROJ == "fvp" ]; then mkdir -p $HOME/$REPO_PROJ/Foundation_Platformpkg; fi
- cd $HOME/$REPO_PROJ && repo init -u https://github.com/OP-TEE/manifest.git -m $REPO_PROJ.xml </dev/null && repo sync -j2 --no-clone-bundle --no-tags --quiet
# Fetch a local copy of dtc+libfdt to avoid issues with a possibly outdated libfdt-dev
- if [ $REPO_PROJ == "qemu_v8" ]; then cd $HOME/$REPO_PROJ/qemu && git submodule update --init dtc; fi
# Here we are using Travis environment variables to select the correct branch
# based on either a pull request or a normal push.
- |
Expand Down
4 changes: 3 additions & 1 deletion qemu_v8.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 && \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather avoid changing the source tree in the build step, if possible. Could this be done before, such as in manifest.xml?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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" />

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@etienne-lms etienne-lms Jul 3, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@etienne-lms

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" />

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jforissier

        <!-- 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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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'

./configure --target-list=aarch64-softmmu \
$(QEMU_CONFIGURE_PARAMS_COMMON)
$(MAKE) -C $(QEMU_PATH)

Expand Down