Re: [Qemu-devel] [PULL] RISC-V QEMU Port Submission

[Peter Maydell]

On 27 February 2018 at 00:15, Michael Clark <address@hidden> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> The following changes since commit 0a773d55ac76c5aa89ed9187a3bc5af8c5c2a6d0:
>
>   maintainers: Add myself as a OpenBSD maintainer (2018-02-23 12:05:07 +0000)
>
> are available in the git repository at:
>
>   https://github.com/riscv/riscv-qemu.git tags/riscv-qemu-upstream-v7
>
> for you to fetch changes up to 170a9d412ca1eb3b7ae6f6c1ff86dcbdff0fd7a8:
>
>   RISC-V Build Infrastructure (2018-02-27 11:09:43 +1300)
>
> - ----------------------------------------------------------------
> QEMU RISC-V Emulation Support (RV64GC, RV32GC)

Hi; thanks for this pull request. Unfortunately it seems to
be missing Signed-off-by: tags. Every commit needs to have
the Signed-off-by: tags from the people who contributed code to
it, indicating that they're OK with the code going into QEMU.
(If the work was done by and copyright a company then you don't
need to provide signoffs from every person at the company who
worked on the code if you don't want to.)

> The spike_v1.9
> machine has been renamed to spike_v1.9.1 to match the privileged ISA
> version and spike_v1.10 has been made the default machine.

I'm confused about this. Generally QEMU boards should model
hardware, and the board shouldn't care about the ISA versions.
Versioned board names in QEMU generally follow _QEMU_'s versioning,
and indicate that a board is identical to whatever we modelled
in that earlier QEMU version, for VM migration compatibility.
Board renames for minor ISA version bumps sounds like there's going
to be a lot of churn and breakage -- is this stuff really ready?
(Also, should we really have two different board source files
for two different ISA versions? I would have expected these to
share a source file to share code.)

I did a test build and there are some compile errors:

/home/pm215/qemu/linux-user/main.c:38:24: fatal error: target_elf.h:
No such file or directory
 #include "target_elf.h"
                        ^
compilation terminated.

This is because your patchset has a clash with commit 542ca4349878a2e
which has just merged to master, and refactors out an ifdef ladder,
so now all targets supporting linux-user need to provide a
linux-user/$ARCH/target_elf.h file. Could you fix that up and rebase,
please?

thanks
-- PMM