Re: [PATCH v8 02/29] target/loongarch: Add core definition

[gaosong]

Hi,

On 2021/11/2 下午4:38, Philippe Mathieu-Daudé wrote:
> On 11/1/21 10:51, Song Gao wrote:
> > This patch adds target state header, target definitions
> > and initialization routines.
> >
> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> > Signed-off-by: Song Gao <gaosong@loongson.cn>
> > Signed-off-by: Xiaojuan Yang <yangxiaojuan@loongson.cn>
> > ---
> >   target/loongarch/cpu-param.h |  19 +++
> >   target/loongarch/cpu.c       | 352 +++++++++++++++++++++++++++++++++++++++++++
> >   target/loongarch/cpu.h       | 254 +++++++++++++++++++++++++++++++
> >   target/loongarch/internals.h |  22 +++
> >   4 files changed, 647 insertions(+)
> >   create mode 100644 target/loongarch/cpu-param.h
> >   create mode 100644 target/loongarch/cpu.c
> >   create mode 100644 target/loongarch/cpu.h
> >   create mode 100644 target/loongarch/internals.h
> >
> > +static void set_loongarch_cpucfg(CPULoongArchState *env)
> > +{
> > +    int i;
> > +
> > +    for (i = 0; i < 49; i++) {
> > +        env->cpucfg[i] = 0x0;
> > +    }
> > +
> > +    env->cpucfg[0] = 0x14c010;  /* PRID */
> Why do you insist in calling this generically and not
> loongarch_3a5000_initfn()?

This function is called in 2 places,  loongarch_3a5000_initfn() and 
loongarch_cpu_reset().  So I think we need this function,

But now I realize that the CPUCFG instruction is read-only the cpucfg[i] 
values,  We need't to call this function in loongarch_cpu_reset().  We 
should move this code in loongarch_3a5000_initfn().

> If you want a generic function,
> why not pass PRID and xtal freq as arguments?

Currently the cpucfg[i] values are fixed.   the cpucfg[i] values are 
from 3a5000 Hardware.

I didn't realize that the cpucfg[i] valuse might be different on 3a6000 
or  later.

I think we need a  variable features  on CPULoongArchState, like  arm.  
This is what you mentioned before.

> > +
> > +    uint32_t data = 0;
> > +    data = FIELD_DP32(data, CPUCFG1, ARCH, 2);
> > +    data = FIELD_DP32(data, CPUCFG1, PGMMU, 1);
> > +    data = FIELD_DP32(data, CPUCFG1, IOCSR, 1);
> > +    data = FIELD_DP32(data, CPUCFG1, PALEN, 0x2f);
> > +    data = FIELD_DP32(data, CPUCFG1, VALEN, 0x2f);
> > +    data = FIELD_DP32(data, CPUCFG1, UAL, 1);
> > +    data = FIELD_DP32(data, CPUCFG1, RI, 1);
> > +    data = FIELD_DP32(data, CPUCFG1, EP, 1);
> > +    data = FIELD_DP32(data, CPUCFG1, RPLV, 1);
> > +    data = FIELD_DP32(data, CPUCFG1, HP, 1);
> > +    data = FIELD_DP32(data, CPUCFG1, IOCSR_BRD, 1);
> > +    env->cpucfg[1] = data;
> > +
> > +    data = 0;
> > +    data = FIELD_DP32(data, CPUCFG2, FP, 1);
> > +    data = FIELD_DP32(data, CPUCFG2, FP_SP, 1);
> > +    data = FIELD_DP32(data, CPUCFG2, FP_DP, 1);
> > +    data = FIELD_DP32(data, CPUCFG2, FP_VER, 1);
> > +    data = FIELD_DP32(data, CPUCFG2, LSX, 1);
> > +    data = FIELD_DP32(data, CPUCFG2, LASX, 1);
> > +    data = FIELD_DP32(data, CPUCFG2, COMPLEX, 1);
> > +    data = FIELD_DP32(data, CPUCFG2, CRYPTO, 1);
> > +    data = FIELD_DP32(data, CPUCFG2, LLFTP, 1);
> > +    data = FIELD_DP32(data, CPUCFG2, LLFTP_VER, 1);
> > +    data = FIELD_DP32(data, CPUCFG2, LSPW, 1);
> > +    data = FIELD_DP32(data, CPUCFG2, LAM, 1);
> > +    env->cpucfg[2] = data;
> > +
> > +    data = 0;
> > +    data = FIELD_DP32(data, CPUCFG3, CCDMA, 1);
> > +    data = FIELD_DP32(data, CPUCFG3, SFB, 1);
> > +    data = FIELD_DP32(data, CPUCFG3, UCACC, 1);
> > +    data = FIELD_DP32(data, CPUCFG3, LLEXC, 1);
> > +    data = FIELD_DP32(data, CPUCFG3, SCDLY, 1);
> > +    data = FIELD_DP32(data, CPUCFG3, LLDBAR, 1);
> > +    data = FIELD_DP32(data, CPUCFG3, ITLBHMC, 1);
> > +    data = FIELD_DP32(data, CPUCFG3, ICHMC, 1);
> > +    data = FIELD_DP32(data, CPUCFG3, SPW_LVL, 4);
> > +    data = FIELD_DP32(data, CPUCFG3, SPW_HP_HF, 1);
> > +    env->cpucfg[3] = data;
> > +
> > +    env->cpucfg[4] = 0x5f5e100; /* Crystal frequency */
> > +
> > +    data = 0;
> > +    data = FIELD_DP32(data, CPUCFG5, CC_MUL, 1);
> > +    data = FIELD_DP32(data, CPUCFG5, CC_DIV, 1);
> > +    env->cpucfg[5] = data;
> > +
> > +    data = 0;
> > +    data = FIELD_DP32(data, CPUCFG16, L1_IUPRE, 1);
> > +    data = FIELD_DP32(data, CPUCFG16, L1_DPRE, 1);
> > +    data = FIELD_DP32(data, CPUCFG16, L2_IUPRE, 1);
> > +    data = FIELD_DP32(data, CPUCFG16, L2_IUUNIFY, 1);
> > +    data = FIELD_DP32(data, CPUCFG16, L2_IUPRIV, 1);
> > +    data = FIELD_DP32(data, CPUCFG16, L3_IUPRE, 1);
> > +    data = FIELD_DP32(data, CPUCFG16, L3_IUUNIFY, 1);
> > +    data = FIELD_DP32(data, CPUCFG16, L3_IUINCL, 1);
> > +    env->cpucfg[16] = data;
> > +
> > +    data = 0;
> > +    data = FIELD_DP32(data, CPUCFG17, L1IU_WAYS, 0x8003);
> > +    data = FIELD_DP32(data, CPUCFG17, L1IU_SETS, 0x60);
> > +    env->cpucfg[17] =  data;
> > +
> > +    data = 0;
> > +    data = FIELD_DP32(data, CPUCFG18, L1D_WAYS, 0x8003);
> > +    data = FIELD_DP32(data, CPUCFG18, L1D_SETS, 0x60);
> > +    env->cpucfg[18] = data;
> > +
> > +    data = 0;
> > +    data = FIELD_DP32(data, CPUCFG19, L2IU_WAYS, 0x800f);
> > +    data = FIELD_DP32(data, CPUCFG19, L2IU_SETS, 0x60);
> > +    env->cpucfg[19] = data;
> > +
> > +    data = 0;
> > +    data = FIELD_DP32(data, CPUCFG20, L3IU_WAYS, 0xf00f);
> > +    data = FIELD_DP32(data, CPUCFG20, L3IU_SETS, 0x60);
> > +    env->cpucfg[20] = data;
> > +}
> > +
> > +static inline void loongarch_3a5000_initfn(Object *obj)
> 'inline' is not justified.

I misunderstood what you meant in V7 comments,  I'll correct it.

Thanks.

Song Gao