diff mbox

target-moxie: Add moxie Marin SoC support

Message ID 87vbyqn4s3.fsf@moxielogic.com
State New
Headers show

Commit Message

Anthony Green Dec. 15, 2013, 3:59 a.m. UTC
This adds initial support for the Marin SoC, including the SoC's uart
interface.


Signed-off-by: Anthony Green <green@moxielogic.com>
---
 default-configs/moxie-softmmu.mak |   1 +
 hw/char/Makefile.objs             |   1 +
 hw/char/marin-uart.c              | 198 ++++++++++++++++++++++++++++++++++++++
 hw/moxie/Makefile.objs            |   2 +-
 hw/moxie/marin.c                  | 167 ++++++++++++++++++++++++++++++++
 5 files changed, 368 insertions(+), 1 deletion(-)
 create mode 100644 hw/char/marin-uart.c
 create mode 100644 hw/moxie/marin.c

Comments

Peter Crosthwaite Dec. 15, 2013, 5:17 a.m. UTC | #1
Hi Anthony,

On Sun, Dec 15, 2013 at 1:59 PM, Anthony Green <green@moxielogic.com> wrote:
>
> This adds initial support for the Marin SoC, including the SoC's uart
> interface.
>
>
> Signed-off-by: Anthony Green <green@moxielogic.com>
> ---
>  default-configs/moxie-softmmu.mak |   1 +
>  hw/char/Makefile.objs             |   1 +
>  hw/char/marin-uart.c              | 198 ++++++++++++++++++++++++++++++++++++++
>  hw/moxie/Makefile.objs            |   2 +-
>  hw/moxie/marin.c                  | 167 ++++++++++++++++++++++++++++++++

This should be at least two patches. One for the UART device and one
for your SoC. Maybe more depending on the descision regarding SoC v
board (see comment below).

>  5 files changed, 368 insertions(+), 1 deletion(-)
>  create mode 100644 hw/char/marin-uart.c
>  create mode 100644 hw/moxie/marin.c
>
> diff --git a/default-configs/moxie-softmmu.mak b/default-configs/moxie-softmmu.mak
> index 1a95476..65a21de 100644
> --- a/default-configs/moxie-softmmu.mak
> +++ b/default-configs/moxie-softmmu.mak
> @@ -1,5 +1,6 @@
>  # Default configuration for moxie-softmmu
>
>  CONFIG_MC146818RTC=y
> +CONFIG_MOXIE=y
>  CONFIG_SERIAL=y
>  CONFIG_VGA=y
> diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
> index cbd6a00..48bc5d0 100644
> --- a/hw/char/Makefile.objs
> +++ b/hw/char/Makefile.objs
> @@ -14,6 +14,7 @@ obj-$(CONFIG_COLDFIRE) += mcf_uart.o
>  obj-$(CONFIG_OMAP) += omap_uart.o
>  obj-$(CONFIG_SH4) += sh_serial.o
>  obj-$(CONFIG_PSERIES) += spapr_vty.o
> +obj-$(CONFIG_MOXIE) += marin-uart.o
>
>  common-obj-$(CONFIG_ETRAXFS) += etraxfs_ser.o
>  common-obj-$(CONFIG_ISA_DEBUG) += debugcon.o
> diff --git a/hw/char/marin-uart.c b/hw/char/marin-uart.c
> new file mode 100644
> index 0000000..f0d46d4
> --- /dev/null
> +++ b/hw/char/marin-uart.c
> @@ -0,0 +1,198 @@
> +/*
> + *  QEMU model of the Marin UART.
> + *
> + *  Copyright (c) 2013 Anthony Green <green@moxielogic.com>
> + *
> + * 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 "hw/hw.h"
> +#include "hw/sysbus.h"
> +#include "trace.h"
> +#include "sysemu/char.h"
> +#include "qemu/error-report.h"
> +
> +enum {
> +    R_RXREADY = 0,
> +    R_TXREADY,
> +    R_RXBYTE,
> +    R_TXBYTE,
> +    R_MAX
> +};
> +
> +#define TYPE_MARIN_UART "marin-uart"
> +#define MARIN_UART(obj) OBJECT_CHECK(MarinUartState, (obj), TYPE_MARIN_UART)
> +
> +struct MarinUartState {

Check your QoM conventions coding style.

http://wiki.qemu.org/QOMConventions

/* < private > */

> +    SysBusDevice busdev;

SysBusDevice parent_obj;

/* < public > */

> +    MemoryRegion regs_region;
> +    CharDriverState *chr;
> +    qemu_irq irq;
> +
> +    uint16_t regs[R_MAX];
> +};
> +typedef struct MarinUartState MarinUartState;
> +

You could just do the typedefing in the struct decl above to save this LOC.

> +static void uart_update_irq(MarinUartState *s)
> +{
> +}

Why the NOP function?

> +
> +static uint64_t uart_read(void *opaque, hwaddr addr,
> +                          unsigned size)
> +{
> +    MarinUartState *s = opaque;
> +    uint32_t r = 0;
> +
> +    addr >>= 1;
> +    switch (addr) {
> +    case R_RXREADY:
> +        r = s->regs[R_RXREADY];

You do kind of defeat the purpose of arrayified regs, if you just
index them all one by one maually. Can you have a default of r =
s->regs[addr]? ...

> +        break;
> +    case R_TXREADY:
> +        r = 1;

which is then overriden by this exceptional case.

> +        break;
> +    case R_TXBYTE:
> +        r = s->regs[R_TXBYTE];
> +        break;
> +    case R_RXBYTE:
> +        r = s->regs[R_RXBYTE];
> +        s->regs[R_RXREADY] = 0;
> +        qemu_chr_accept_input(s->chr);

Do you need a NULL guard on s->chr here?

> +        break;
> +    default:
> +        error_report("marin_uart: read access to unknown register 0x"
> +                TARGET_FMT_plx, addr << 1);
> +        break;

This is a guest error and should use qemu_log_mask(LOG_GUEST_ERROR, .
Same for write handler below.

> +    }
> +
> +    return r;
> +}
> +
> +static void uart_write(void *opaque, hwaddr addr, uint64_t value,
> +                       unsigned size)
> +{
> +    MarinUartState *s = opaque;
> +    unsigned char ch = value;
> +
> +    addr >>= 1;
> +    switch (addr) {
> +    case R_TXBYTE:
> +        if (s->chr) {
> +            qemu_chr_fe_write(s->chr, &ch, 1);

What happens if qemu_chr_fe_write short returns? Do you just drop your char?

qemu_chr_fe_write_all will improve this, although it has problems too
(that i'm working on myself).

> +        }
> +        break;
> +
> +    default:
> +        error_report("marin_uart: write access to unknown register 0x"
> +                TARGET_FMT_plx, addr << 1);
> +        break;
> +    }
> +
> +    uart_update_irq(s);
> +}
> +
> +static const MemoryRegionOps uart_mmio_ops = {
> +    .read = uart_read,
> +    .write = uart_write,
> +    .valid = {
> +        .min_access_size = 2,
> +        .max_access_size = 2,
> +    },
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static void uart_rx(void *opaque, const uint8_t *buf, int size)
> +{
> +    MarinUartState *s = opaque;
> +
> +    s->regs[R_RXBYTE] = *buf;
> +    s->regs[R_RXREADY] = 1;
> +
> +    uart_update_irq(s);
> +}
> +
> +static int uart_can_rx(void *opaque)
> +{
> +    MarinUartState *s = opaque;
> +
> +    return !(s->regs[R_RXREADY]);
> +}
> +
> +static void uart_event(void *opaque, int event)
> +{
> +}
> +
> +static void marin_uart_reset(DeviceState *d)
> +{
> +    MarinUartState *s = MARIN_UART(d);
> +    int i;
> +
> +    for (i = 0; i < R_MAX; i++) {
> +        s->regs[i] = 0;

Can you just reset your TX_READY bit to 1? This would cleanup the
exceptional case i mentioned above, and anyone introspecting your
device with gdb will see the true and correct value in s->regs.

> +    }
> +}
> +
> +static int marin_uart_init(SysBusDevice *dev)

Use of the SysBusDevice::init function is depracted, Please use the
device::realise or object::init functions instead. Check Antony
Pavlovs Digic UART (on list and in late stages of review) for the most
modern example of a QEMU UART.

> +{
> +    MarinUartState *s = MARIN_UART(dev);
> +
> +    sysbus_init_irq(dev, &s->irq);
> +
> +    memory_region_init_io(&s->regs_region, OBJECT(s), &uart_mmio_ops, s,
> +            TYPE_MARIN_UART, R_MAX * 4);
> +    sysbus_init_mmio(dev, &s->regs_region);
> +
> +    s->chr = qemu_char_get_next_serial();
> +    if (s->chr) {
> +        qemu_chr_add_handlers(s->chr, uart_can_rx, uart_rx, uart_event, s);
> +    }
> +
> +    return 0;
> +}
> +
> +static const VMStateDescription vmstate_marin_uart = {
> +    .name = TYPE_MARIN_UART,
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT16_ARRAY(regs, MarinUartState, R_MAX),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static void marin_uart_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);

This will go away when you convert the init fn.

> +
> +    k->init = marin_uart_init;
> +    dc->reset = marin_uart_reset;
> +    dc->vmsd = &vmstate_marin_uart;
> +}
> +
> +static const TypeInfo marin_uart_info = {
> +    .name          = TYPE_MARIN_UART,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(MarinUartState),
> +    .class_init    = marin_uart_class_init,
> +};
> +
> +static void marin_uart_register_types(void)
> +{
> +    type_register_static(&marin_uart_info);
> +}
> +
> +type_init(marin_uart_register_types)
> diff --git a/hw/moxie/Makefile.objs b/hw/moxie/Makefile.objs
> index bfc9001..4fa3b30 100644
> --- a/hw/moxie/Makefile.objs
> +++ b/hw/moxie/Makefile.objs
> @@ -1,2 +1,2 @@
>  # moxie boards
> -obj-y += moxiesim.o
> +obj-y += moxiesim.o marin.o
> diff --git a/hw/moxie/marin.c b/hw/moxie/marin.c
> new file mode 100644
> index 0000000..0a998e4
> --- /dev/null
> +++ b/hw/moxie/marin.c
> @@ -0,0 +1,167 @@
> +/*
> + * QEMU/marin SoC emulation
> + *
> + * Emulates the FPGA-hosted Marin SoC
> + *
> + * Copyright (c) 2013 Anthony Green
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +#include "hw/sysbus.h"
> +#include "hw/hw.h"
> +#include "hw/boards.h"
> +#include "hw/loader.h"
> +#include "exec/address-spaces.h"
> +
> +typedef struct {
> +    uint64_t ram_size;
> +    const char *kernel_filename;
> +    const char *kernel_cmdline;
> +    const char *initrd_filename;
> +} LoaderParams;
> +
> +static void load_kernel(MoxieCPU *cpu, LoaderParams *loader_params)
> +{
> +    uint64_t entry, kernel_low, kernel_high;
> +    long kernel_size;
> +    long initrd_size;
> +    ram_addr_t initrd_offset;
> +
> +    kernel_size = load_elf(loader_params->kernel_filename,  NULL, NULL,
> +                           &entry, &kernel_low, &kernel_high, 1,
> +                           ELF_MACHINE, 0);
> +
> +    if (!kernel_size) {
> +        fprintf(stderr, "qemu: could not load kernel '%s'\n",
> +                loader_params->kernel_filename);

error_report()

> +        exit(1);
> +    }
> +
> +    /* load initrd */
> +    initrd_size = 0;
> +    initrd_offset = 0;
> +    if (loader_params->initrd_filename) {
> +        initrd_size = get_image_size(loader_params->initrd_filename);
> +        if (initrd_size > 0) {
> +            initrd_offset = (kernel_high + ~TARGET_PAGE_MASK)
> +              & TARGET_PAGE_MASK;
> +            if (initrd_offset + initrd_size > loader_params->ram_size) {
> +                fprintf(stderr,
> +                        "qemu: memory too small for initial ram disk '%s'\n",
> +                        loader_params->initrd_filename);
> +                exit(1);
> +            }
> +            initrd_size = load_image_targphys(loader_params->initrd_filename,
> +                                              initrd_offset,
> +                                              ram_size);
> +        }
> +        if (initrd_size == (target_ulong)-1) {
> +            fprintf(stderr, "qemu: could not load initial ram disk '%s'\n",
> +                    loader_params->initrd_filename);
> +            exit(1);
> +        }
> +    }
> +}

You should consider pulling your bootloader our into a seperate file
(and patch). Does Moxie define a specific linux boot protocol? Check
hw/arm/boot.c or hw/arm/microblaze.c for examples of modular
bootloaders.

> +
> +static void main_cpu_reset(void *opaque)
> +{
> +    MoxieCPU *cpu = opaque;
> +
> +    cpu_reset(CPU(cpu));
> +}
> +
> +static inline DeviceState *marin_uart_create(hwaddr base,
> +        qemu_irq irq)
> +{
> +    DeviceState *dev;
> +
> +    dev = qdev_create(NULL, "marin-uart");
> +    qdev_init_nofail(dev);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq);
> +
> +    return dev;
> +}

This is an old style qdev init function.

> +
> +static void marin_init(QEMUMachineInitArgs *args)
> +{
> +    MoxieCPU *cpu = NULL;
> +    ram_addr_t ram_size = args->ram_size;
> +    const char *cpu_model = args->cpu_model;
> +    const char *kernel_filename = args->kernel_filename;
> +    const char *kernel_cmdline = args->kernel_cmdline;
> +    const char *initrd_filename = args->initrd_filename;
> +    CPUMoxieState *env;
> +    MemoryRegion *address_space_mem = get_system_memory();
> +    MemoryRegion *ocram = g_new(MemoryRegion, 1);
> +    MemoryRegion *ram = g_new(MemoryRegion, 1);
> +    MemoryRegion *rom = g_new(MemoryRegion, 1);
> +    hwaddr ram_base = 0x30000000;
> +    LoaderParams loader_params;
> +
> +    /* Init CPUs. */
> +    if (cpu_model == NULL) {
> +        cpu_model = "MoxieLite-moxie-cpu";
> +    }
> +    cpu = cpu_moxie_init(cpu_model);
> +    if (!cpu) {
> +        fprintf(stderr, "Unable to find CPU definition\n");

error_report()

> +        exit(1);
> +    }
> +    env = &cpu->env;
> +
> +    qemu_register_reset(main_cpu_reset, cpu);
> +
> +    /* Allocate RAM. */
> +    memory_region_init_ram(ocram, NULL, "marin-onchip.ram", 0x1000*4);
> +    vmstate_register_ram_global(ocram);
> +    memory_region_add_subregion(address_space_mem, 0x10000000, ocram);
> +
> +    memory_region_init_ram(ram, NULL, "marin-external.ram", ram_size);
> +    vmstate_register_ram_global(ram);
> +    memory_region_add_subregion(address_space_mem, ram_base, ram);
> +
> +    memory_region_init_ram(rom, NULL, "moxie.rom", 128*0x1000);
> +    vmstate_register_ram_global(rom);
> +    memory_region_add_subregion(get_system_memory(), 0x1000, rom);
> +
> +    if (kernel_filename) {
> +        loader_params.ram_size = ram_size;
> +        loader_params.kernel_filename = kernel_filename;
> +        loader_params.kernel_cmdline = kernel_cmdline;
> +        loader_params.initrd_filename = initrd_filename;
> +        load_kernel(cpu, &loader_params);
> +    }
> +
> +    marin_uart_create(0xF0000008, env->irq[4]);
> +}
> +
> +static QEMUMachine marin_machine = {
> +    .name = "marin",
> +    .desc = "Marin SoC",

So SoCs should generally be implemented on two levels. There is the
SoC device, which contains the devices that are on the SoC chip, then
the board level instantiates the SoC. This looks like a flat
board-and-SoC in one (on board level). Your deisgn is trivial so far
(and good for a first series), but long term what is the organsation?
Is this going towards a particular board emulation? Have a look at
Liguangs Allwinner series (and some of the early review comments) for
a discussion on this topic:

http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg03940.html

As a starting point, can you tell us what is and isn't hosted on the
FPGA in this board model? That might be the best way to split this.

Regards,
Peter

> +    .init = marin_init,
> +    .is_default = 1,
> +};
> +
> +static void moxie_machine_init(void)
> +{
> +    qemu_register_machine(&marin_machine);
> +}
> +
> +machine_init(moxie_machine_init)
> --
> 1.8.3.1
>
>
Peter Crosthwaite Dec. 15, 2013, 5:27 a.m. UTC | #2
On Sun, Dec 15, 2013 at 3:17 PM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> Hi Anthony,
>
> On Sun, Dec 15, 2013 at 1:59 PM, Anthony Green <green@moxielogic.com> wrote:
>>
>> This adds initial support for the Marin SoC, including the SoC's uart
>> interface.
>>
>>
>> Signed-off-by: Anthony Green <green@moxielogic.com>
>> ---
>>  default-configs/moxie-softmmu.mak |   1 +
>>  hw/char/Makefile.objs             |   1 +
>>  hw/char/marin-uart.c              | 198 ++++++++++++++++++++++++++++++++++++++
>>  hw/moxie/Makefile.objs            |   2 +-
>>  hw/moxie/marin.c                  | 167 ++++++++++++++++++++++++++++++++
>
> This should be at least two patches. One for the UART device and one
> for your SoC. Maybe more depending on the descision regarding SoC v
> board (see comment below).
>
>>  5 files changed, 368 insertions(+), 1 deletion(-)
>>  create mode 100644 hw/char/marin-uart.c
>>  create mode 100644 hw/moxie/marin.c
>>
>> diff --git a/default-configs/moxie-softmmu.mak b/default-configs/moxie-softmmu.mak
>> index 1a95476..65a21de 100644
>> --- a/default-configs/moxie-softmmu.mak
>> +++ b/default-configs/moxie-softmmu.mak
>> @@ -1,5 +1,6 @@
>>  # Default configuration for moxie-softmmu
>>
>>  CONFIG_MC146818RTC=y
>> +CONFIG_MOXIE=y
>>  CONFIG_SERIAL=y
>>  CONFIG_VGA=y
>> diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
>> index cbd6a00..48bc5d0 100644
>> --- a/hw/char/Makefile.objs
>> +++ b/hw/char/Makefile.objs
>> @@ -14,6 +14,7 @@ obj-$(CONFIG_COLDFIRE) += mcf_uart.o
>>  obj-$(CONFIG_OMAP) += omap_uart.o
>>  obj-$(CONFIG_SH4) += sh_serial.o
>>  obj-$(CONFIG_PSERIES) += spapr_vty.o
>> +obj-$(CONFIG_MOXIE) += marin-uart.o
>>
>>  common-obj-$(CONFIG_ETRAXFS) += etraxfs_ser.o
>>  common-obj-$(CONFIG_ISA_DEBUG) += debugcon.o
>> diff --git a/hw/char/marin-uart.c b/hw/char/marin-uart.c
>> new file mode 100644
>> index 0000000..f0d46d4
>> --- /dev/null
>> +++ b/hw/char/marin-uart.c
>> @@ -0,0 +1,198 @@
>> +/*
>> + *  QEMU model of the Marin UART.
>> + *
>> + *  Copyright (c) 2013 Anthony Green <green@moxielogic.com>
>> + *
>> + * 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 "hw/hw.h"
>> +#include "hw/sysbus.h"
>> +#include "trace.h"
>> +#include "sysemu/char.h"
>> +#include "qemu/error-report.h"
>> +
>> +enum {
>> +    R_RXREADY = 0,
>> +    R_TXREADY,
>> +    R_RXBYTE,
>> +    R_TXBYTE,
>> +    R_MAX
>> +};
>> +
>> +#define TYPE_MARIN_UART "marin-uart"
>> +#define MARIN_UART(obj) OBJECT_CHECK(MarinUartState, (obj), TYPE_MARIN_UART)
>> +
>> +struct MarinUartState {
>
> Check your QoM conventions coding style.
>
> http://wiki.qemu.org/QOMConventions
>
> /* < private > */
>
>> +    SysBusDevice busdev;
>
> SysBusDevice parent_obj;
>
> /* < public > */
>
>> +    MemoryRegion regs_region;
>> +    CharDriverState *chr;
>> +    qemu_irq irq;
>> +
>> +    uint16_t regs[R_MAX];
>> +};
>> +typedef struct MarinUartState MarinUartState;
>> +
>
> You could just do the typedefing in the struct decl above to save this LOC.
>
>> +static void uart_update_irq(MarinUartState *s)
>> +{
>> +}
>
> Why the NOP function?
>
>> +
>> +static uint64_t uart_read(void *opaque, hwaddr addr,
>> +                          unsigned size)
>> +{
>> +    MarinUartState *s = opaque;
>> +    uint32_t r = 0;
>> +
>> +    addr >>= 1;
>> +    switch (addr) {
>> +    case R_RXREADY:
>> +        r = s->regs[R_RXREADY];
>
> You do kind of defeat the purpose of arrayified regs, if you just
> index them all one by one maually. Can you have a default of r =
> s->regs[addr]? ...
>
>> +        break;
>> +    case R_TXREADY:
>> +        r = 1;
>
> which is then overriden by this exceptional case.
>
>> +        break;
>> +    case R_TXBYTE:
>> +        r = s->regs[R_TXBYTE];
>> +        break;
>> +    case R_RXBYTE:
>> +        r = s->regs[R_RXBYTE];
>> +        s->regs[R_RXREADY] = 0;
>> +        qemu_chr_accept_input(s->chr);
>
> Do you need a NULL guard on s->chr here?
>
>> +        break;
>> +    default:
>> +        error_report("marin_uart: read access to unknown register 0x"
>> +                TARGET_FMT_plx, addr << 1);
>> +        break;
>
> This is a guest error and should use qemu_log_mask(LOG_GUEST_ERROR, .
> Same for write handler below.
>
>> +    }
>> +
>> +    return r;
>> +}
>> +
>> +static void uart_write(void *opaque, hwaddr addr, uint64_t value,
>> +                       unsigned size)
>> +{
>> +    MarinUartState *s = opaque;
>> +    unsigned char ch = value;
>> +
>> +    addr >>= 1;
>> +    switch (addr) {
>> +    case R_TXBYTE:
>> +        if (s->chr) {
>> +            qemu_chr_fe_write(s->chr, &ch, 1);
>
> What happens if qemu_chr_fe_write short returns? Do you just drop your char?
>
> qemu_chr_fe_write_all will improve this, although it has problems too
> (that i'm working on myself).
>
>> +        }
>> +        break;
>> +
>> +    default:
>> +        error_report("marin_uart: write access to unknown register 0x"
>> +                TARGET_FMT_plx, addr << 1);
>> +        break;
>> +    }
>> +
>> +    uart_update_irq(s);
>> +}
>> +
>> +static const MemoryRegionOps uart_mmio_ops = {
>> +    .read = uart_read,
>> +    .write = uart_write,
>> +    .valid = {
>> +        .min_access_size = 2,
>> +        .max_access_size = 2,
>> +    },
>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>> +};
>> +
>> +static void uart_rx(void *opaque, const uint8_t *buf, int size)
>> +{
>> +    MarinUartState *s = opaque;
>> +
>> +    s->regs[R_RXBYTE] = *buf;
>> +    s->regs[R_RXREADY] = 1;
>> +
>> +    uart_update_irq(s);
>> +}
>> +
>> +static int uart_can_rx(void *opaque)
>> +{
>> +    MarinUartState *s = opaque;
>> +
>> +    return !(s->regs[R_RXREADY]);
>> +}
>> +
>> +static void uart_event(void *opaque, int event)
>> +{
>> +}
>> +
>> +static void marin_uart_reset(DeviceState *d)
>> +{
>> +    MarinUartState *s = MARIN_UART(d);
>> +    int i;
>> +
>> +    for (i = 0; i < R_MAX; i++) {
>> +        s->regs[i] = 0;
>
> Can you just reset your TX_READY bit to 1? This would cleanup the
> exceptional case i mentioned above, and anyone introspecting your
> device with gdb will see the true and correct value in s->regs.
>
>> +    }
>> +}
>> +
>> +static int marin_uart_init(SysBusDevice *dev)
>
> Use of the SysBusDevice::init function is depracted, Please use the
> device::realise or object::init functions instead. Check Antony
> Pavlovs Digic UART (on list and in late stages of review) for the most
> modern example of a QEMU UART.
>
>> +{
>> +    MarinUartState *s = MARIN_UART(dev);
>> +
>> +    sysbus_init_irq(dev, &s->irq);
>> +
>> +    memory_region_init_io(&s->regs_region, OBJECT(s), &uart_mmio_ops, s,
>> +            TYPE_MARIN_UART, R_MAX * 4);
>> +    sysbus_init_mmio(dev, &s->regs_region);
>> +
>> +    s->chr = qemu_char_get_next_serial();
>> +    if (s->chr) {
>> +        qemu_chr_add_handlers(s->chr, uart_can_rx, uart_rx, uart_event, s);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static const VMStateDescription vmstate_marin_uart = {
>> +    .name = TYPE_MARIN_UART,
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .minimum_version_id_old = 1,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_UINT16_ARRAY(regs, MarinUartState, R_MAX),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +static void marin_uart_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>
> This will go away when you convert the init fn.
>
>> +
>> +    k->init = marin_uart_init;
>> +    dc->reset = marin_uart_reset;
>> +    dc->vmsd = &vmstate_marin_uart;
>> +}
>> +
>> +static const TypeInfo marin_uart_info = {
>> +    .name          = TYPE_MARIN_UART,
>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>> +    .instance_size = sizeof(MarinUartState),
>> +    .class_init    = marin_uart_class_init,
>> +};
>> +
>> +static void marin_uart_register_types(void)
>> +{
>> +    type_register_static(&marin_uart_info);
>> +}
>> +
>> +type_init(marin_uart_register_types)
>> diff --git a/hw/moxie/Makefile.objs b/hw/moxie/Makefile.objs
>> index bfc9001..4fa3b30 100644
>> --- a/hw/moxie/Makefile.objs
>> +++ b/hw/moxie/Makefile.objs
>> @@ -1,2 +1,2 @@
>>  # moxie boards
>> -obj-y += moxiesim.o
>> +obj-y += moxiesim.o marin.o
>> diff --git a/hw/moxie/marin.c b/hw/moxie/marin.c
>> new file mode 100644
>> index 0000000..0a998e4
>> --- /dev/null
>> +++ b/hw/moxie/marin.c
>> @@ -0,0 +1,167 @@
>> +/*
>> + * QEMU/marin SoC emulation
>> + *
>> + * Emulates the FPGA-hosted Marin SoC
>> + *
>> + * Copyright (c) 2013 Anthony Green
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a copy
>> + * of this software and associated documentation files (the "Software"), to deal
>> + * in the Software without restriction, including without limitation the rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>> + */
>> +#include "hw/sysbus.h"
>> +#include "hw/hw.h"
>> +#include "hw/boards.h"
>> +#include "hw/loader.h"
>> +#include "exec/address-spaces.h"
>> +
>> +typedef struct {
>> +    uint64_t ram_size;
>> +    const char *kernel_filename;
>> +    const char *kernel_cmdline;
>> +    const char *initrd_filename;
>> +} LoaderParams;
>> +
>> +static void load_kernel(MoxieCPU *cpu, LoaderParams *loader_params)
>> +{
>> +    uint64_t entry, kernel_low, kernel_high;
>> +    long kernel_size;
>> +    long initrd_size;
>> +    ram_addr_t initrd_offset;
>> +
>> +    kernel_size = load_elf(loader_params->kernel_filename,  NULL, NULL,
>> +                           &entry, &kernel_low, &kernel_high, 1,
>> +                           ELF_MACHINE, 0);
>> +
>> +    if (!kernel_size) {
>> +        fprintf(stderr, "qemu: could not load kernel '%s'\n",
>> +                loader_params->kernel_filename);
>
> error_report()
>
>> +        exit(1);
>> +    }
>> +
>> +    /* load initrd */
>> +    initrd_size = 0;
>> +    initrd_offset = 0;
>> +    if (loader_params->initrd_filename) {
>> +        initrd_size = get_image_size(loader_params->initrd_filename);
>> +        if (initrd_size > 0) {
>> +            initrd_offset = (kernel_high + ~TARGET_PAGE_MASK)
>> +              & TARGET_PAGE_MASK;
>> +            if (initrd_offset + initrd_size > loader_params->ram_size) {
>> +                fprintf(stderr,
>> +                        "qemu: memory too small for initial ram disk '%s'\n",
>> +                        loader_params->initrd_filename);
>> +                exit(1);
>> +            }
>> +            initrd_size = load_image_targphys(loader_params->initrd_filename,
>> +                                              initrd_offset,
>> +                                              ram_size);
>> +        }
>> +        if (initrd_size == (target_ulong)-1) {
>> +            fprintf(stderr, "qemu: could not load initial ram disk '%s'\n",
>> +                    loader_params->initrd_filename);
>> +            exit(1);
>> +        }
>> +    }
>> +}
>
> You should consider pulling your bootloader our into a seperate file
> (and patch). Does Moxie define a specific linux boot protocol? Check
> hw/arm/boot.c or hw/arm/microblaze.c for examples of modular
> bootloaders.
>

hw/microblaze/boot.c. Sry.

>> +
>> +static void main_cpu_reset(void *opaque)
>> +{
>> +    MoxieCPU *cpu = opaque;
>> +
>> +    cpu_reset(CPU(cpu));
>> +}
>> +
>> +static inline DeviceState *marin_uart_create(hwaddr base,
>> +        qemu_irq irq)
>> +{
>> +    DeviceState *dev;
>> +
>> +    dev = qdev_create(NULL, "marin-uart");
>> +    qdev_init_nofail(dev);
>> +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
>> +    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq);
>> +
>> +    return dev;
>> +}
>
> This is an old style qdev init function.
>
>> +
>> +static void marin_init(QEMUMachineInitArgs *args)
>> +{
>> +    MoxieCPU *cpu = NULL;
>> +    ram_addr_t ram_size = args->ram_size;
>> +    const char *cpu_model = args->cpu_model;
>> +    const char *kernel_filename = args->kernel_filename;
>> +    const char *kernel_cmdline = args->kernel_cmdline;
>> +    const char *initrd_filename = args->initrd_filename;
>> +    CPUMoxieState *env;
>> +    MemoryRegion *address_space_mem = get_system_memory();
>> +    MemoryRegion *ocram = g_new(MemoryRegion, 1);
>> +    MemoryRegion *ram = g_new(MemoryRegion, 1);
>> +    MemoryRegion *rom = g_new(MemoryRegion, 1);
>> +    hwaddr ram_base = 0x30000000;
>> +    LoaderParams loader_params;
>> +
>> +    /* Init CPUs. */
>> +    if (cpu_model == NULL) {
>> +        cpu_model = "MoxieLite-moxie-cpu";
>> +    }
>> +    cpu = cpu_moxie_init(cpu_model);
>> +    if (!cpu) {
>> +        fprintf(stderr, "Unable to find CPU definition\n");
>
> error_report()
>
>> +        exit(1);
>> +    }
>> +    env = &cpu->env;
>> +
>> +    qemu_register_reset(main_cpu_reset, cpu);
>> +
>> +    /* Allocate RAM. */
>> +    memory_region_init_ram(ocram, NULL, "marin-onchip.ram", 0x1000*4);
>> +    vmstate_register_ram_global(ocram);
>> +    memory_region_add_subregion(address_space_mem, 0x10000000, ocram);
>> +
>> +    memory_region_init_ram(ram, NULL, "marin-external.ram", ram_size);
>> +    vmstate_register_ram_global(ram);
>> +    memory_region_add_subregion(address_space_mem, ram_base, ram);
>> +
>> +    memory_region_init_ram(rom, NULL, "moxie.rom", 128*0x1000);
>> +    vmstate_register_ram_global(rom);
>> +    memory_region_add_subregion(get_system_memory(), 0x1000, rom);
>> +
>> +    if (kernel_filename) {
>> +        loader_params.ram_size = ram_size;
>> +        loader_params.kernel_filename = kernel_filename;
>> +        loader_params.kernel_cmdline = kernel_cmdline;
>> +        loader_params.initrd_filename = initrd_filename;
>> +        load_kernel(cpu, &loader_params);
>> +    }
>> +
>> +    marin_uart_create(0xF0000008, env->irq[4]);
>> +}
>> +
>> +static QEMUMachine marin_machine = {
>> +    .name = "marin",
>> +    .desc = "Marin SoC",
>
> So SoCs should generally be implemented on two levels. There is the
> SoC device, which contains the devices that are on the SoC chip, then
> the board level instantiates the SoC. This looks like a flat
> board-and-SoC in one (on board level). Your deisgn is trivial so far
> (and good for a first series), but long term what is the organsation?
> Is this going towards a particular board emulation? Have a look at
> Liguangs Allwinner series (and some of the early review comments) for
> a discussion on this topic:
>
> http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg03940.html
>
> As a starting point, can you tell us what is and isn't hosted on the
> FPGA in this board model? That might be the best way to split this.
>
> Regards,
> Peter
>
>> +    .init = marin_init,
>> +    .is_default = 1,
>> +};
>> +
>> +static void moxie_machine_init(void)
>> +{
>> +    qemu_register_machine(&marin_machine);
>> +}
>> +
>> +machine_init(moxie_machine_init)
>> --
>> 1.8.3.1
>>
>>
Anthony Green Dec. 15, 2013, 12:48 p.m. UTC | #3
Peter - thank you for taking the time to review my patch.  Comments
below.

Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes:

> Hi Anthony,
>
> On Sun, Dec 15, 2013 at 1:59 PM, Anthony Green <green@moxielogic.com> wrote:
>>
>> This adds initial support for the Marin SoC, including the SoC's uart
>> interface.
>>
>>
>> Signed-off-by: Anthony Green <green@moxielogic.com>
>> ---
>>  default-configs/moxie-softmmu.mak |   1 +
>>  hw/char/Makefile.objs             |   1 +
>>  hw/char/marin-uart.c | 198 ++++++++++++++++++++++++++++++++++++++
>>  hw/moxie/Makefile.objs            |   2 +-
>>  hw/moxie/marin.c                  | 167 ++++++++++++++++++++++++++++++++
>
> This should be at least two patches. One for the UART device and one
> for your SoC. Maybe more depending on the descision regarding SoC v
> board (see comment below).

Ok, I can split these.


>>  5 files changed, 368 insertions(+), 1 deletion(-)
>>  create mode 100644 hw/char/marin-uart.c
>>  create mode 100644 hw/moxie/marin.c
>>
>> diff --git a/default-configs/moxie-softmmu.mak
>> b/default-configs/moxie-softmmu.mak
>> index 1a95476..65a21de 100644
>> --- a/default-configs/moxie-softmmu.mak
>> +++ b/default-configs/moxie-softmmu.mak
>> @@ -1,5 +1,6 @@
>>  # Default configuration for moxie-softmmu
>>
>>  CONFIG_MC146818RTC=y
>> +CONFIG_MOXIE=y
>>  CONFIG_SERIAL=y
>>  CONFIG_VGA=y
>> diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
>> index cbd6a00..48bc5d0 100644
>> --- a/hw/char/Makefile.objs
>> +++ b/hw/char/Makefile.objs
>> @@ -14,6 +14,7 @@ obj-$(CONFIG_COLDFIRE) += mcf_uart.o
>>  obj-$(CONFIG_OMAP) += omap_uart.o
>>  obj-$(CONFIG_SH4) += sh_serial.o
>>  obj-$(CONFIG_PSERIES) += spapr_vty.o
>> +obj-$(CONFIG_MOXIE) += marin-uart.o
>>
>>  common-obj-$(CONFIG_ETRAXFS) += etraxfs_ser.o
>>  common-obj-$(CONFIG_ISA_DEBUG) += debugcon.o
>> diff --git a/hw/char/marin-uart.c b/hw/char/marin-uart.c
>> new file mode 100644
>> index 0000000..f0d46d4
>> --- /dev/null
>> +++ b/hw/char/marin-uart.c
>> @@ -0,0 +1,198 @@
>> +/*
>> + *  QEMU model of the Marin UART.
>> + *
>> + *  Copyright (c) 2013 Anthony Green <green@moxielogic.com>
>> + *
>> + * 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 "hw/hw.h"
>> +#include "hw/sysbus.h"
>> +#include "trace.h"
>> +#include "sysemu/char.h"
>> +#include "qemu/error-report.h"
>> +
>> +enum {
>> +    R_RXREADY = 0,
>> +    R_TXREADY,
>> +    R_RXBYTE,
>> +    R_TXBYTE,
>> +    R_MAX
>> +};
>> +
>> +#define TYPE_MARIN_UART "marin-uart"
>> +#define MARIN_UART(obj) OBJECT_CHECK(MarinUartState, (obj), TYPE_MARIN_UART)
>> +
>> +struct MarinUartState {
>
> Check your QoM conventions coding style.
>
> http://wiki.qemu.org/QOMConventions

Thanks for the pointer.


>
> /* < private > */
>
>> +    SysBusDevice busdev;
>
> SysBusDevice parent_obj;
>
> /* < public > */
>
>> +    MemoryRegion regs_region;
>> +    CharDriverState *chr;
>> +    qemu_irq irq;
>> +
>> +    uint16_t regs[R_MAX];
>> +};
>> +typedef struct MarinUartState MarinUartState;
>> +
>
> You could just do the typedefing in the struct decl above to save this
> LOC.

Yes.


>
>> +static void uart_update_irq(MarinUartState *s)
>> +{
>> +}
>
> Why the NOP function?

Place holder for a WIP.  I'll remove it.  The Marin SoC has an interrupt
controller as well. but I haven't modeled it in QEMU yet.


>> +
>> +static uint64_t uart_read(void *opaque, hwaddr addr,
>> +                          unsigned size)
>> +{
>> +    MarinUartState *s = opaque;
>> +    uint32_t r = 0;
>> +
>> +    addr >>= 1;
>> +    switch (addr) {
>> +    case R_RXREADY:
>> +        r = s->regs[R_RXREADY];
>
> You do kind of defeat the purpose of arrayified regs, if you just
> index them all one by one maually. Can you have a default of r =
> s->regs[addr]? ...

I suppose you could, but even if you fix the R_TXREADY special case,
you'd still have to handle the R_RXBYTE special case, and the range test
currently handled by 'default'.  I don't think it's much savings, and
the current switch statement is simpler to understand IMO.

>
>> +        break;
>> +    case R_TXREADY:
>> +        r = 1;
>
> which is then overriden by this exceptional case.
>
>> +        break;
>> +    case R_TXBYTE:
>> +        r = s->regs[R_TXBYTE];
>> +        break;
>> +    case R_RXBYTE:
>> +        r = s->regs[R_RXBYTE];
>> +        s->regs[R_RXREADY] = 0;
>> +        qemu_chr_accept_input(s->chr);
>
> Do you need a NULL guard on s->chr here?

I can add one.


>
>> +        break;
>> +    default:
>> +        error_report("marin_uart: read access to unknown register 0x"
>> +                TARGET_FMT_plx, addr << 1);
>> +        break;
>
> This is a guest error and should use qemu_log_mask(LOG_GUEST_ERROR, .
> Same for write handler below.

Ok, thanks.


>
>> +    }
>> +
>> +    return r;
>> +}
>> +
>> +static void uart_write(void *opaque, hwaddr addr, uint64_t value,
>> +                       unsigned size)
>> +{
>> +    MarinUartState *s = opaque;
>> +    unsigned char ch = value;
>> +
>> +    addr >>= 1;
>> +    switch (addr) {
>> +    case R_TXBYTE:
>> +        if (s->chr) {
>> +            qemu_chr_fe_write(s->chr, &ch, 1);
>
> What happens if qemu_chr_fe_write short returns? Do you just drop your
> char?

That's right.

> qemu_chr_fe_write_all will improve this, although it has problems too
> (that i'm working on myself).
>
>> +        }
>> +        break;
>> +
>> +    default:
>> +        error_report("marin_uart: write access to unknown register 0x"
>> +                TARGET_FMT_plx, addr << 1);
>> +        break;
>> +    }
>> +
>> +    uart_update_irq(s);
>> +}
>> +
>> +static const MemoryRegionOps uart_mmio_ops = {
>> +    .read = uart_read,
>> +    .write = uart_write,
>> +    .valid = {
>> +        .min_access_size = 2,
>> +        .max_access_size = 2,
>> +    },
>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>> +};
>> +
>> +static void uart_rx(void *opaque, const uint8_t *buf, int size)
>> +{
>> +    MarinUartState *s = opaque;
>> +
>> +    s->regs[R_RXBYTE] = *buf;
>> +    s->regs[R_RXREADY] = 1;
>> +
>> +    uart_update_irq(s);
>> +}
>> +
>> +static int uart_can_rx(void *opaque)
>> +{
>> +    MarinUartState *s = opaque;
>> +
>> +    return !(s->regs[R_RXREADY]);
>> +}
>> +
>> +static void uart_event(void *opaque, int event)
>> +{
>> +}
>> +
>> +static void marin_uart_reset(DeviceState *d)
>> +{
>> +    MarinUartState *s = MARIN_UART(d);
>> +    int i;
>> +
>> +    for (i = 0; i < R_MAX; i++) {
>> +        s->regs[i] = 0;
>
> Can you just reset your TX_READY bit to 1? This would cleanup the
> exceptional case i mentioned above, and anyone introspecting your
> device with gdb will see the true and correct value in s->regs.

I'll set it to '1' for the gdb reason. 

>> +    }
>> +}
>> +
>> +static int marin_uart_init(SysBusDevice *dev)
>
> Use of the SysBusDevice::init function is depracted, Please use the
> device::realise or object::init functions instead. Check Antony
> Pavlovs Digic UART (on list and in late stages of review) for the most
> modern example of a QEMU UART.

Ok.


>
>> +{
>> +    MarinUartState *s = MARIN_UART(dev);
>> +
>> +    sysbus_init_irq(dev, &s->irq);
>> +
>> +    memory_region_init_io(&s->regs_region, OBJECT(s), &uart_mmio_ops, s,
>> +            TYPE_MARIN_UART, R_MAX * 4);
>> +    sysbus_init_mmio(dev, &s->regs_region);
>> +
>> +    s->chr = qemu_char_get_next_serial();
>> +    if (s->chr) {
>> +        qemu_chr_add_handlers(s->chr, uart_can_rx, uart_rx, uart_event, s);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static const VMStateDescription vmstate_marin_uart = {
>> +    .name = TYPE_MARIN_UART,
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .minimum_version_id_old = 1,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_UINT16_ARRAY(regs, MarinUartState, R_MAX),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +static void marin_uart_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>
> This will go away when you convert the init fn.
>
>> +
>> +    k->init = marin_uart_init;
>> +    dc->reset = marin_uart_reset;
>> +    dc->vmsd = &vmstate_marin_uart;
>> +}
>> +
>> +static const TypeInfo marin_uart_info = {
>> +    .name          = TYPE_MARIN_UART,
>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>> +    .instance_size = sizeof(MarinUartState),
>> +    .class_init    = marin_uart_class_init,
>> +};
>> +
>> +static void marin_uart_register_types(void)
>> +{
>> +    type_register_static(&marin_uart_info);
>> +}
>> +
>> +type_init(marin_uart_register_types)
>> diff --git a/hw/moxie/Makefile.objs b/hw/moxie/Makefile.objs
>> index bfc9001..4fa3b30 100644
>> --- a/hw/moxie/Makefile.objs
>> +++ b/hw/moxie/Makefile.objs
>> @@ -1,2 +1,2 @@
>>  # moxie boards
>> -obj-y += moxiesim.o
>> +obj-y += moxiesim.o marin.o
>> diff --git a/hw/moxie/marin.c b/hw/moxie/marin.c
>> new file mode 100644
>> index 0000000..0a998e4
>> --- /dev/null
>> +++ b/hw/moxie/marin.c
>> @@ -0,0 +1,167 @@
>> +/*
>> + * QEMU/marin SoC emulation
>> + *
>> + * Emulates the FPGA-hosted Marin SoC
>> + *
>> + * Copyright (c) 2013 Anthony Green
>> + *
>> + * Permission is hereby granted, free of charge, to any person
>> obtaining a copy
>> + * of this software and associated documentation files (the
>> "Software"), to deal
>> + * in the Software without restriction, including without
>> limitation the rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> ARISING FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>> + */
>> +#include "hw/sysbus.h"
>> +#include "hw/hw.h"
>> +#include "hw/boards.h"
>> +#include "hw/loader.h"
>> +#include "exec/address-spaces.h"
>> +
>> +typedef struct {
>> +    uint64_t ram_size;
>> +    const char *kernel_filename;
>> +    const char *kernel_cmdline;
>> +    const char *initrd_filename;
>> +} LoaderParams;
>> +
>> +static void load_kernel(MoxieCPU *cpu, LoaderParams *loader_params)
>> +{
>> +    uint64_t entry, kernel_low, kernel_high;
>> +    long kernel_size;
>> +    long initrd_size;
>> +    ram_addr_t initrd_offset;
>> +
>> +    kernel_size = load_elf(loader_params->kernel_filename,  NULL, NULL,
>> +                           &entry, &kernel_low, &kernel_high, 1,
>> +                           ELF_MACHINE, 0);
>> +
>> +    if (!kernel_size) {
>> +        fprintf(stderr, "qemu: could not load kernel '%s'\n",
>> +                loader_params->kernel_filename);
>
> error_report()

Ok.

>
>> +        exit(1);
>> +    }
>> +
>> +    /* load initrd */
>> +    initrd_size = 0;
>> +    initrd_offset = 0;
>> +    if (loader_params->initrd_filename) {
>> +        initrd_size = get_image_size(loader_params->initrd_filename);
>> +        if (initrd_size > 0) {
>> +            initrd_offset = (kernel_high + ~TARGET_PAGE_MASK)
>> +              & TARGET_PAGE_MASK;
>> +            if (initrd_offset + initrd_size > loader_params->ram_size) {
>> +                fprintf(stderr,
>> +                        "qemu: memory too small for initial ram disk '%s'\n",
>> +                        loader_params->initrd_filename);
>> +                exit(1);
>> +            }
>> +            initrd_size = load_image_targphys(loader_params->initrd_filename,
>> +                                              initrd_offset,
>> +                                              ram_size);
>> +        }
>> +        if (initrd_size == (target_ulong)-1) {
>> +            fprintf(stderr, "qemu: could not load initial ram disk '%s'\n",
>> +                    loader_params->initrd_filename);
>> +            exit(1);
>> +        }
>> +    }
>> +}
>
> You should consider pulling your bootloader our into a seperate file
> (and patch). Does Moxie define a specific linux boot protocol? Check
> hw/arm/boot.c or hw/arm/microblaze.c for examples of modular
> bootloaders.

No, not yet.  I have an oldish uClinux kernel port that used device
trees, but it will be a while before I get back to that - there's so
much more to do first!

>> +
>> +static void main_cpu_reset(void *opaque)
>> +{
>> +    MoxieCPU *cpu = opaque;
>> +
>> +    cpu_reset(CPU(cpu));
>> +}
>> +
>> +static inline DeviceState *marin_uart_create(hwaddr base,
>> +        qemu_irq irq)
>> +{
>> +    DeviceState *dev;
>> +
>> +    dev = qdev_create(NULL, "marin-uart");
>> +    qdev_init_nofail(dev);
>> +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
>> +    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq);
>> +
>> +    return dev;
>> +}
>
> This is an old style qdev init function.

Any good pointers for a new style init function?

>
>> +
>> +static void marin_init(QEMUMachineInitArgs *args)
>> +{
>> +    MoxieCPU *cpu = NULL;
>> +    ram_addr_t ram_size = args->ram_size;
>> +    const char *cpu_model = args->cpu_model;
>> +    const char *kernel_filename = args->kernel_filename;
>> +    const char *kernel_cmdline = args->kernel_cmdline;
>> +    const char *initrd_filename = args->initrd_filename;
>> +    CPUMoxieState *env;
>> +    MemoryRegion *address_space_mem = get_system_memory();
>> +    MemoryRegion *ocram = g_new(MemoryRegion, 1);
>> +    MemoryRegion *ram = g_new(MemoryRegion, 1);
>> +    MemoryRegion *rom = g_new(MemoryRegion, 1);
>> +    hwaddr ram_base = 0x30000000;
>> +    LoaderParams loader_params;
>> +
>> +    /* Init CPUs. */
>> +    if (cpu_model == NULL) {
>> +        cpu_model = "MoxieLite-moxie-cpu";
>> +    }
>> +    cpu = cpu_moxie_init(cpu_model);
>> +    if (!cpu) {
>> +        fprintf(stderr, "Unable to find CPU definition\n");
>
> error_report()
>
>> +        exit(1);
>> +    }
>> +    env = &cpu->env;
>> +
>> +    qemu_register_reset(main_cpu_reset, cpu);
>> +
>> +    /* Allocate RAM. */
>> +    memory_region_init_ram(ocram, NULL, "marin-onchip.ram", 0x1000*4);
>> +    vmstate_register_ram_global(ocram);
>> +    memory_region_add_subregion(address_space_mem, 0x10000000, ocram);
>> +
>> +    memory_region_init_ram(ram, NULL, "marin-external.ram", ram_size);
>> +    vmstate_register_ram_global(ram);
>> +    memory_region_add_subregion(address_space_mem, ram_base, ram);
>> +
>> +    memory_region_init_ram(rom, NULL, "moxie.rom", 128*0x1000);
>> +    vmstate_register_ram_global(rom);
>> +    memory_region_add_subregion(get_system_memory(), 0x1000, rom);
>> +
>> +    if (kernel_filename) {
>> +        loader_params.ram_size = ram_size;
>> +        loader_params.kernel_filename = kernel_filename;
>> +        loader_params.kernel_cmdline = kernel_cmdline;
>> +        loader_params.initrd_filename = initrd_filename;
>> +        load_kernel(cpu, &loader_params);
>> +    }
>> +
>> +    marin_uart_create(0xF0000008, env->irq[4]);
>> +}
>> +
>> +static QEMUMachine marin_machine = {
>> +    .name = "marin",
>> +    .desc = "Marin SoC",
>
> So SoCs should generally be implemented on two levels. There is the
> SoC device, which contains the devices that are on the SoC chip, then
> the board level instantiates the SoC. This looks like a flat
> board-and-SoC in one (on board level). Your deisgn is trivial so far
> (and good for a first series), but long term what is the organsation?
> Is this going towards a particular board emulation? Have a look at
> Liguangs Allwinner series (and some of the early review comments) for
> a discussion on this topic:
>
> http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg03940.html
>
> As a starting point, can you tell us what is and isn't hosted on the
> FPGA in this board model? That might be the best way to split this.

The Marin SoC currently runs on two boards: the Nexys3 (Xilinx) and DE-2
(Altera).  They are pretty much identical from the software side of
things.  Marin currently provides the UART, PIC, 7 segment display and
timer devices, as well as various memory controllers.  There's no useful
distinction between SoC and board at this time.  I'd like to keep it
simple as per my patch rather than try to factor them out prematurely.

Thanks,

Anthony Green


>
> Regards,
> Peter
>
>> +    .init = marin_init,
>> +    .is_default = 1,
>> +};
>> +
>> +static void moxie_machine_init(void)
>> +{
>> +    qemu_register_machine(&marin_machine);
>> +}
>> +
>> +machine_init(moxie_machine_init)
>> --
>> 1.8.3.1
>>
>>
Andreas Färber Dec. 15, 2013, 7:31 p.m. UTC | #4
Am 15.12.2013 13:48, schrieb Anthony Green:
> Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes:
>> On Sun, Dec 15, 2013 at 1:59 PM, Anthony Green <green@moxielogic.com> wrote:
>>> diff --git a/hw/moxie/marin.c b/hw/moxie/marin.c
>>> new file mode 100644
>>> index 0000000..0a998e4
>>> --- /dev/null
>>> +++ b/hw/moxie/marin.c
[...]
>>> +static inline DeviceState *marin_uart_create(hwaddr base,
>>> +        qemu_irq irq)
>>> +{
>>> +    DeviceState *dev;
>>> +
>>> +    dev = qdev_create(NULL, "marin-uart");
>>> +    qdev_init_nofail(dev);
>>> +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
>>> +    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq);
>>> +
>>> +    return dev;
>>> +}
>>
>> This is an old style qdev init function.
> 
> Any good pointers for a new style init function?

What Peter C. probably meant here was it is a pre-qdev init function.

Unless there is a strong need for reuse, just inline it into the machine
init function to show that it is using pure QOM constructs.

As for new-style init functions, since you are not setting any
properties, chances are good that you can do in an instance_init
function (rather than realize function) what you did in a deprecated
SysBus init function IIRC.

Regards,
Andreas
Andreas Färber Dec. 15, 2013, 8:05 p.m. UTC | #5
Hi,

Am 15.12.2013 13:48, schrieb Anthony Green:
> Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes:
>> On Sun, Dec 15, 2013 at 1:59 PM, Anthony Green <green@moxielogic.com> wrote:
>>> diff --git a/hw/moxie/marin.c b/hw/moxie/marin.c
>>> new file mode 100644
>>> index 0000000..0a998e4
>>> --- /dev/null
>>> +++ b/hw/moxie/marin.c
>>> @@ -0,0 +1,167 @@
>>> +/*
>>> + * QEMU/marin SoC emulation
>>> + *
>>> + * Emulates the FPGA-hosted Marin SoC
[...]
>>> +static QEMUMachine marin_machine = {
>>> +    .name = "marin",
>>> +    .desc = "Marin SoC",
>>
>> So SoCs should generally be implemented on two levels. There is the
>> SoC device, which contains the devices that are on the SoC chip, then
>> the board level instantiates the SoC. This looks like a flat
>> board-and-SoC in one (on board level). Your deisgn is trivial so far
>> (and good for a first series), but long term what is the organsation?
>> Is this going towards a particular board emulation? Have a look at
>> Liguangs Allwinner series (and some of the early review comments) for
>> a discussion on this topic:
>>
>> http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg03940.html
>>
>> As a starting point, can you tell us what is and isn't hosted on the
>> FPGA in this board model? That might be the best way to split this.
> 
> The Marin SoC currently runs on two boards: the Nexys3 (Xilinx) and DE-2
> (Altera).  They are pretty much identical from the software side of
> things.  Marin currently provides the UART, PIC, 7 segment display and
> timer devices, as well as various memory controllers.  There's no useful
> distinction between SoC and board at this time.  I'd like to keep it
> simple as per my patch rather than try to factor them out prematurely.

I thought I've seen a number of odd embedded systems already, but I'm
having trouble understanding your combination of SoC and FPGA: Xilinx
and Altera both have SoCs combining a Cortex-A9 with an FPGA. But your
reference to Xilinx and Altera boards rather sounds as if Moxie is used
as a soft-core processor on the FPGA? In that case the term "SoC" would
be really confusing to me... Can you clarify or aid with some links?

Regards,
Andreas
Anthony Green Dec. 15, 2013, 9:02 p.m. UTC | #6
Andreas Färber <afaerber@suse.de> writes:

>> The Marin SoC currently runs on two boards: the Nexys3 (Xilinx) and DE-2
>> (Altera).  They are pretty much identical from the software side of
>> things.  Marin currently provides the UART, PIC, 7 segment display and
>> timer devices, as well as various memory controllers.  There's no useful
>> distinction between SoC and board at this time.  I'd like to keep it
>> simple as per my patch rather than try to factor them out prematurely.
>
> I thought I've seen a number of odd embedded systems already, but I'm
> having trouble understanding your combination of SoC and FPGA: Xilinx
> and Altera both have SoCs combining a Cortex-A9 with an FPGA. But your
> reference to Xilinx and Altera boards rather sounds as if Moxie is used
> as a soft-core processor on the FPGA? In that case the term "SoC" would
> be really confusing to me... Can you clarify or aid with some links?

Moxie is an architecture.  MoxieLite is one implementation of that
architecture (non-pipelined, resource-light).  Marin is a kind of SoC -
the combination of the MoxieLite core along with various peripherals and
controllers.

http://github.com/atgreen/moxie-cores

It is similar, in a way, to the Miklymist SoC, which uses an LM32
soft-core (and is supported by qemu).

The Xilinx and Altera parts with Cortex-A9 cores are similar, except
that that Cortex-A9 is an on-chip ASIC, instead of a soft SoC.

AG
Peter Crosthwaite Dec. 16, 2013, 12:33 a.m. UTC | #7
On Mon, Dec 16, 2013 at 7:02 AM, Anthony Green <green@moxielogic.com> wrote:
> Andreas Färber <afaerber@suse.de> writes:
>
>>> The Marin SoC currently runs on two boards: the Nexys3 (Xilinx) and DE-2
>>> (Altera).  They are pretty much identical from the software side of
>>> things.

Ok but do they have the same peripheral set? We had this discussion
with Allwinner where it was decided that even though the initial
peripheral set was generic enough for software to boot, going forward
it would not be true to the actual hardware. Sooner or later as you
add peripherals the difference between Altera and Xilinx board
definitions will stop your all-in-one approach wont it?

>>>  Marin currently provides the UART, PIC, 7 segment display and
>>> timer devices, as well as various memory controllers.  There's no useful
>>> distinction between SoC and board at this time.

I see the definition of Marin as a proper QEMU SoC quite useful here.
Marin is then instantiated on Nexys3 or DE2 on the board level.

Ideally we have a nexys3 or DE2 board definition that can instantiate
a number of soft SoCs (e.g. swap out Microblaze or Marin even), but
i'd leave that as follow up as its kind of hard to pull off.

 I'd like to keep it
>>> simple as per my patch rather than try to factor them out prematurely.
>>
>> I thought I've seen a number of odd embedded systems already, but I'm
>> having trouble understanding your combination of SoC and FPGA: Xilinx
>> and Altera both have SoCs combining a Cortex-A9 with an FPGA. But your
>> reference to Xilinx and Altera boards rather sounds as if Moxie is used
>> as a soft-core processor on the FPGA? In that case the term "SoC" would
>> be really confusing to me... Can you clarify or aid with some links?

So we need to clarify terminology. SoC and FPGA are not mutually
exclusive, and not just talking about Zynq and Altera hard ARM
FPGA+SoC. If you just have a raw FPGA and configure it with a
processor and peripherals then isn't that now a SoC in its own right?

I have seen the SoC term used for years when referring to soft
Microblaze systems so I think Anthonys terminology is good. You don't
need hard silicon to be a SoC.

Google this (exluding Zynq from the search helps a lot):

microblaze SoC -zynq

>
> Moxie is an architecture.  MoxieLite is one implementation of that
> architecture (non-pipelined, resource-light).  Marin is a kind of SoC -
> the combination of the MoxieLite core along with various peripherals and
> controllers.
>
> http://github.com/atgreen/moxie-cores
>

So you just synthesize this Verilog into a FPGA bistream right?

Have a Nexys bitstream and linux boot instructions handy?

> It is similar, in a way, to the Miklymist SoC, which uses an LM32
> soft-core (and is supported by qemu).
>

And the PetaLogix+Microblaze stuff.

> The Xilinx and Altera parts with Cortex-A9 cores are similar, except
> that that Cortex-A9 is an on-chip ASIC, instead of a soft SoC.
>

I don't see the need for QEMU to make this hard/soft distinction. It
shouldn't have to worry too much about underlying silicon
implementation.

Regards,
Peter

> AG
>
Anthony Green Dec. 16, 2013, 3:35 a.m. UTC | #8
Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes:

> On Mon, Dec 16, 2013 at 7:02 AM, Anthony Green <green@moxielogic.com> wrote:
>> Andreas Färber <afaerber@suse.de> writes:
>>
>>>> The Marin SoC currently runs on two boards: the Nexys3 (Xilinx) and DE-2
>>>> (Altera).  They are pretty much identical from the software side of
>>>> things.
>
> Ok but do they have the same peripheral set? We had this discussion
> with Allwinner where it was decided that even though the initial
> peripheral set was generic enough for software to boot, going forward
> it would not be true to the actual hardware. Sooner or later as you
> add peripherals the difference between Altera and Xilinx board
> definitions will stop your all-in-one approach wont it?

The peripherals are mostly similar.  All the important ones are the
same.  I will probably ignore where they differ (different numbers of
programmable buttons, for instance).

>>>>  Marin currently provides the UART, PIC, 7 segment display and
>>>> timer devices, as well as various memory controllers.  There's no useful
>>>> distinction between SoC and board at this time.
>
> I see the definition of Marin as a proper QEMU SoC quite useful here.
> Marin is then instantiated on Nexys3 or DE2 on the board level.
>
> Ideally we have a nexys3 or DE2 board definition that can instantiate
> a number of soft SoCs (e.g. swap out Microblaze or Marin even), but
> i'd leave that as follow up as its kind of hard to pull off.

I'm not sure it even makes sense.  The common elements between
Microblaze, Marin and Milkymist (for instance) are things like "has 16MB
of pseudo static RAM".  These are thin things to hang an abstraction on.

>  I'd like to keep it
>>>> simple as per my patch rather than try to factor them out prematurely.
>>>
>>> I thought I've seen a number of odd embedded systems already, but I'm
>>> having trouble understanding your combination of SoC and FPGA: Xilinx
>>> and Altera both have SoCs combining a Cortex-A9 with an FPGA. But your
>>> reference to Xilinx and Altera boards rather sounds as if Moxie is used
>>> as a soft-core processor on the FPGA? In that case the term "SoC" would
>>> be really confusing to me... Can you clarify or aid with some links?
>
> So we need to clarify terminology. SoC and FPGA are not mutually
> exclusive, and not just talking about Zynq and Altera hard ARM
> FPGA+SoC. If you just have a raw FPGA and configure it with a
> processor and peripherals then isn't that now a SoC in its own right?
>
> I have seen the SoC term used for years when referring to soft
> Microblaze systems so I think Anthonys terminology is good. You don't
> need hard silicon to be a SoC.
>
> Google this (exluding Zynq from the search helps a lot):
>
> microblaze SoC -zynq
>
>>
>> Moxie is an architecture.  MoxieLite is one implementation of that
>> architecture (non-pipelined, resource-light).  Marin is a kind of SoC -
>> the combination of the MoxieLite core along with various peripherals and
>> controllers.
>>
>> http://github.com/atgreen/moxie-cores
>>
>
> So you just synthesize this Verilog into a FPGA bistream right?

Correct.  Marin is actually a mix of Verilog and VHDL.

> Have a Nexys bitstream and linux boot instructions handy?

1. Nexys3 bitstream:
    https://dl.dropboxusercontent.com/u/3786210/marin.bit.gz 
   This will bring you to the on-chip srecord-loader/gdbstub.  The tools
   directory in the moxie-cores github project has scripts to build a
   toolchain so you can start hacking.

2. Linux boot instructions: step 1. port linux.  you can guess the rest.
   Actually, I have a very early uClinux kernel port that boots to
   userland here: https://github.com/atgreen/linux-2.6-moxie . It will
   die once it hits userland thanks to vfork() madness that will
   actually require an architectural change to fix.

>> It is similar, in a way, to the Miklymist SoC, which uses an LM32
>> soft-core (and is supported by qemu).
>>
>
> And the PetaLogix+Microblaze stuff.
>
>> The Xilinx and Altera parts with Cortex-A9 cores are similar, except
>> that that Cortex-A9 is an on-chip ASIC, instead of a soft SoC.
>>
>
> I don't see the need for QEMU to make this hard/soft distinction. It
> shouldn't have to worry too much about underlying silicon
> implementation.
>
> Regards,
> Peter
>
>> AG
>>
diff mbox

Patch

diff --git a/default-configs/moxie-softmmu.mak b/default-configs/moxie-softmmu.mak
index 1a95476..65a21de 100644
--- a/default-configs/moxie-softmmu.mak
+++ b/default-configs/moxie-softmmu.mak
@@ -1,5 +1,6 @@ 
 # Default configuration for moxie-softmmu
 
 CONFIG_MC146818RTC=y
+CONFIG_MOXIE=y
 CONFIG_SERIAL=y
 CONFIG_VGA=y
diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
index cbd6a00..48bc5d0 100644
--- a/hw/char/Makefile.objs
+++ b/hw/char/Makefile.objs
@@ -14,6 +14,7 @@  obj-$(CONFIG_COLDFIRE) += mcf_uart.o
 obj-$(CONFIG_OMAP) += omap_uart.o
 obj-$(CONFIG_SH4) += sh_serial.o
 obj-$(CONFIG_PSERIES) += spapr_vty.o
+obj-$(CONFIG_MOXIE) += marin-uart.o
 
 common-obj-$(CONFIG_ETRAXFS) += etraxfs_ser.o
 common-obj-$(CONFIG_ISA_DEBUG) += debugcon.o
diff --git a/hw/char/marin-uart.c b/hw/char/marin-uart.c
new file mode 100644
index 0000000..f0d46d4
--- /dev/null
+++ b/hw/char/marin-uart.c
@@ -0,0 +1,198 @@ 
+/*
+ *  QEMU model of the Marin UART.
+ *
+ *  Copyright (c) 2013 Anthony Green <green@moxielogic.com>
+ *
+ * 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 "hw/hw.h"
+#include "hw/sysbus.h"
+#include "trace.h"
+#include "sysemu/char.h"
+#include "qemu/error-report.h"
+
+enum {
+    R_RXREADY = 0,
+    R_TXREADY,
+    R_RXBYTE,
+    R_TXBYTE,
+    R_MAX
+};
+
+#define TYPE_MARIN_UART "marin-uart"
+#define MARIN_UART(obj) OBJECT_CHECK(MarinUartState, (obj), TYPE_MARIN_UART)
+
+struct MarinUartState {
+    SysBusDevice busdev;
+    MemoryRegion regs_region;
+    CharDriverState *chr;
+    qemu_irq irq;
+
+    uint16_t regs[R_MAX];
+};
+typedef struct MarinUartState MarinUartState;
+
+static void uart_update_irq(MarinUartState *s)
+{
+}
+
+static uint64_t uart_read(void *opaque, hwaddr addr,
+                          unsigned size)
+{
+    MarinUartState *s = opaque;
+    uint32_t r = 0;
+
+    addr >>= 1;
+    switch (addr) {
+    case R_RXREADY:
+        r = s->regs[R_RXREADY];
+        break;
+    case R_TXREADY:
+        r = 1;
+        break;
+    case R_TXBYTE:
+        r = s->regs[R_TXBYTE];
+        break;
+    case R_RXBYTE:
+        r = s->regs[R_RXBYTE];
+        s->regs[R_RXREADY] = 0;
+        qemu_chr_accept_input(s->chr);
+        break;
+    default:
+        error_report("marin_uart: read access to unknown register 0x"
+                TARGET_FMT_plx, addr << 1);
+        break;
+    }
+
+    return r;
+}
+
+static void uart_write(void *opaque, hwaddr addr, uint64_t value,
+                       unsigned size)
+{
+    MarinUartState *s = opaque;
+    unsigned char ch = value;
+
+    addr >>= 1;
+    switch (addr) {
+    case R_TXBYTE:
+        if (s->chr) {
+            qemu_chr_fe_write(s->chr, &ch, 1);
+        }
+        break;
+
+    default:
+        error_report("marin_uart: write access to unknown register 0x"
+                TARGET_FMT_plx, addr << 1);
+        break;
+    }
+
+    uart_update_irq(s);
+}
+
+static const MemoryRegionOps uart_mmio_ops = {
+    .read = uart_read,
+    .write = uart_write,
+    .valid = {
+        .min_access_size = 2,
+        .max_access_size = 2,
+    },
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void uart_rx(void *opaque, const uint8_t *buf, int size)
+{
+    MarinUartState *s = opaque;
+
+    s->regs[R_RXBYTE] = *buf;
+    s->regs[R_RXREADY] = 1;
+
+    uart_update_irq(s);
+}
+
+static int uart_can_rx(void *opaque)
+{
+    MarinUartState *s = opaque;
+
+    return !(s->regs[R_RXREADY]);
+}
+
+static void uart_event(void *opaque, int event)
+{
+}
+
+static void marin_uart_reset(DeviceState *d)
+{
+    MarinUartState *s = MARIN_UART(d);
+    int i;
+
+    for (i = 0; i < R_MAX; i++) {
+        s->regs[i] = 0;
+    }
+}
+
+static int marin_uart_init(SysBusDevice *dev)
+{
+    MarinUartState *s = MARIN_UART(dev);
+
+    sysbus_init_irq(dev, &s->irq);
+
+    memory_region_init_io(&s->regs_region, OBJECT(s), &uart_mmio_ops, s,
+            TYPE_MARIN_UART, R_MAX * 4);
+    sysbus_init_mmio(dev, &s->regs_region);
+
+    s->chr = qemu_char_get_next_serial();
+    if (s->chr) {
+        qemu_chr_add_handlers(s->chr, uart_can_rx, uart_rx, uart_event, s);
+    }
+
+    return 0;
+}
+
+static const VMStateDescription vmstate_marin_uart = {
+    .name = TYPE_MARIN_UART,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT16_ARRAY(regs, MarinUartState, R_MAX),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void marin_uart_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+
+    k->init = marin_uart_init;
+    dc->reset = marin_uart_reset;
+    dc->vmsd = &vmstate_marin_uart;
+}
+
+static const TypeInfo marin_uart_info = {
+    .name          = TYPE_MARIN_UART,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(MarinUartState),
+    .class_init    = marin_uart_class_init,
+};
+
+static void marin_uart_register_types(void)
+{
+    type_register_static(&marin_uart_info);
+}
+
+type_init(marin_uart_register_types)
diff --git a/hw/moxie/Makefile.objs b/hw/moxie/Makefile.objs
index bfc9001..4fa3b30 100644
--- a/hw/moxie/Makefile.objs
+++ b/hw/moxie/Makefile.objs
@@ -1,2 +1,2 @@ 
 # moxie boards
-obj-y += moxiesim.o
+obj-y += moxiesim.o marin.o
diff --git a/hw/moxie/marin.c b/hw/moxie/marin.c
new file mode 100644
index 0000000..0a998e4
--- /dev/null
+++ b/hw/moxie/marin.c
@@ -0,0 +1,167 @@ 
+/*
+ * QEMU/marin SoC emulation
+ *
+ * Emulates the FPGA-hosted Marin SoC
+ *
+ * Copyright (c) 2013 Anthony Green
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "hw/sysbus.h"
+#include "hw/hw.h"
+#include "hw/boards.h"
+#include "hw/loader.h"
+#include "exec/address-spaces.h"
+
+typedef struct {
+    uint64_t ram_size;
+    const char *kernel_filename;
+    const char *kernel_cmdline;
+    const char *initrd_filename;
+} LoaderParams;
+
+static void load_kernel(MoxieCPU *cpu, LoaderParams *loader_params)
+{
+    uint64_t entry, kernel_low, kernel_high;
+    long kernel_size;
+    long initrd_size;
+    ram_addr_t initrd_offset;
+
+    kernel_size = load_elf(loader_params->kernel_filename,  NULL, NULL,
+                           &entry, &kernel_low, &kernel_high, 1,
+                           ELF_MACHINE, 0);
+
+    if (!kernel_size) {
+        fprintf(stderr, "qemu: could not load kernel '%s'\n",
+                loader_params->kernel_filename);
+        exit(1);
+    }
+
+    /* load initrd */
+    initrd_size = 0;
+    initrd_offset = 0;
+    if (loader_params->initrd_filename) {
+        initrd_size = get_image_size(loader_params->initrd_filename);
+        if (initrd_size > 0) {
+            initrd_offset = (kernel_high + ~TARGET_PAGE_MASK)
+              & TARGET_PAGE_MASK;
+            if (initrd_offset + initrd_size > loader_params->ram_size) {
+                fprintf(stderr,
+                        "qemu: memory too small for initial ram disk '%s'\n",
+                        loader_params->initrd_filename);
+                exit(1);
+            }
+            initrd_size = load_image_targphys(loader_params->initrd_filename,
+                                              initrd_offset,
+                                              ram_size);
+        }
+        if (initrd_size == (target_ulong)-1) {
+            fprintf(stderr, "qemu: could not load initial ram disk '%s'\n",
+                    loader_params->initrd_filename);
+            exit(1);
+        }
+    }
+}
+
+static void main_cpu_reset(void *opaque)
+{
+    MoxieCPU *cpu = opaque;
+
+    cpu_reset(CPU(cpu));
+}
+
+static inline DeviceState *marin_uart_create(hwaddr base,
+        qemu_irq irq)
+{
+    DeviceState *dev;
+
+    dev = qdev_create(NULL, "marin-uart");
+    qdev_init_nofail(dev);
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
+    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq);
+
+    return dev;
+}
+
+static void marin_init(QEMUMachineInitArgs *args)
+{
+    MoxieCPU *cpu = NULL;
+    ram_addr_t ram_size = args->ram_size;
+    const char *cpu_model = args->cpu_model;
+    const char *kernel_filename = args->kernel_filename;
+    const char *kernel_cmdline = args->kernel_cmdline;
+    const char *initrd_filename = args->initrd_filename;
+    CPUMoxieState *env;
+    MemoryRegion *address_space_mem = get_system_memory();
+    MemoryRegion *ocram = g_new(MemoryRegion, 1);
+    MemoryRegion *ram = g_new(MemoryRegion, 1);
+    MemoryRegion *rom = g_new(MemoryRegion, 1);
+    hwaddr ram_base = 0x30000000;
+    LoaderParams loader_params;
+
+    /* Init CPUs. */
+    if (cpu_model == NULL) {
+        cpu_model = "MoxieLite-moxie-cpu";
+    }
+    cpu = cpu_moxie_init(cpu_model);
+    if (!cpu) {
+        fprintf(stderr, "Unable to find CPU definition\n");
+        exit(1);
+    }
+    env = &cpu->env;
+
+    qemu_register_reset(main_cpu_reset, cpu);
+
+    /* Allocate RAM. */
+    memory_region_init_ram(ocram, NULL, "marin-onchip.ram", 0x1000*4);
+    vmstate_register_ram_global(ocram);
+    memory_region_add_subregion(address_space_mem, 0x10000000, ocram);
+
+    memory_region_init_ram(ram, NULL, "marin-external.ram", ram_size);
+    vmstate_register_ram_global(ram);
+    memory_region_add_subregion(address_space_mem, ram_base, ram);
+
+    memory_region_init_ram(rom, NULL, "moxie.rom", 128*0x1000);
+    vmstate_register_ram_global(rom);
+    memory_region_add_subregion(get_system_memory(), 0x1000, rom);
+
+    if (kernel_filename) {
+        loader_params.ram_size = ram_size;
+        loader_params.kernel_filename = kernel_filename;
+        loader_params.kernel_cmdline = kernel_cmdline;
+        loader_params.initrd_filename = initrd_filename;
+        load_kernel(cpu, &loader_params);
+    }
+
+    marin_uart_create(0xF0000008, env->irq[4]);
+}
+
+static QEMUMachine marin_machine = {
+    .name = "marin",
+    .desc = "Marin SoC",
+    .init = marin_init,
+    .is_default = 1,
+};
+
+static void moxie_machine_init(void)
+{
+    qemu_register_machine(&marin_machine);
+}
+
+machine_init(moxie_machine_init)