Re: [PATCH v2 2/4] riscv: Add initial support for Shakti C machine

[Alistair Francis]

On Thu, Apr 1, 2021 at 1:18 PM Vijai Kumar K <vijai@behindbytes.com> wrote:
>
>
>
>
> ---- On Wed, 31 Mar 2021 21:05:47 +0530 Alistair Francis 
> <alistair23@gmail.com> wrote ----
>
>  > On Sun, Mar 21, 2021 at 1:09 AM Vijai Kumar K <vijai@behindbytes.com> 
> wrote:
>  > >
>  > > Add support for emulating Shakti reference platform based on C-class
>  > > running on arty-100T board.
>  > >
>  > > 
> https://gitlab.com/shaktiproject/cores/shakti-soc/-/blob/master/README.rst
>  > >
>  > > Signed-off-by: Vijai Kumar K <vijai@behindbytes.com>
>  > > ---
>  > >  MAINTAINERS                                 |   7 +
>  > >  default-configs/devices/riscv64-softmmu.mak |   1 +
>  > >  hw/riscv/Kconfig                            |  10 ++
>  > >  hw/riscv/meson.build                        |   1 +
>  > >  hw/riscv/shakti_c.c                         | 171 ++++++++++++++++++++
>  > >  include/hw/riscv/shakti_c.h                 |  74 +++++++++
>  > >  6 files changed, 264 insertions(+)
>  > >  create mode 100644 hw/riscv/shakti_c.c
>  > >  create mode 100644 include/hw/riscv/shakti_c.h
>  > >
>  > > diff --git a/MAINTAINERS b/MAINTAINERS
>  > > index 8e9f0d591e..9f71c4cc3f 100644
>  > > --- a/MAINTAINERS
>  > > +++ b/MAINTAINERS
>  > > @@ -1380,6 +1380,13 @@ F: include/hw/misc/mchp_pfsoc_dmc.h
>  > >  F: include/hw/misc/mchp_pfsoc_ioscb.h
>  > >  F: include/hw/misc/mchp_pfsoc_sysreg.h
>  > >
>  > > +Shakti C class SoC
>  > > +M: Vijai Kumar K <vijai@behindbytes.com>
>  > > +L: qemu-riscv@nongnu.org
>  > > +S: Supported
>  > > +F: hw/riscv/shakti_c.c
>  > > +F: include/hw/riscv/shakti_c.h
>  > > +
>  > >  SiFive Machines
>  > >  M: Alistair Francis <Alistair.Francis@wdc.com>
>  > >  M: Bin Meng <bin.meng@windriver.com>
>  > > diff --git a/default-configs/devices/riscv64-softmmu.mak 
> b/default-configs/devices/riscv64-softmmu.mak
>  > > index d5eec75f05..bc69301fa4 100644
>  > > --- a/default-configs/devices/riscv64-softmmu.mak
>  > > +++ b/default-configs/devices/riscv64-softmmu.mak
>  > > @@ -13,3 +13,4 @@ CONFIG_SIFIVE_E=y
>  > >  CONFIG_SIFIVE_U=y
>  > >  CONFIG_RISCV_VIRT=y
>  > >  CONFIG_MICROCHIP_PFSOC=y
>  > > +CONFIG_SHAKTI_C=y
>  > > diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
>  > > index d139074b02..92a62b5ce9 100644
>  > > --- a/hw/riscv/Kconfig
>  > > +++ b/hw/riscv/Kconfig
>  > > @@ -19,6 +19,16 @@ config OPENTITAN
>  > >      select IBEX
>  > >      select UNIMP
>  > >
>  > > +config SHAKTI
>  > > +    bool
>  > > +
>  > > +config SHAKTI_C
>  > > +    bool
>  > > +    select UNIMP
>  > > +    select SHAKTI
>  > > +    select SIFIVE_CLINT
>  > > +    select SIFIVE_PLIC
>  > > +
>  > >  config RISCV_VIRT
>  > >      bool
>  > >      imply PCI_DEVICES
>  > > diff --git a/hw/riscv/meson.build b/hw/riscv/meson.build
>  > > index 275c0f7eb7..a97454661c 100644
>  > > --- a/hw/riscv/meson.build
>  > > +++ b/hw/riscv/meson.build
>  > > @@ -4,6 +4,7 @@ riscv_ss.add(files('numa.c'))
>  > >  riscv_ss.add(files('riscv_hart.c'))
>  > >  riscv_ss.add(when: 'CONFIG_OPENTITAN', if_true: files('opentitan.c'))
>  > >  riscv_ss.add(when: 'CONFIG_RISCV_VIRT', if_true: files('virt.c'))
>  > > +riscv_ss.add(when: 'CONFIG_SHAKTI_C', if_true: files('shakti_c.c'))
>  > >  riscv_ss.add(when: 'CONFIG_SIFIVE_E', if_true: files('sifive_e.c'))
>  > >  riscv_ss.add(when: 'CONFIG_SIFIVE_U', if_true: files('sifive_u.c'))
>  > >  riscv_ss.add(when: 'CONFIG_SPIKE', if_true: files('spike.c'))
>  > > diff --git a/hw/riscv/shakti_c.c b/hw/riscv/shakti_c.c
>  > > new file mode 100644
>  > > index 0000000000..45d0eedabd
>  > > --- /dev/null
>  > > +++ b/hw/riscv/shakti_c.c
>  > > @@ -0,0 +1,171 @@
>  > > +/*
>  > > + * Shakti C-class SoC emulation
>  > > + *
>  > > + * Copyright (c) 2021 Vijai Kumar K <vijai@behindbytes.com>
>  > > + *
>  > > + * This program is free software; you can redistribute it and/or modify 
> it
>  > > + * under the terms and conditions of the GNU General Public License,
>  > > + * version 2 or later, as published by the Free Software Foundation.
>  > > + *
>  > > + * This program is distributed in the hope it will be useful, but 
> WITHOUT
>  > > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>  > > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public 
> License for
>  > > + * more details.
>  > > + *
>  > > + * You should have received a copy of the GNU General Public License 
> along with
>  > > + * this program.  If not, see <http://www.gnu.org/licenses/>.
>  > > + */
>  > > +
>  > > +#include "qemu/osdep.h"
>  > > +#include "hw/boards.h"
>  > > +#include "hw/riscv/shakti_c.h"
>  > > +#include "qapi/error.h"
>  > > +#include "hw/intc/sifive_plic.h"
>  > > +#include "hw/intc/sifive_clint.h"
>  > > +#include "sysemu/sysemu.h"
>  > > +#include "hw/qdev-properties.h"
>  > > +#include "exec/address-spaces.h"
>  > > +#include "hw/riscv/boot.h"
>  > > +
>  > > +
>  > > +static const struct MemmapEntry {
>  > > +    hwaddr base;
>  > > +    hwaddr size;
>  > > +} shakti_c_memmap[] = {
>  > > +    [SHAKTI_C_ROM]   =  {  0x00001000,  0x2000   },
>  > > +    [SHAKTI_C_RAM]   =  {  0x80000000,  0x0      },
>  > > +    [SHAKTI_C_UART]  =  {  0x00011300,  0x00040  },
>  > > +    [SHAKTI_C_GPIO]  =  {  0x020d0000,  0x00100  },
>  > > +    [SHAKTI_C_PLIC]  =  {  0x0c000000,  0x20000  },
>  > > +    [SHAKTI_C_CLINT] =  {  0x02000000,  0xc0000  },
>  > > +    [SHAKTI_C_DEBUG] =  {  0x00000000,  0x00010  },
>  >
>  > What is this debug used for?
>
> I could not find the answer in docs. I got in touch with the developer and 
> this is the response I got.
>
> "It where the debug self-loop lies. When halted by a debugger, the core 
> executes a loop of [nop; j nop]. This loop is placed at the debug_loop 
> location."
>
> This has no purpose in QEMU. It is there for sake of completion.

Hmm... Maybe let's just remove it then. If it's not being used, not
going to be used and not exposed to software there isn't much benefit
in keeping it.

Alistair

>
> Thanks,
> Vijai Kumar K