Re: [Qemu-devel] [PATCH pic32 v2 4/5] Two new processor variants: M4K and microAptivP.

[Serge Vakulenko]

On Wed, Jul 1, 2015 at 6:37 AM, Aurelien Jarno <address@hidden> wrote:
> On 2015-06-30 21:12, Serge Vakulenko wrote:
>> Signed-off-by: Serge Vakulenko <address@hidden>
>> ---
>>  target-mips/cpu.h            |  2 ++
>>  target-mips/translate_init.c | 46 
>> ++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 48 insertions(+)
>>
>> diff --git a/target-mips/cpu.h b/target-mips/cpu.h
>> index ab830ee..9f5890c 100644
>> --- a/target-mips/cpu.h
>> +++ b/target-mips/cpu.h
>> @@ -394,6 +394,7 @@ struct CPUMIPSState {
>>  #define CP0C0_M    31
>>  #define CP0C0_K23  28
>>  #define CP0C0_KU   25
>> +#define CP0C0_SB   21
>
> Bits in the range 16:24 are implementation specific, so I do wonder if
> we want to have this bit there. At least we should mark it as
> implementation specific.

I tried to make the configuration as close as possible to a real PIC32
microcontroller - that's why I added Config0.SB and Config7.WII bits.
These bits are described in appropriate Microchip docs. As they are
not relevant for the simulation purposes, I'll better remove them for
simplicity.

>>  #define CP0C0_MDU  20
>>  #define CP0C0_MM   17
>>  #define CP0C0_BM   16
>> @@ -479,6 +480,7 @@ struct CPUMIPSState {
>>  #define CP0C5_NFExists   0
>>      int32_t CP0_Config6;
>>      int32_t CP0_Config7;
>> +#define CP0C7_WII        31
>
> Same as above, Config6 and Config7 are implementation dependent.
>
>>      /* XXX: Maybe make LLAddr per-TC? */
>>      uint64_t lladdr;
>>      target_ulong llval;
>> diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c
>> index ddfaff8..430a547 100644
>> --- a/target-mips/translate_init.c
>> +++ b/target-mips/translate_init.c
>> @@ -232,6 +232,52 @@ static const mips_def_t mips_defs[] =
>>          .mmu_type = MMU_TYPE_FMT,
>>      },
>>      {
>> +        /* Configuration for Microchip PIC32MX microcontroller. */
>> +        .name = "M4K",
>> +        .CP0_PRid = 0x00018765,
>> +        .CP0_Config0 = MIPS_CONFIG0 | (2 << CP0C0_K23) | (2 << CP0C0_KU) |
>> +                       (1 << CP0C0_SB) | (1 << CP0C0_BM) |
>> +                       (1 << CP0C0_AR) | (MMU_TYPE_FMT << CP0C0_MT),
>> +        .CP0_Config1 = (1U << CP0C1_M) | (1 << CP0C1_CA) | (1 << CP0C1_EP),
>> +        .CP0_Config2 = MIPS_CONFIG2,
>> +        .CP0_Config3 = (1 << CP0C3_VEIC) | (1 << CP0C3_VInt),
>> +        .CP0_LLAddr_rw_bitmask = 0,
>> +        .CP0_LLAddr_shift = 4,
>> +        .SYNCI_Step = 32,
>> +        .CCRes = 2,
>> +        .CP0_Status_rw_bitmask = 0x1258FF17,
>> +        .SEGBITS = 32,
>> +        .PABITS = 32,
>> +        .insn_flags = CPU_MIPS32R2 | ASE_MIPS16,
>> +        .mmu_type = MMU_TYPE_FMT,
>> +    },
>> +    {
>> +        /* Configuration for Microchip PIC32MZ microcontroller. */
>> +        .name = "microAptivP",
>> +        .CP0_PRid = 0x00019e28,
>> +        .CP0_Config0 = MIPS_CONFIG0 | (0x1 << CP0C0_AR) |
>> +                    (MMU_TYPE_R4000 << CP0C0_MT),
>> +        .CP0_Config1 = MIPS_CONFIG1 | (15 << CP0C1_MMU) | (1 << CP0C1_PC),
>> +        .CP0_Config2 = MIPS_CONFIG2,
>> +        .CP0_Config3 = (1 << CP0C3_M) | (1 << CP0C3_IPLW) | (1 << 
>> CP0C3_MCU) |
>> +                    (2 << CP0C3_ISA) | (1 << CP0C3_ULRI) | (1 << CP0C3_RXI) 
>> |
>> +                    (1 << CP0C3_DSP2P) | (1 << CP0C3_DSPP) | (1 << 
>> CP0C3_VEIC) |
>> +                    (1 << CP0C3_VInt),
>
> DSP and DSPr2 are enabled here...
>
>> +        .CP0_Config4 = (1 << CP0C4_M),
>> +        .CP0_Config5 = (1 << CP0C5_NFExists),
>> +        .CP0_Config6 = 0,
>> +        .CP0_Config7 = (1 << CP0C7_WII),
>> +        .CP0_LLAddr_rw_bitmask = 0,
>> +        .CP0_LLAddr_shift = 4,
>> +        .SYNCI_Step = 32,
>> +        .CCRes = 2,
>> +        .CP0_Status_rw_bitmask = 0x1278FF17,
>> +        .SEGBITS = 32,
>> +        .PABITS = 32,
>> +        .insn_flags = CPU_MIPS32R2,
>
> so I guess you want to enable ASE_DSP and ASE_DSPR2 here.

Makes sense. Thank you for noticing this.

>> +        .mmu_type = MMU_TYPE_R4000,
>> +    },
>> +    {
>>          .name = "24Kc",
>>          .CP0_PRid = 0x00019300,
>>          .CP0_Config0 = MIPS_CONFIG0 | (0x1 << CP0C0_AR) |
>
> Otherwise it looks ok, though I haven't look at the PIC32 manual to
> check the values.
>
> --
> Aurelien Jarno                          GPG: 4096R/1DDD8C9B
> address@hidden                 http://www.aurel32.net