On Thu, Nov 28, 2019 at 12:26 AM Philippe Mathieu-Daudé <
address@hidden> wrote:
Hi Michael,
On 11/27/19 6:52 PM, Michael Rolnik wrote:
> This includes:
> - CPU data structures
> - object model classes and functions
> - migration functions
> - GDB hooks
>
> Co-developed-by: Michael Rolnik <address@hidden>
> Co-developed-by: Sarah Harris <address@hidden>
> Signed-off-by: Michael Rolnik <address@hidden>
> Signed-off-by: Sarah Harris <address@hidden>
> Signed-off-by: Michael Rolnik <address@hidden>
> Acked-by: Igor Mammedov <address@hidden>
> Tested-by: Philippe Mathieu-Daudé <address@hidden>
> ---
> target/avr/cpu-param.h | 37 +++
> target/avr/cpu-qom.h | 54 ++++
> target/avr/cpu.h | 253 ++++++++++++++++++
> target/avr/cpu.c | 576 +++++++++++++++++++++++++++++++++++++++++
> target/avr/gdbstub.c | 85 ++++++
> target/avr/machine.c | 121 +++++++++
> gdb-xml/avr-cpu.xml | 49 ++++
> 7 files changed, 1175 insertions(+)
> create mode 100644 target/avr/cpu-param.h
> create mode 100644 target/avr/cpu-qom.h
> create mode 100644 target/avr/cpu.h
> create mode 100644 target/avr/cpu.c
> create mode 100644 target/avr/gdbstub.c
> create mode 100644 target/avr/machine.c
> create mode 100644 gdb-xml/avr-cpu.xml
>
> diff --git a/target/avr/cpu-param.h b/target/avr/cpu-param.h
> new file mode 100644
> index 0000000000..ccd1ea3429
> --- /dev/null
> +++ b/target/avr/cpu-param.h
> @@ -0,0 +1,37 @@
> +/*
> + * QEMU AVR CPU
> + *
> + * Copyright (c) 2019 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>
> + */
> +
> +#ifndef AVR_CPU_PARAM_H
> +#define AVR_CPU_PARAM_H 1
> +
> +#define TARGET_LONG_BITS 32
> +/*
> + * TARGET_PAGE_BITS cannot be more than 8 bits because
> + * 1. all IO registers occupy [0x0000 .. 0x00ff] address range, and they
> + * should be implemented as a device and not memory
> + * 2. SRAM starts at the address 0x0100
> + */
> +#define TARGET_PAGE_BITS 8
> +#define TARGET_PHYS_ADDR_SPACE_BITS 24
> +#define TARGET_VIRT_ADDR_SPACE_BITS 24
> +#define NB_MMU_MODES 2
> +
> +
> +#endif
> diff --git a/target/avr/cpu-qom.h b/target/avr/cpu-qom.h
> new file mode 100644
> index 0000000000..e28b58c897
> --- /dev/null
> +++ b/target/avr/cpu-qom.h
> @@ -0,0 +1,54 @@
> +/*
> + * QEMU AVR CPU
> + *
> + * Copyright (c) 2019 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>
> + */
> +
> +#ifndef QEMU_AVR_QOM_H
> +#define QEMU_AVR_QOM_H
> +
> +#include "hw/core/cpu.h"
> +
> +#define TYPE_AVR_CPU "avr-cpu"
> +
> +#define AVR_CPU_CLASS(klass) \
> + OBJECT_CLASS_CHECK(AVRCPUClass, (klass), TYPE_AVR_CPU)
> +#define AVR_CPU(obj) \
> + OBJECT_CHECK(AVRCPU, (obj), TYPE_AVR_CPU)
> +#define AVR_CPU_GET_CLASS(obj) \
> + OBJECT_GET_CLASS(AVRCPUClass, (obj), TYPE_AVR_CPU)
> +
> +/**
> + * AVRCPUClass:
> + * @parent_realize: The parent class' realize handler.
> + * @parent_reset: The parent class' reset handler.
> + * @vr: Version Register value.
> + *
> + * A AVR CPU model.
> + */
> +typedef struct AVRCPUClass {
> + /*< private >*/
> + CPUClass parent_class;
> + /*< public >*/
> + DeviceRealize parent_realize;
> + void (*parent_reset)(CPUState *cpu);
> +} AVRCPUClass;
> +
> +typedef struct AVRCPU AVRCPU;
> +
> +
> +#endif /* !defined (QEMU_AVR_CPU_QOM_H) */
> diff --git a/target/avr/cpu.h b/target/avr/cpu.h
> new file mode 100644
> index 0000000000..9ea5260165
> --- /dev/null
> +++ b/target/avr/cpu.h
> @@ -0,0 +1,253 @@
> +/*
> + * QEMU AVR CPU
> + *
> + * Copyright (c) 2019 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>
> + */
> +
> +#ifndef QEMU_AVR_CPU_H
> +#define QEMU_AVR_CPU_H
> +
> +#include "cpu-qom.h"
> +#include "exec/cpu-defs.h"
> +
> +#define TCG_GUEST_DEFAULT_MO 0
> +
Please add:
#define AVR_CPU_TYPE_SUFFIX "-" TYPE_AVR_CPU
#define AVR_CPU_TYPE_NAME(name) (name AVR_CPU_TYPE_SUFFIX)
> +#define CPU_RESOLVING_TYPE TYPE_AVR_CPU
> +
> +/*
> + * AVR has two memory spaces, data & code.
> + * e.g. both have 0 address
> + * ST/LD instructions access data space
> + * LPM/SPM and instruction fetching access code memory space
> + */
> +#define MMU_CODE_IDX 0
> +#define MMU_DATA_IDX 1
> +
> +#define EXCP_RESET 1
> +#define EXCP_INT(n) (EXCP_RESET + (n) + 1)
> +
> +/* Number of CPU registers */
> +#define NUMBER_OF_CPU_REGISTERS 32
> +/* Number of IO registers accessible by ld/st/in/out */
> +#define NUMBER_OF_IO_REGISTERS 64
> +
Is the following block <...
> +/*
> + * Offsets of AVR memory regions in host memory space.
> + *
> + * This is needed because the AVR has separate code and data address
> + * spaces that both have start from zero but have to go somewhere in
> + * host memory.
> + *
> + * It's also useful to know where some things are, like the IO registers.
> + */
> +/* Flash program memory */
> +#define OFFSET_CODE 0x00000000
> +/* CPU registers, IO registers, and SRAM */
> +#define OFFSET_DATA 0x00800000
> +/* CPU registers specifically, these are mapped at the start of data */
> +#define OFFSET_CPU_REGISTERS OFFSET_DATA
> +/*
> + * IO registers, including status register, stack pointer, and memory
> + * mapped peripherals, mapped just after CPU registers
> + */
> +#define OFFSET_IO_REGISTERS (OFFSET_DATA + NUMBER_OF_CPU_REGISTERS)
...> really CPU specific? This doesn't seem used by the CPU (core) code,
but by the MCU devices. Maybe we can extract them into
"target/avr/addrspace.h".
These defines are used in the helper.c file
Can be done later.
[...]
> +static void avr_cpu_realizefn(DeviceState *dev, Error **errp)
> +{
> + CPUState *cs = CPU(dev);
> + AVRCPUClass *mcc = AVR_CPU_GET_CLASS(dev);
> + Error *local_err = NULL;
> +
> + cpu_exec_realizefn(cs, &local_err);
> + if (local_err != NULL) {
> + error_propagate(errp, local_err);
> + return;
> + }
> + qemu_init_vcpu(cs);
> + cpu_reset(cs);
> +
> + mcc->parent_realize(dev, errp);
> +}
> +
> +static void avr_cpu_set_int(void *opaque, int irq, int level)
> +{
> + AVRCPU *cpu = opaque;
> + CPUAVRState *env = &cpu->env;
> + CPUState *cs = CPU(cpu);
> +
> + uint64_t mask = (1ull << irq);
> + if (level) {
> + env->intsrc |= mask;
> + cpu_interrupt(cs, CPU_INTERRUPT_HARD);
> + } else {
> + env->intsrc &= ~mask;
> + if (env->intsrc == 0) {
> + cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
> + }
> + }
> +}
> +
> +static void avr_cpu_initfn(Object *obj)
> +{
> + AVRCPU *cpu = AVR_CPU(obj);
> +
> + cpu_set_cpustate_pointers(cpu);
> +
> +#ifndef CONFIG_USER_ONLY
> + /* Set the number of interrupts supported by the CPU. */
> + qdev_init_gpio_in(DEVICE(cpu), avr_cpu_set_int, 57);
Deeply buried magic number...
I'm trying to understand the following comment in sample.c:
/*
* These IRQ numbers don't match the datasheet because we're
counting from
* zero and not including reset.
*/
RESET is IRQ#1. IIUC qdev_get_gpio_in(cpu, 0) will return IRQ#2?
Then qdev_get_gpio_in(cpu, 55) will return IRQ#57?
This is related to:
#define EXCP_RESET 1
#define EXCP_INT(n) (EXCP_RESET + (n) + 1)
But then you use the hardware numbering:
/* Interrupt numbers used by peripherals */
#define USART_RXC_IRQ 24
#define USART_DRE_IRQ 25
#define USART_TXC_IRQ 26
So for USART3_TX=57, have we qdev_get_gpio_in(cpu, 57) out of bound?
Trying so trigger an abort:
#1 0x00007f2ed58e1895 in abort () at /lib64/libc.so.6
#2 0x00007f2ed58e1769 in _nl_load_domain.cold () at /lib64/libc.so.6
#3 0x00007f2ed58ef566 in annobin_assert.c_end () at /lib64/libc.so.6
#4 0x0000561fbbb37417 in qdev_get_gpio_in_named (dev=0x561fbd8b19d0,
name=0x0, n=57) at hw/core/qdev.c:478
#5 0x0000561fbbb37454 in qdev_get_gpio_in (dev=0x561fbd8b19d0, n=57) at
hw/core/qdev.c:484
[This has to be fixed before merging]
Also, what about other MCUs with more than 57 lines? Can we use some
maximum value instead? Maybe 64?
Else we need to add a property and move that into realize(), because we
won't know the value at init() time.
Please use a #define for this value.
> +#endif
> +}
[...]
Thanks,
Phil.