Re: [PATCH for-5.1 4/7] target/mips: Add Loongson-3 CPU definition

["Philippe Mathieu-Daudé" <f4bug@amsat.org>]

On 4/29/20 11:25 AM, Huacai Chen wrote:
> Hi, Philippe,
> 
> I think that this CPU definition is for Guest, not for Host (Maybe I'm wrong?).

OK, it was not obvious to me (I haven't received the cover letter of 
this series).
The code you modified is used by both host emulation and guest 
virtualization.

If you want to only add a guest cpu type, you should name it 
"Loongson-3A virtualized" and can restrict with:

   if (kvm_enabled()) {
     add_guest_cpu();
   }

But cleaner is to add the host cpu regardless, and in cpu_state_reset() 
restrict when virtualized:

   if (kvm_enabled()) {
     /* disable host features on guest */
     env->CP0_Config3 &= ~CP0C3_VZ;
     ...
   }

> 
> On Wed, Apr 29, 2020 at 4:58 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> On 4/29/20 10:27 AM, Huacai Chen wrote:
>>> Hi, Philippe,
>>>
>>> The major differences of R3 and R4 are:
>>> 1, R4 has complete MIPS VZ ASE (while R3 is incomplete), so very
>>> usable for KVM host;
>>
>> So you need to set CP0C3_VZ.
> If the definition is for Guest, I think CP0C3_VZ should not be set in
> CP0_Config3, because only the Host can see VZ ASE.
> 
>>
>>> 2, R4 has MSA ASE while R3 hasn't;
>>
>> So you need to set CP0C5_MSAEn.
> I have already set CP0C5_MSAEn CP0_Config5_rw_bitmask, but I don't
> think it should be set in CP0_Config5 because MSA is disabled by
> default.
> 
>>
>>> 3, R4 has cpucfg, rdcsr and wrcsr instructions (similar to cpuid,
>>> rdmsr and wrmsr in X86).
>>
>> OK, these are not implemented.

I was looking at TCG emulation, now I understand why you don't mind 
about them there.

>>
>> [...]
>>
>>>>>>>>> diff --git a/target/mips/translate_init.inc.c b/target/mips/translate_init.inc.c
>>>>>>>>> index 6d145a9..a32412d 100644
>>>>>>>>> --- a/target/mips/translate_init.inc.c
>>>>>>>>> +++ b/target/mips/translate_init.inc.c
>>>>>>>>> @@ -802,6 +802,57 @@ const mips_def_t mips_defs[] =
>>>>>>>>>             .mmu_type = MMU_TYPE_R4000,
>>>>>>>>>         },
>>>>>>>>>         {
>>>>>>>>> +        .name = "Loongson-3A",
>>>>>>>>> +        .CP0_PRid = 0x14C000,
>>>>>>>>> +        /* 64KB I-cache and d-cache. 4 way with 32 bit cache line size.  */
>>>>>>>>> +        .CP0_Config0 = MIPS_CONFIG0 | (0x1 << CP0C0_AR) | (0x2 << CP0C0_AT) |
>>>>>>>>> +                       (MMU_TYPE_R4000 << CP0C0_MT),
>>>>>>>>> +        .CP0_Config1 = MIPS_CONFIG1 | (1 << CP0C1_FP) | (63 << CP0C1_MMU) |
>>>>>>>>> +                       (2 << CP0C1_IS) | (4 << CP0C1_IL) | (3 << CP0C1_IA) |
>>>>>>>>> +                       (2 << CP0C1_DS) | (4 << CP0C1_DL) | (3 << CP0C1_DA) |
>>>>>>>>> +                       (1 << CP0C1_PC) | (1 << CP0C1_WR) | (1 << CP0C1_EP),
>>>>>>>>> +        .CP0_Config2 = MIPS_CONFIG2,
>>>>>>>>> +        .CP0_Config3 = MIPS_CONFIG3 | (1U << CP0C3_M) | (1 << CP0C3_MSAP) |
>>>>>>>>> +                       (1 << CP0C3_BP) | (1 << CP0C3_BI) | (1 << CP0C3_ULRI) |
>>>>>>>>> +                       (1 << CP0C3_RXI) | (1 << CP0C3_LPA) | (1 << CP0C3_VInt),
>>>>>>>>> +        .CP0_Config4 = MIPS_CONFIG4 | (1U << CP0C4_M) | (2 << CP0C4_IE) |
>>>>>>>>> +                       (1 << CP0C4_AE) | (0x1c << CP0C4_KScrExist),
>>>>>>>>> +        .CP0_Config4_rw_bitmask = 0,
>>>>>>>>> +        .CP0_Config5 = MIPS_CONFIG5 | (1 << CP0C5_NFExists) | (1 << 18),
>>>>>>>>> +        .CP0_Config5_rw_bitmask = (1 << CP0C5_K) | (1 << CP0C5_CV) |
>>>>>>>>> +                                  (1 << CP0C5_MSAEn) | (1 << CP0C5_UFE) |
>>>>>>>>> +                                  (1 << CP0C5_FRE) | (1 << CP0C5_SBRI),
>>>>>>>>> +        .CP0_Config6 = (1 << CP0C6_VCLRU) | (1 << CP0C6_DCLRU) | (1 << CP0C6_SFBEN) |
>>>>>>>>> +                       (1 << CP0C6_FLTINT) | (1 << CP0C6_INSTPREF) | (1 << CP0C6_DATAPREF),
>>>>>>>>> +        .CP0_Config6_rw_bitmask = (1 << CP0C6_BPPASS) | (0x3f << CP0C6_KPOS) |
>>>>>>>>> +                                  (1 << CP0C6_KE) | (1 << CP0C6_VTLBONLY) | (1 << CP0C6_LASX) |
>>>>>>>>> +                                  (1 << CP0C6_SSEN) | (1 << CP0C6_DISDRTIME) |
>>>>>>>>> +                                  (1 << CP0C6_PIXNUEN) | (1 << CP0C6_SCRAND) |
>>>>>>>>> +                                  (1 << CP0C6_LLEXCEN) | (1 << CP0C6_DISVC) |
>>>>>>>>> +                                  (1 << CP0C6_VCLRU) | (1 << CP0C6_DCLRU) |
>>>>>>>>> +                                  (1 << CP0C6_PIXUEN) | (1 << CP0C6_DISBLKLYEN) |
>>>>>>>>> +                                  (1 << CP0C6_UMEMUALEN) | (1 << CP0C6_SFBEN) |
>>>>>>>>> +                                  (1 << CP0C6_FLTINT) | (1 << CP0C6_VLTINT) |
>>>>>>>>> +                                  (1 << CP0C6_DISBTB) | (3 << CP0C6_STPREFCTL) |
>>>>>>>>> +                                  (1 << CP0C6_INSTPREF) | (1 << CP0C6_DATAPREF),
>>>>>>>>> +        .CP0_Config7 = 0,
>>>>>>>>> +        .CP0_Config7_rw_bitmask = (1 << CP0C7_NAPCGEN) | (1 << CP0C7_UNIMUEN) | \
>>>>>>>>> +                                  (1 << CP0C7_VFPUCGEN),
>>>>>>>>> +        .CP0_LLAddr_rw_bitmask = 1,
>>>>>>>>> +        .SYNCI_Step = 16,
>>>>>>>>> +        .CCRes = 2,
>>>>>>>>> +        .CP0_Status_rw_bitmask = 0x7DDBFFFF,
>>>>>>>>> +        .CP0_PageGrain_rw_bitmask = (1U << CP0PG_RIE) | (1 << CP0PG_XIE) |
>>>>>>>>> +                    (1 << CP0PG_ELPA) | (1 << CP0PG_IEC),
>>>>>>>>> +        .CP1_fcr0 = (0x5 << FCR0_PRID) | (0x1 << FCR0_REV) | (0x1 << FCR0_F64),
>>>>>>>>> +        .CP1_fcr31 = 0,
>>>>>>>>> +        .CP1_fcr31_rw_bitmask = 0xFF83FFFF,
>>>>>>>>> +        .SEGBITS = 48,
>>>>>>>>> +        .PABITS = 48,
>>>>>>>>> +        .insn_flags = CPU_LOONGSON3A,
>>>>>>>>> +        .mmu_type = MMU_TYPE_R4000,
>>>>>>>>> +    },
>>>>>>>>> +    {
>>>>>>>>>             /* A generic CPU providing MIPS64 DSP R2 ASE features.
>>>>>>>>>                FIXME: Eventually this should be replaced by a real CPU model. */
>>>>>>>>>             .name = "mips64dspr2",
>>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Huacai Chen
>>>>>
>>>
>