qemu-devel
[Top][All Lists]
Advanced

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

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


From: Cupertino Miranda
Subject: Re: [PATCH 12/14] arc: Add Synopsys ARC emulation boards
Date: Tue, 13 Oct 2020 16:04:46 +0100

Hi Philippe,

On Wed, Oct 7, 2020 at 5:31 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Hi Cupertino, Claudiu,
>
> On 9/30/20 10:46 PM, cupertinomiranda@gmail.com wrote:
> > From: Claudiu Zissulescu <claziss@synopsys.com>
> >
> > 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 <claziss@synopsys.com>
> > ---
> >  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 0000000000..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 0000000000..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 
> > <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#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;
> > +
> > +    dev = qdev_new("open_eth");
> > +    qdev_set_nic_properties(dev, nd);
> > +    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> > +
> > +    s = SYS_BUS_DEVICE(dev);
> > +    sysbus_connect_irq(s, 0, irq);
> > +    memory_region_add_subregion(address_space, base,
> > +                                sysbus_mmio_get_region(s, 0));
> > +    memory_region_add_subregion(address_space, descriptors,
> > +                                sysbus_mmio_get_region(s, 1));
> > +}
> > +
> > +static uint64_t arc_io_read(void *opaque, hwaddr addr, unsigned size)
> > +{
> > +    return 0;
> > +}
> > +
> > +static void arc_io_write(void *opaque, hwaddr addr,
> > +                         uint64_t val, unsigned size)
> > +{
> > +    switch (addr) {
> > +    case 0x08: /* board reset. */
> > +        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> > +        break;
> > +    default:
> > +        break;
> > +    }
> > +}
> > +
> > +static const MemoryRegionOps arc_io_ops = {
> > +    .read = arc_io_read,
> > +    .write = arc_io_write,
> > +    .endianness = DEVICE_NATIVE_ENDIAN,
> > +};
> > +
> > +static void arc_sim_init(MachineState *machine)
> > +{
> > +    static struct arc_boot_info boot_info;
> > +    unsigned int smp_cpus = machine->smp.cpus;
> > +    ram_addr_t ram_base = 0;
> > +    ram_addr_t ram_size = machine->ram_size;
> > +    ARCCPU *cpu = NULL;
> > +    MemoryRegion *ram, *system_io;
> > +    int n;
> > +
> > +    boot_info.ram_start = ram_base;
> > +    boot_info.ram_size = ram_size;
> > +    boot_info.kernel_filename = machine->kernel_filename;
> > +
> > +    for (n = 0; n < smp_cpus; n++) {
> > +        cpu = ARC_CPU(object_new(machine->cpu_type));
> > +        if (cpu == NULL) {
> > +            fprintf(stderr, "Unable to find CPU definition!\n");
> > +            exit(1);
> > +        }
> > +
> > +        /* Set the initial CPU properties. */
> > +        object_property_set_uint(OBJECT(cpu), "freq_hz", 1000000, 
> > &error_fatal);
> > +        object_property_set_bool(OBJECT(cpu), "rtc-opt", true, 
> > &error_fatal);
> > +        object_property_set_bool(OBJECT(cpu), "realized", true, 
> > &error_fatal);
> > +
> > +        /* Initialize internal devices. */
> > +        cpu_arc_pic_init(cpu);
> > +        cpu_arc_clock_init(cpu);
> > +
> > +        qemu_register_reset(arc_cpu_reset, cpu);
> > +    }
> > +
> > +    ram = g_new(MemoryRegion, 1);
> > +    memory_region_init_ram(ram, NULL, "arc.ram", ram_size, &error_fatal);
> > +    memory_region_add_subregion(get_system_memory(), ram_base, ram);
> > +
> > +    system_io = g_new(MemoryRegion, 1);
> > +    memory_region_init_io(system_io, NULL, &arc_io_ops, NULL, "arc.io",
> > +                           1024);
> > +    memory_region_add_subregion(get_system_memory(), 0xf0000000, 
> > system_io);
> > +
> > +    serial_mm_init(get_system_memory(), 0x90000000, 2, cpu->env.irq[20],
> > +                   115200, serial_hd(0), DEVICE_NATIVE_ENDIAN);
> > +
> > +    if (nd_table[0].used) {
> > +        arc_sim_net_init(get_system_memory(), 0x92000000,
> > +                         0x92000400, cpu->env.irq[4], nd_table);
> > +    }
> > +
> > +    arc_load_kernel(cpu, &boot_info);
> > +}
> > +
> > +static void arc_sim_machine_init(MachineClass *mc)
> > +{
> > +    mc->desc = "ARCxx simulation";
> > +    mc->init = arc_sim_init;
> > +    mc->max_cpus = 1;
> > +    mc->is_default = false;
> > +    mc->default_cpu_type = ARC_CPU_TYPE_NAME("archs");
> > +}
> > +
> > +DEFINE_MACHINE("arc-sim", arc_sim_machine_init)
>
> Can you share the link to the documentation of this simulator please?
> I couldn't find it on the link you provided in the cover
> (https://www.synopsys.com/designware-ip/processor-solutions.html)
> and https://www.synopsys.com/dw/ipdir.php?ds=sim_nSIM doesn't
> seem relevant.
The free version of this simulator is in:
https://www.synopsys.com/cgi-bin/dwarcnsim/req1.cgi

Documentation is packed in that download. I am not aware of any
location that would contain documentation for this.

In any case, we believe that we will not need any nSIM board, and we
would like to remove both the nsim board and the ARC UART
implementation.
There is no real benefit, as the support for ARC UART is being phased
out even on nSIM simulator and it does not make sense to include it
now in QEMU.

We only implemented ARC UART since initially nSIM did not support
generic UART, and we wanted to execute the same binaries on both nSIM
and QEMU.
At this point sim-hs board is doing precisely that. In any case we
want to rename it to something more meaningful.

>
> > diff --git a/hw/arc/sample.c b/hw/arc/sample.c
> > new file mode 100644
> > index 0000000000..0ecc11cf15
> > --- /dev/null
> > +++ b/hw/arc/sample.c
> > @@ -0,0 +1,77 @@
> > +/*
> > + * 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
> > + * http://www.gnu.org/licenses/lgpl-2.1.html
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qapi/error.h"
> > +#include "qemu-common.h"
> > +#include "cpu.h"
> > +#include "hw/hw.h"
> > +#include "sysemu/sysemu.h"
> > +#include "sysemu/qtest.h"
> > +#include "ui/console.h"
> > +#include "hw/boards.h"
> > +#include "hw/loader.h"
> > +#include "qemu/error-report.h"
> > +#include "exec/address-spaces.h"
> > +#include "include/hw/sysbus.h"
> > +
> > +#define SIZE_RAM 0x00020000
> > +
> > +static void sample_init(MachineState *machine)
> > +{
> > +    MemoryRegion *ram;
> > +
> > +    ARCCPU *cpu_arc ATTRIBUTE_UNUSED;
> > +
> > +    ram = g_new(MemoryRegion, 1);
> > +
> > +    cpu_arc = ARC_CPU(cpu_create("archs-" TYPE_ARC_CPU));
> > +
> > +    memory_region_init_ram(ram, NULL, "ram", SIZE_RAM, &error_fatal);
> > +    memory_region_add_subregion(get_system_memory(), PHYS_BASE_RAM, ram);
> > +
> > +    char const *firmware = NULL;
> > +    char const *filename;
> > +
> > +    if (machine->firmware) {
> > +        firmware = machine->firmware;
> > +    }
> > +
> > +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, firmware);
> > +    if (!filename) {
> > +        error_report("Could not find flash image file '%s'", firmware);
> > +        exit(1);
> > +    }
> > +
> > +    load_image_targphys(filename, PHYS_BASE_RAM + 0x100, SIZE_RAM);
> > +}
> > +
> > +static void sample_machine_init(MachineClass *mc)
> > +{
> > +    mc->desc = "ARC sample/example board";
> > +    mc->init = sample_init;
> > +    mc->is_default = false;
> > +}
> > +
> > +DEFINE_MACHINE("sample", sample_machine_init)
>
> You don't need a "sample" board, you can use the "none" machine instead.
Ok, will remove.

>
> Regards,
>
> Phil.

Regards,
Cupertino



reply via email to

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