[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH pic32 1/7] Speed of MIPS CPU timer made configur
|
From: |
Serge Vakulenko |
|
Subject: |
Re: [Qemu-devel] [PATCH pic32 1/7] Speed of MIPS CPU timer made configurable per platform. |
|
Date: |
Tue, 30 Jun 2015 18:32:23 -0700 |
Hi Peter,
Thanks for feedback. I will prepare v2 patch set and resubmit it with
cover letter, as you proposed. Some other issues fixed as well.
Regards,
--Serge
2015-06-30 0:44 GMT-07:00 Peter Crosthwaite <address@hidden>:
> Hi,
>
> On Mon, Jun 29, 2015 at 10:00 PM, Serge Vakulenko <address@hidden> wrote:
>> Please find below a set of patches, which allow to simulate Microchip
>> PIC32 microcontrollers on QEMU. For examples of real PIC32
>> applications running on QEMU, see page:
>> https://github.com/sergev/qemu/wiki
>>
>> (1) Make the CPU clock frequency configurable per platform. Currently
>> the clock rate for all MIPS platforms is fixed at 100MHz. Need to make
>> it 40MHz for pic32mx7.
>>
>> (2) When WAIT instruction is executed with interrupts disabled, the
>> processor is essentially suspended forever. Terminate the simulation
>> in this case.
>>
>> (3) For TLBWR instruction, the generated random index value has been
>> not quite random and did not take into account the Wired register
>> value. Fixed.
>>
>> (4) Added support for external interrupt controller mode (EIC).
>> Required for pic32.
>>
>> (5) Added two processor variants: M4K and microAptivUP. Needed for
>> pic32mx and pic32mz simulation.
>>
>> (6) Added two machine platforms: Microchip pic32mx7 and pic32mz
>> microcontrollers. Several board types supported for each platform:
>>
>> pic32mx7-explorer16 PIC32MX7 microcontroller on Microchip Explorer-16
>> board
>> pic32mx7-max32 PIC32MX7 microcontroller on chipKIT Max32 board
>> pic32mx7-maximite PIC32MX7 microcontroller on Geoff's Maximite board
>> pic32mz-explorer16 PIC32MZ microcontroller on Microchip Explorer-16
>> board
>> pic32mz-meb2 PIC32MZ microcontroller on Microchip MEB-II board
>> pic32mz-wifire PIC32MZ microcontroller on chipKIT WiFire board
>>
>> (7) When "-sd" option is used to set a name of file with SD card
>> image, a warning is printed: "WARNING: Image format was not
>> specified". Fixed.
>>
>
> This looks and feels like a cover letter covering the whole series.
>
> I suggest taking your patch-by-patch branch and:
>
> git format-patch master --thread --cover-letter.
>
> Edit the 0000-cover-letter explaining series overall. Send it with the
> rest of the series, which will have a patch 0 documenting the overall
> change.
>
> Some of these listed messages look like they might be better as commit
> messages to individual patches.
>
> Regards,
> Peter
>
>> Signed-off-by: Serge Vakulenko <address@hidden>
>> ---
>> hw/mips/cputimer.c | 13 +++++++------
>> hw/mips/mips_fulong2e.c | 2 +-
>> hw/mips/mips_jazz.c | 2 +-
>> hw/mips/mips_malta.c | 4 ++--
>> hw/mips/mips_mipssim.c | 2 +-
>> hw/mips/mips_r4k.c | 2 +-
>> include/hw/mips/cpudevs.h | 2 +-
>> target-mips/cpu.h | 1 +
>> 8 files changed, 15 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/mips/cputimer.c b/hw/mips/cputimer.c
>> index 577c9ae..3d23c1b 100644
>> --- a/hw/mips/cputimer.c
>> +++ b/hw/mips/cputimer.c
>> @@ -50,8 +50,8 @@ static void cpu_mips_timer_update(CPUMIPSState *env)
>>
>> now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>> wait = env->CP0_Compare - env->CP0_Count -
>> - (uint32_t)muldiv64(now, TIMER_FREQ, get_ticks_per_sec());
>> - next = now + muldiv64(wait, get_ticks_per_sec(), TIMER_FREQ);
>> + (uint32_t)muldiv64(now, env->count_freq, get_ticks_per_sec());
>> + next = now + muldiv64(wait, get_ticks_per_sec(), env->count_freq);
>> timer_mod(env->timer, next);
>> }
>>
>> @@ -80,7 +80,7 @@ uint32_t cpu_mips_get_count (CPUMIPSState *env)
>> }
>>
>> return env->CP0_Count +
>> - (uint32_t)muldiv64(now, TIMER_FREQ, get_ticks_per_sec());
>> + (uint32_t)muldiv64(now, env->count_freq, get_ticks_per_sec());
>> }
>> }
>>
>> @@ -97,7 +97,7 @@ void cpu_mips_store_count (CPUMIPSState *env, uint32_t
>> count)
>> /* Store new count register */
>> env->CP0_Count =
>> count -
>> (uint32_t)muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
>> - TIMER_FREQ, get_ticks_per_sec());
>> + env->count_freq,
>> get_ticks_per_sec());
>> /* Update timer timer */
>> cpu_mips_timer_update(env);
>> }
>> @@ -122,7 +122,7 @@ void cpu_mips_stop_count(CPUMIPSState *env)
>> {
>> /* Store the current value */
>> env->CP0_Count +=
>> (uint32_t)muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
>> - TIMER_FREQ, get_ticks_per_sec());
>> + env->count_freq,
>> get_ticks_per_sec());
>> }
>>
>> static void mips_timer_cb (void *opaque)
>> @@ -145,7 +145,7 @@ static void mips_timer_cb (void *opaque)
>> env->CP0_Count--;
>> }
>>
>> -void cpu_mips_clock_init (CPUMIPSState *env)
>> +void cpu_mips_clock_init(CPUMIPSState *env, unsigned count_freq)
>> {
>> /*
>> * If we're in KVM mode, don't create the periodic timer, that is
>> handled in
>> @@ -153,5 +153,6 @@ void cpu_mips_clock_init (CPUMIPSState *env)
>> */
>> if (!kvm_enabled()) {
>> env->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, &mips_timer_cb, env);
>> + env->count_freq = count_freq;
>> }
>> }
>> diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
>> index dea941a..5decc30 100644
>> --- a/hw/mips/mips_fulong2e.c
>> +++ b/hw/mips/mips_fulong2e.c
>> @@ -343,7 +343,7 @@ static void mips_fulong2e_init(MachineState *machine)
>>
>> /* Init internal devices */
>> cpu_mips_irq_init_cpu(env);
>> - cpu_mips_clock_init(env);
>> + cpu_mips_clock_init(env, 100*1000*1000);
>>
>> /* North bridge, Bonito --> IP2 */
>> pci_bus = bonito_init((qemu_irq *)&(env->irq[2]));
>> diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
>> index 9d60633..02629fa 100644
>> --- a/hw/mips/mips_jazz.c
>> +++ b/hw/mips/mips_jazz.c
>> @@ -210,7 +210,7 @@ static void mips_jazz_init(MachineState *machine,
>>
>> /* Init CPU internal devices */
>> cpu_mips_irq_init_cpu(env);
>> - cpu_mips_clock_init(env);
>> + cpu_mips_clock_init(env, 100*1000*1000);
>>
>> /* Chipset */
>> rc4030 = rc4030_init(&dmas, &rc4030_dma_mr);
>> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
>> index 3082e75..0393017 100644
>> --- a/hw/mips/mips_malta.c
>> +++ b/hw/mips/mips_malta.c
>> @@ -988,7 +988,7 @@ void mips_malta_init(MachineState *machine)
>>
>> /* Init internal devices */
>> cpu_mips_irq_init_cpu(env);
>> - cpu_mips_clock_init(env);
>> + cpu_mips_clock_init(env, 100*1000*1000);
>> qemu_register_reset(main_cpu_reset, cpu);
>> }
>> cpu = MIPS_CPU(first_cpu);
>> @@ -1144,7 +1144,7 @@ void mips_malta_init(MachineState *machine)
>>
>> /* Init internal devices */
>> cpu_mips_irq_init_cpu(env);
>> - cpu_mips_clock_init(env);
>> + cpu_mips_clock_init(env, 100*1000*1000);
>>
>> /*
>> * We have a circular dependency problem: pci_bus depends on isa_irq,
>> diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
>> index 61f74a6..940ca58 100644
>> --- a/hw/mips/mips_mipssim.c
>> +++ b/hw/mips/mips_mipssim.c
>> @@ -213,7 +213,7 @@ mips_mipssim_init(MachineState *machine)
>>
>> /* Init CPU internal devices. */
>> cpu_mips_irq_init_cpu(env);
>> - cpu_mips_clock_init(env);
>> + cpu_mips_clock_init(env, 100*1000*1000);
>>
>> /* Register 64 KB of ISA IO space at 0x1fd00000. */
>> memory_region_init_alias(isa, NULL, "isa_mmio",
>> diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
>> index f4dcacd..5f0b766 100644
>> --- a/hw/mips/mips_r4k.c
>> +++ b/hw/mips/mips_r4k.c
>> @@ -266,7 +266,7 @@ void mips_r4k_init(MachineState *machine)
>>
>> /* Init CPU internal devices */
>> cpu_mips_irq_init_cpu(env);
>> - cpu_mips_clock_init(env);
>> + cpu_mips_clock_init(env, 100*1000*1000);
>>
>> /* ISA bus: IO space at 0x14000000, mem space at 0x10000000 */
>> memory_region_init_alias(isa_io, NULL, "isa-io",
>> diff --git a/include/hw/mips/cpudevs.h b/include/hw/mips/cpudevs.h
>> index b2626f2..4d4e62a 100644
>> --- a/include/hw/mips/cpudevs.h
>> +++ b/include/hw/mips/cpudevs.h
>> @@ -12,6 +12,6 @@ uint64_t cpu_mips_kvm_um_phys_to_kseg0(void *opaque,
>> uint64_t addr);
>> void cpu_mips_irq_init_cpu(CPUMIPSState *env);
>>
>> /* mips_timer.c */
>> -void cpu_mips_clock_init(CPUMIPSState *);
>> +void cpu_mips_clock_init(CPUMIPSState *, unsigned count_freq);
>>
>> #endif
>> diff --git a/target-mips/cpu.h b/target-mips/cpu.h
>> index 474a0e3..c476166 100644
>> --- a/target-mips/cpu.h
>> +++ b/target-mips/cpu.h
>> @@ -595,6 +595,7 @@ struct CPUMIPSState {
>> const mips_def_t *cpu_model;
>> void *irq[8];
>> QEMUTimer *timer; /* Internal timer */
>> + unsigned count_freq; /* rate of Count register */
>> };
>>
>> #include "cpu-qom.h"
>> --
>> 1.9.1
>>