Re: [PATCH 01/14] arc: Add initial core cpu files

[Cupertino Miranda]

Hi Philippe,

Thank you for your time reviewing our patches.
My apologies for reacting to it so late. :-(

Once we decided to make this port we noticed that Michael Rolnik had
submitt a port for ARC700 to QEMU mailinglist.
As we tested it, we decided to use his directory structure, and for
that reason the most generic files as well, although significantly
changing everything else.

As a way to credit him for the initial work, we left his copyright
header in that file. Maybe that should instead mention him in the
commits, or in the cover letter instead. Please let me know of the
proper way.

Regarding "unsigned", some of these variables are used as "auxiliary
registers" and should at least be 32bit. Some others might perfectly
well be resized to "unsigned".We will certainly revisit these
definitions to make sure we use the proper types for the case.

Regards,
Cupertino



On Wed, Oct 7, 2020 at 5:09 AM Philippe Mathieu-Daudé  wrote:
>
> Hi Cupertino,
>
> On 9/30/20 10:45 PM, cupertinomira...@gmail.com wrote:
> > From: Cupertino Miranda 
> >
> > Signed-off-by: Cupertino Miranda 
> > ---
> ...
>
> > diff --git a/target/arc/Makefile.objs b/target/arc/Makefile.objs
> > new file mode 100644
> > index 00..7b2afd08e4
> > --- /dev/null
> > +++ b/target/arc/Makefile.objs
> > @@ -0,0 +1,34 @@
> > +#
> > +#  QEMU ARC CPU
> > +#
> > +#  Copyright (c) 2020
> > +#
> > +#  This library is free software; you can redistribute it and/or
> > +#  modify it under the terms of the GNU Lesser General Public
> > +#  License as published by the Free Software Foundation; either
> > +#  version 2.1 of the License, or (at your option) any later version.
> > +#
> > +#  This library is distributed in the hope that it will be useful,
> > +#  but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +#  Lesser General Public License for more details.
> > +#
> > +#  You should have received a copy of the GNU Lesser General Public
> > +#  License along with this library; if not, see
> > +#  
> > +#
> > +
> > +obj-y   += translate.o
> > +obj-y   += helper.o
> > +obj-y   += cpu.o
> > +obj-y   += op_helper.o
> > +obj-y   += gdbstub.o
> > +obj-y   += decoder.o
> > +obj-y   += regs.o
> > +obj-y   += semfunc.o
> > +obj-y   += semfunc-helper.o
> > +obj-y   += mmu.o
> > +obj-y   += mpu.o
> > +obj-y   += timer.o
> > +obj-y   += irq.o
> > +obj-y   += cache.o
>
> We don't use Makefiles anymore, and you already provides meson.build.
>
> > diff --git a/target/arc/arc-common.h b/target/arc/arc-common.h
> > new file mode 100644
> > index 00..8013e1d2ed
> > --- /dev/null
> > +++ b/target/arc/arc-common.h
> > @@ -0,0 +1,55 @@
> > +/*
> > + *  Common header file to be used by cpu and disassembler.
> > + *  Copyright (C) 2017 Free Software Foundation, Inc.
> > + *
> > + *  You should have received a copy of the GNU General Public License
> > + *  along with GAS or GDB; see the file COPYING3. If not, write to
> > + *  the Free Software Foundation, 51 Franklin Street - Fifth Floor, Boston,
> > + *  MA 02110-1301, USA.
> > + */
> > +
> > +#ifndef ARC_COMMON_H
> > +#define ARC_COMMON_H
> > +
> > +#include "qemu/osdep.h"
> ...
>
> Do not include "qemu/osdep.h" in headers.
>
> > +/*-*-indent-tabs-mode:nil;tab-width:4;indent-line-function:'insert-tab'-*-*/
> > +/* vim: set ts=4 sw=4 et: */
> > diff --git a/target/arc/cpu-qom.h b/target/arc/cpu-qom.h
> > new file mode 100644
> > index 00..413b693558
> > --- /dev/null
> > +++ b/target/arc/cpu-qom.h
> > @@ -0,0 +1,53 @@
> > +/*
> > + * QEMU ARC CPU
> > + *
> > + * Copyright (c) 2016 Michael Rolnik
>
> ???
>
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see
> > + * 
> > + */
> > +
> ...
>
> > diff --git a/target/arc/cpu.c b/target/arc/cpu.c
> > new file mode 100644
> > index 00..bbcb371760
> > --- /dev/null
> > +++ b/target/arc/cpu.c
> > @@ -0,0 +1,468 @@
> > +/*
> > + * QEMU ARC CPU
> > + *
> > + * Copyright (c) 2020
>
> (c) Synopsys?
>
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License,

Re: [PATCH 12/14] arc: Add Synopsys ARC emulation boards

[Cupertino Miranda]

Hi Philippe,

On Wed, Oct 7, 2020 at 5:31 AM Philippe Mathieu-Daudé  wrote:
>
> Hi Cupertino, Claudiu,
>
> On 9/30/20 10:46 PM, cupertinomira...@gmail.com wrote:
> > From: Claudiu Zissulescu 
> >
> > Add the Synopsys ARC boards, arc_sim for testing, sim-hs main emulation
> > board using standard UART and nsim which includes a Synopsys ARC specific
> > UART implementation.
> >
> > Signed-off-by: Claudiu Zissulescu 
> > ---
> >  hw/arc/Makefile.objs  |  21 +++
> >  hw/arc/arc_sim.c  | 143 
> >  hw/arc/arc_uart.c | 267 ++
> >  hw/arc/board-hsdk.c   | 107 +++
> >  hw/arc/boot.c |  95 ++
> >  hw/arc/boot.h |  21 +++
> >  hw/arc/meson.build|  13 ++
> >  hw/arc/nsim.c |  86 
> >  hw/arc/pic_cpu.c  | 111 
> >  hw/arc/sample.c   |  77 +++
> >  hw/arc/sim-hs.c   | 107 +++
> >  include/hw/arc/arc_uart.h |  43 ++
> >  include/hw/arc/cpudevs.h  |  10 ++
> >  13 files changed, 1101 insertions(+)
> >  create mode 100644 hw/arc/Makefile.objs
> >  create mode 100644 hw/arc/arc_sim.c
> >  create mode 100644 hw/arc/arc_uart.c
> >  create mode 100644 hw/arc/board-hsdk.c
> >  create mode 100644 hw/arc/boot.c
> >  create mode 100644 hw/arc/boot.h
> >  create mode 100644 hw/arc/meson.build
> >  create mode 100644 hw/arc/nsim.c
> >  create mode 100644 hw/arc/pic_cpu.c
> >  create mode 100644 hw/arc/sample.c
> >  create mode 100644 hw/arc/sim-hs.c
> >  create mode 100644 include/hw/arc/arc_uart.h
> >  create mode 100644 include/hw/arc/cpudevs.h
>
> Please split in various commits:
>
> - hw/char/arc_uart
> - hw/intc/synopsys_pic or something
> - hw/arc/boot
> - hw/arc/*sim*
> - hw/arc/*hsdk*
>
> (Also it would simplify differentiating the architectural
> part of your patches from the hardware ones if you use the
> target/arc/ prefix in your previous patches).

Got it, will make it happen in the next submission.

>
> >
> > diff --git a/hw/arc/Makefile.objs b/hw/arc/Makefile.objs
> > new file mode 100644
> > index 00..28d7766cd9
> > --- /dev/null
> > +++ b/hw/arc/Makefile.objs
> > @@ -0,0 +1,21 @@
> > +#
> > +#  QEMU ARC CPU
> > +#
> > +#  Copyright (c) 2019
> > +#
> > +#  This library is free software; you can redistribute it and/or
> > +#  modify it under the terms of the GNU Lesser General Public
> > +#  License as published by the Free Software Foundation; either
> > +#  version 2.1 of the License, or (at your option) any later version.
> > +#
> > +#  This library is distributed in the hope that it will be useful,
> > +#  but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +#  Lesser General Public License for more details.
> > +#
> > +#  You should have received a copy of the GNU Lesser General Public
> > +#  License along with this library; if not, see
> > +#  http://www.gnu.org/licenses/lgpl-2.1.html
> > +#
> > +
> > +obj-y   = arc_sim.o arc_uart.o sample.o pic_cpu.o boot.o board-hsdk.o 
> > sim-hs.o nsim.o
>
> We don't use Makefile anymore.
Oups, sorry for that, we had just rebased older code ... need to remove those.

>
> > diff --git a/hw/arc/arc_sim.c b/hw/arc/arc_sim.c
> > new file mode 100644
> > index 00..8020a03d85
> > --- /dev/null
> > +++ b/hw/arc/arc_sim.c
> > @@ -0,0 +1,143 @@
> > +/*
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see 
> > .
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qapi/error.h"
> > +#include "cpu.h"
> > +#include "hw/hw.h"
> > +#include "hw/boards.h"
> > +#include "elf.h"
> > +#include "hw/char/serial.h"
> > +#include "net/net.h"
> > +#include "hw/loader.h"
> > +#include "exec/memory.h"
> > +#include "exec/address-spaces.h"
> > +#include "sysemu/reset.h"
> > +#include "sysemu/runstate.h"
> > +#include "sysemu/sysemu.h"
> > +#include "hw/sysbus.h"
> > +#include "hw/arc/cpudevs.h"
> > +#include "boot.h"
> > +
> > +static void arc_sim_net_init(MemoryRegion *address_space,
> > + hwaddr base,
> > + hwaddr descriptors,
> > + qemu_irq irq, NICInfo *nd)
> > +{
> > +DeviceState *dev;
> > +SysBusDevice *s;
> > +
> >