qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Igor Mammedov
Subject: Re: [Qemu-devel] [patches] Re: [PULL] RISC-V QEMU Port Submission
Date: Wed, 28 Feb 2018 11:41:11 +0100

On Wed, 28 Feb 2018 13:41:25 +1300
Michael Clark <address@hidden> wrote:

> On Wed, Feb 28, 2018 at 5:00 AM, Igor Mammedov <address@hidden> wrote:
> 
> > On Tue, 27 Feb 2018 14:01:05 +0000
> > Peter Maydell <address@hidden> wrote:
> >  
> > > On 27 February 2018 at 00:15, Michael Clark <address@hidden> wrote:  
> > > > -----BEGIN PGP SIGNED MESSAGE-----
> > > > Hash: SHA1
> > > >
> > > > The following changes since commit 0a773d55ac76c5aa89ed9187a3bc5a  
> > f8c5c2a6d0:  
> > > >
> > > >   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 170a9d412ca1eb3b7ae6f6c1ff86dc  
> > bdff0fd7a8:  
> > > >
> > > >   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?  
> > also '[PATCH v7 03/23] RISC-V CPU Core Definition' still hasn't addressed
> > comment http://lists.gnu.org/archive/html/qemu-devel/2018-02/msg06412.html
> > which isn't fixed since it was first pointed out (v4).
> >
> > I'd prefer that being fixed before merge so another people
> > won't have to clean it up later after original authors,
> > When they try to generalize cpu_type -> cpu_model conversion.
> >  
> 
> I re-read the email and it doesn't seem clear what you want us to do.
> I changed the CPU suffix to a prefix as you requested. The rest of the CPU
> initialisation is "modelled" on arm not sh4.
Not addressed comment is not related to CPU suffix, it's a separate issue.

I haven't had time to clean up that part of ARM cpu initialization yet,
so it still uses old approach, but you've been pointed to right
approach rather early (v4) but ignored it.

There is no point to use legacy approach with new patches
that could block other people and would force them to cleanup
before doing what they intended to do.

I've pointed to sh4 as example that you should use for new CPU
types internalization. Beside making RISC-V consistent with
direction that part of code moves to, it will also simplify patch.

Please ask if something still unclear.

> If you want to make a pull request, please use this branch:
> 
> - https://github.com/riscv/riscv-qemu/tree/qemu-upstream-v7




reply via email to

[Prev in Thread] Current Thread [Next in Thread]