Re: [Qemu-devel] [PATCH v5 6/8] target/mips: Define the R5900 CPU

[Philippe Mathieu-Daudé]

Hi Maciej, Fredrik.

On 9/21/18 1:51 AM, Maciej W. Rozycki wrote:
> Hi Philippe,
> 
>>> diff --git a/target/mips/translate_init.inc.c 
>>> b/target/mips/translate_init.inc.c
>>> index b3320b9dc7..71fd83de06 100644
>>> --- a/target/mips/translate_init.inc.c
>>> +++ b/target/mips/translate_init.inc.c
>>> @@ -410,6 +410,53 @@ const mips_def_t mips_defs[] =
>>>          .insn_flags = CPU_MIPS32R5 | ASE_MSA,
>>>          .mmu_type = MMU_TYPE_R4000,
>>>      },
>>> +    {
>>> +        .name = "R5900",
>>
>> What bothers me here is you are not modeling "The" unique R5900, but a
>> cpu which implements the R5900 architecture.
> 
>  There was only one R5900 implementation.
> 
>> >From the "TX7901 User’s Manual (Rev. 6.30T – Nov, 2001)":
>>
>>   The TX7901 MIPS RISC microcontroller is a highly integrated solution
>>   based on Toshiba’s dual-issue super-scalar pipeline Processor Core,
>>   the C790 (henceforth referred to as “the C790”).
>>
>>   C790 High-performance MIPS CPU Core on which the TX7901 is based.
>>
>> So the correct core name is "C790".
> 
>  The C790 is a follow-up to the R5900.  The R5900 has an FPU that is not 
> compliant to the IEEE 754 standard for floating-point arithmetic.  It 
> doesn't implement exceptions, infinities, NaNs or denormals.  It doesn't 
> implement the the double format either, but that is really tangential, 
> because the same was the case with the IDT R4650, which had a standard 
> IEEE 754 FPU, but no double format either.  Finally it has some anomalies 
> in the FP instruction set; not all opcode encodings are standard.
> 
>  No documentation has been made publicly available for the R5900; my 
> understanding is it was only released under NDA as a part of the PS2 SDK.  
> However I have been told that the only software-visible difference (except 
> for graphic coprocessors) between the R5900 and the C790 is the FPU, which 
> in the latter processor is a standard IEEE 754 implementation.
> 
>  So the document named is the best public reference for the R5900.  Also 
> any emulation of the C790 will have to use a separate template.

OK.

> 
>>> +        .CP0_PRid = 0x00003800,
>>
>> "The implementation number of the C790 processor is 0x38".
> 
>  I'll leave the answer to what the values of PRId are for the R5900 vs the 
> C790 to Fredrik or Jürgen.  I suspect that the difference is in the Rev 
> field only.

Yes, I think so.

Fredrik: maybe you can simply name the C790 in the comment pointing to
the DS documentation.

>> >From the DS:
>>
>>   The C790 core has the following features:
>>    - Large on-chip caches
>>     • Instruction cache: 32KB, 2-way set associative
>>     • Data cache: 32KB, 2-way set-associative (with write-back protocol)
>>
>> 0x2 << CP0C0_K0 is 'Uncached', why you selected this and not 0x3 for
>> 'Cacheable, write-back, write allocate'?
> 
>  No other MIPS emulation does what you suggest, which is why I refrained 
> from commenting on the K0 field and commented on the ICE/DCE bits only.  
> See:
> 
> /* Have config1, uncached coherency */
> #define MIPS_CONFIG0                                              \
>   ((1U << CP0C0_M) | (0x2 << CP0C0_K0))
> 
> elsewhere in this file.

Yes, this was just out of curiosity.

>>> +        .PABITS = 32,
>>
>> 20 bits PFN + 12 bits offset = 32 (Maciej, can you verify please?).
> 
>  That's what I wrote previously.
> 
>  NB the offset being 12-bit is only the minimum, depending on the PageMask 
> register and used for 4kiB pages only.  But it is a useful simplification.

OK.

Fredrik, except the linux-user part for kernel FPU emulation that I
don't feel confident to review, the rest of this patch looks OK to me:
Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

Regards,

Phil.