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

[Philippe Mathieu-Daudé]

Hi Cupertino,

On 9/30/20 10:45 PM, cupertinomiranda@gmail.com wrote:
> From: Cupertino Miranda <cmiranda@synopsys.com>
> 
> Signed-off-by: Cupertino Miranda <cmiranda@synopsys.com>
> ---
...

> diff --git a/target/arc/Makefile.objs b/target/arc/Makefile.objs
> new file mode 100644
> index 0000000000..7b2afd08e4
> --- /dev/null
> +++ b/target/arc/Makefile.objs
> @@ -0,0 +1,34 @@
> +#
> +#  QEMU ARC CPU
> +#
> +#  Copyright (c) 2020
> +#
> +#  This library is free software; you can redistribute it and/or
> +#  modify it under the terms of the GNU Lesser General Public
> +#  License as published by the Free Software Foundation; either
> +#  version 2.1 of the License, or (at your option) any later version.
> +#
> +#  This library is distributed in the hope that it will be useful,
> +#  but WITHOUT ANY WARRANTY; without even the implied warranty of
> +#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +#  Lesser General Public License for more details.
> +#
> +#  You should have received a copy of the GNU Lesser General Public
> +#  License along with this library; if not, see
> +#  <http://www.gnu.org/licenses/lgpl-2.1.html>
> +#
> +
> +obj-y   += translate.o
> +obj-y   += helper.o
> +obj-y   += cpu.o
> +obj-y   += op_helper.o
> +obj-y   += gdbstub.o
> +obj-y   += decoder.o
> +obj-y   += regs.o
> +obj-y   += semfunc.o
> +obj-y   += semfunc-helper.o
> +obj-y   += mmu.o
> +obj-y   += mpu.o
> +obj-y   += timer.o
> +obj-y   += irq.o
> +obj-y   += cache.o

We don't use Makefiles anymore, and you already provides meson.build.

> diff --git a/target/arc/arc-common.h b/target/arc/arc-common.h
> new file mode 100644
> index 0000000000..8013e1d2ed
> --- /dev/null
> +++ b/target/arc/arc-common.h
> @@ -0,0 +1,55 @@
> +/*
> + *  Common header file to be used by cpu and disassembler.
> + *  Copyright (C) 2017 Free Software Foundation, Inc.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with GAS or GDB; see the file COPYING3. If not, write to
> + *  the Free Software Foundation, 51 Franklin Street - Fifth Floor, Boston,
> + *  MA 02110-1301, USA.
> + */
> +
> +#ifndef ARC_COMMON_H
> +#define ARC_COMMON_H
> +
> +#include "qemu/osdep.h"
...

Do not include "qemu/osdep.h" in headers.

> +/*-*-indent-tabs-mode:nil;tab-width:4;indent-line-function:'insert-tab'-*-*/
> +/* vim: set ts=4 sw=4 et: */
> diff --git a/target/arc/cpu-qom.h b/target/arc/cpu-qom.h
> new file mode 100644
> index 0000000000..413b693558
> --- /dev/null
> +++ b/target/arc/cpu-qom.h
> @@ -0,0 +1,53 @@
> +/*
> + * QEMU ARC CPU
> + *
> + * Copyright (c) 2016 Michael Rolnik

???

> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see
> + * <http://www.gnu.org/licenses/lgpl-2.1.html>
> + */
> +
...

> diff --git a/target/arc/cpu.c b/target/arc/cpu.c
> new file mode 100644
> index 0000000000..bbcb371760
> --- /dev/null
> +++ b/target/arc/cpu.c
> @@ -0,0 +1,468 @@
> +/*
> + * QEMU ARC CPU
> + *
> + * Copyright (c) 2020

(c) Synopsys?

> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, 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
> + */
...

> +/*-*-indent-tabs-mode:nil;tab-width:4;indent-line-function:'insert-tab'-*-*/
> +/* vim: set ts=4 sw=4 et: */
> diff --git a/target/arc/cpu.h b/target/arc/cpu.h
> new file mode 100644
> index 0000000000..e8446366e5
> --- /dev/null
> +++ b/target/arc/cpu.h
> @@ -0,0 +1,532 @@
> + /*
> +  * QEMU ARC CPU
> +  *
> +  * Copyright (c) 2020

Again.

> +  *
> +  * 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 CPU_ARC_H
> +#define CPU_ARC_H
> +
...

> +
> +/*
> + * ArcCPU:
> + * @env: #CPUMBState
> + *
> + * An ARC CPU.
> + */
> +struct ARCCPU {
> +  /*< private >*/
> +  CPUState parent_obj;
> +
> +  /*< public >*/
> +
> +  /* ARC Configuration Settings. */
> +  struct {
> +    uint32_t addr_size;
> +    bool     aps_feature;
> +    bool     byte_order;
> +    bool     bitscan_option;
> +    uint32_t br_bc_entries;
> +    uint32_t br_pt_entries;
> +    bool     br_bc_full_tag;
> +    uint8_t  br_rs_entries;
> +    uint32_t br_bc_tag_size;
> +    uint8_t  br_tosq_entries;
> +    uint8_t  br_fb_entries;
> +    bool     code_density;
> +    bool     code_protect;
> +    uint8_t  dccm_mem_cycles;
> +    bool     dccm_posedge;
> +    uint8_t  dccm_mem_bancks;
> +    uint8_t  dc_mem_cycles;
> +    bool     dc_posedge;
> +    bool     dmp_unaligned;
> +    bool     ecc_exception;
> +    uint32_t external_interrupts;
> +    uint8_t  ecc_option;
> +    bool     firq_option;
> +    bool     fpu_dp_option;
> +    bool     fpu_fma_option;
> +    bool     fpu_div_option;
> +    bool     has_actionpoints;
> +    bool     has_fpu;
> +    bool     has_interrupts;
> +    bool     has_mmu;
> +    bool     has_mpu;
> +    bool     has_timer_0;
> +    bool     has_timer_1;
> +    bool     has_pct;
> +    bool     has_rtt;
> +    bool     has_smart;
> +    uint32_t intvbase_preset;
> +    uint32_t lpc_size;
> +    uint8_t  mpu_num_regions;
> +    uint8_t  mpy_option;
> +    uint32_t mmu_page_size_sel0;
> +    uint32_t mmu_page_size_sel1;
> +    uint32_t mmu_pae_enabled;
> +    uint32_t ntlb_num_entries;
> +    uint32_t num_actionpoints;
> +    uint32_t number_of_interrupts;
> +    uint32_t number_of_levels;
> +    uint32_t pct_counters;
> +    uint32_t pct_interrupt;
> +    uint32_t pc_size;
> +    uint32_t rgf_num_regs;

Maybe use 'unsigned' for numbers (various uses).

> +    uint32_t rgf_banked_regs;
> +    uint32_t rgf_num_banks;
> +    bool     rtc_option;
> +    uint32_t rtt_feature_level;
> +    bool     stack_checking;
> +    bool     swap_option;
> +    uint32_t smar_stack_entries;
> +    uint32_t smart_implementation;
> +    uint32_t stlb_num_entries;
> +    uint32_t slc_size;
> +    uint32_t slc_line_size;
> +    uint32_t slc_ways;
> +    uint32_t slc_tag_banks;
> +    uint32_t slc_tram_delay;
> +    uint32_t slc_dbank_width;
> +    uint32_t slc_data_banks;
> +    uint32_t slc_dram_delay;
> +    bool     slc_mem_bus_width;
> +    uint32_t slc_ecc_option;
> +    bool     slc_data_halfcycle_steal;
> +    bool     slc_data_add_pre_pipeline;
> +    bool     uaux_option;
> +    uint32_t freq_hz; /* CPU frequency in hz, needed for timers. */
> +  } cfg;
> +
[...]
Regards,

Phil.