Hi Peter, On Fri, Jun 29, 2012 at 11:15 AM, Peter Crosthwaite <peter.crosthwa...@petalogix.com> wrote: > On Thu, Jun 28, 2012 at 11:56 PM, Jia Liu <pro...@gmail.com> wrote: >> Hi, Peter, Andreas, and Peter, >> >> I've replace env with cpu, and rewrite the load_kernel. >> Please review these new files: >> >> openrisc_pic.c >> >> #include "hw.h" >> #include "cpu.h" >> >> /* OpenRISC pic handler */ >> static void openrisc_pic_cpu_handler(void *opaque, int irq, int level) >> { >> OpenRISCCPU *cpu = (OpenRISCCPU *)opaque; >> int i; >> uint32_t irq_bit = 1 << irq; >> >> if (irq > 31 || irq < 0) { >> return; >> } >> >> if (level) { >> cpu->env.picsr |= irq_bit; >> } else { >> cpu->env.picsr &= ~irq_bit; >> } >> >> for (i = 0; i < 32; i++) { >> if ((cpu->env.picsr && (1 << i)) && (cpu->env.picmr && (1 << i))) { >> cpu_interrupt(&cpu->env, CPU_INTERRUPT_HARD); >> } else { >> cpu_reset_interrupt(&cpu->env, CPU_INTERRUPT_HARD); >> cpu->env.picsr &= ~(1 << i); >> } >> } >> } >> >> void cpu_openrisc_pic_init(OpenRISCCPU *cpu) >> { >> int i; >> qemu_irq *qi; >> qi = qemu_allocate_irqs(openrisc_pic_cpu_handler, cpu, NR_IRQS); >> >> for (i = 0; i < NR_IRQS; i++) { >> cpu->env.irq[i] = qi[i]; >> } >> } >> -------------------------------------------------------------------- >> >> openrisc_timer.c >> >> #include "cpu.h" >> #include "hw.h" >> #include "qemu-timer.h" >> >> #define TIMER_FREQ (20 * 1000 * 1000) /* 20MHz */ >> >> /* The time when TTCR changes */ >> static uint64_t last_clk; >> static int is_counting; >> >> void cpu_openrisc_count_update(OpenRISCCPU *cpu) >> { >> uint64_t now, next; >> uint32_t wait; >> >> now = qemu_get_clock_ns(vm_clock); >> if (!is_counting) { >> qemu_del_timer(cpu->env.timer); >> last_clk = now; >> return; >> } >> >> cpu->env.ttcr += (uint32_t)muldiv64(now - last_clk, TIMER_FREQ, >> get_ticks_per_sec()); >> last_clk = now; >> >> if ((cpu->env.ttmr & TTMR_TP) <= (cpu->env.ttcr & TTMR_TP)) { >> wait = TTMR_TP - (cpu->env.ttcr & TTMR_TP) + 1; >> wait += cpu->env.ttmr & TTMR_TP; >> } else { >> wait = (cpu->env.ttmr & TTMR_TP) - (cpu->env.ttcr & TTMR_TP); >> } >> >> next = now + muldiv64(wait, get_ticks_per_sec(), TIMER_FREQ); >> qemu_mod_timer(cpu->env.timer, next); >> } >> >> void cpu_openrisc_count_start(OpenRISCCPU *cpu) >> { >> is_counting = 1; >> cpu_openrisc_count_update(cpu); >> } >> >> void cpu_openrisc_count_stop(OpenRISCCPU *cpu) >> { >> is_counting = 0; >> cpu_openrisc_count_update(cpu); >> } >> >> static void openrisc_timer_cb(void *opaque) >> { >> OpenRISCCPU *cpu = opaque; >> >> if ((cpu->env.ttmr & TTMR_IE) && >> qemu_timer_expired(cpu->env.timer, qemu_get_clock_ns(vm_clock))) { >> cpu->env.ttmr |= TTMR_IP; >> cpu->env.interrupt_request |= CPU_INTERRUPT_TIMER; >> } >> >> switch (cpu->env.ttmr & TTMR_M) { >> case TIMER_NONE: >> break; >> case TIMER_INTR: >> cpu->env.ttcr = 0; >> cpu_openrisc_count_start(cpu); >> break; >> case TIMER_SHOT: >> cpu_openrisc_count_stop(cpu); >> break; >> case TIMER_CONT: >> cpu_openrisc_count_start(cpu); >> break; >> } >> } >> >> void cpu_openrisc_clock_init(OpenRISCCPU *cpu) >> { >> cpu->env.timer = qemu_new_timer_ns(vm_clock, &openrisc_timer_cb, cpu); >> cpu->env.ttmr = 0x00000000; >> cpu->env.ttcr = 0x00000000; >> } >> -------------------------------------------------------------------- >> >> openrisc_sim.c >> >> #include "hw.h" >> #include "boards.h" >> #include "elf.h" >> #include "pc.h" >> #include "loader.h" >> #include "exec-memory.h" >> #include "sysemu.h" >> #include "sysbus.h" >> #include "qtest.h" >> >> #define KERNEL_LOAD_ADDR 0x100 >> >> static void main_cpu_reset(void *opaque) >> { >> OpenRISCCPU *cpu = opaque; >> >> cpu_reset(CPU(cpu)); >> } >> >> static void openrisc_sim_net_init(MemoryRegion *address_space, >> target_phys_addr_t base, >> target_phys_addr_t descriptors, >> qemu_irq irq, NICInfo *nd) >> { >> DeviceState *dev; >> SysBusDevice *s; >> >> dev = qdev_create(NULL, "open_eth"); >> qdev_set_nic_properties(dev, nd); >> qdev_init_nofail(dev); >> >> s = sysbus_from_qdev(dev); >> sysbus_connect_irq(s, 0, irq); >> memory_region_add_subregion(address_space, base, >> sysbus_mmio_get_region(s, 0)); >> memory_region_add_subregion(address_space, descriptors, >> sysbus_mmio_get_region(s, 1)); >> } >> >> static void openrisc_sim_init(ram_addr_t ram_size, >> const char *boot_device, >> const char *kernel_filename, >> const char *kernel_cmdline, >> const char *initrd_filename, >> const char *cpu_model) >> { >> long kernel_size; >> uint64_t elf_entry; >> target_phys_addr_t entry; >> OpenRISCCPU *cpu = NULL; >> MemoryRegion *ram; >> int n; >> >> if (!cpu_model) { >> cpu_model = "or1200"; >> } >> >> for (n = 0; n < smp_cpus; n++) { >> cpu = cpu_openrisc_init(cpu_model); >> if (cpu == NULL) { >> qemu_log("Unable to find CPU defineition!\n"); >> exit(1); >> } >> qemu_register_reset(main_cpu_reset, cpu); >> main_cpu_reset(cpu); >> } >> >> ram = g_malloc(sizeof(*ram)); >> memory_region_init_ram(ram, "openrisc.ram", ram_size); >> vmstate_register_ram_global(ram); >> memory_region_add_subregion(get_system_memory(), 0, ram); >> >> /* load kernel */ >> if (kernel_filename && !qtest_enabled()) { >> kernel_size = load_elf(kernel_filename, NULL, NULL, >> &elf_entry, NULL, NULL, 1, ELF_MACHINE, 1); >> entry = elf_entry; >> if (kernel_size < 0) { >> kernel_size = load_uimage(kernel_filename, >> &entry, NULL, NULL); >> } >> if (kernel_size < 0) { >> kernel_size = load_image_targphys(kernel_filename, >> KERNEL_LOAD_ADDR, >> ram_size - KERNEL_LOAD_ADDR); >> entry = KERNEL_LOAD_ADDR; >> } >> >> if (kernel_size < 0) { >> qemu_log("QEMU: couldn't load the kernel '%s'\n", >> kernel_filename); >> exit(1); >> } >> } > > I think it was cleaner to have the load_kernel() as its own function, > just without all the dead struct args. Seperates bootloader from > machine init and add a little bit of future proofing when we come to > extract out bootloaders from machine models. But I wont block on it. > Can you move this hunk (or the load_kernel() call) to the end of the > function so bootloading happens after machine creation? > > Regards, > Peter > >> >> cpu_openrisc_pic_init(cpu); >> cpu_openrisc_clock_init(cpu); >> >> serial_mm_init(get_system_memory(), 0x90000000, 0, cpu->env.irq[2], >> 115200, serial_hds[0], DEVICE_NATIVE_ENDIAN); >> >> if (nd_table[0].vlan) { >> openrisc_sim_net_init(get_system_memory(), 0x92000000, >> 0x92000400, cpu->env.irq[4], nd_table); >> } > > So after this stuff. Bootload here. > >> >> cpu->env.pc = entry; > > I would roll this into the load_kernel call too if you went for that > approach. Part of loading a kernel is setting the entry point so it > makes sense to have one BL function do everything. Again, I wont > block, just a suggestion. >
Thanks for review. And, thank you very much for your suggestion. Is this new code OK? static void cpu_openrisc_load_kernel(ram_addr_t ram_size, const char* kernel_filename, OpenRISCCPU *cpu) { long kernel_size; uint64_t elf_entry; target_phys_addr_t entry; if (kernel_filename && !qtest_enabled()) { kernel_size = load_elf(kernel_filename, NULL, NULL, &elf_entry, NULL, NULL, 1, ELF_MACHINE, 1); entry = elf_entry; if (kernel_size < 0) { kernel_size = load_uimage(kernel_filename, &entry, NULL, NULL); } if (kernel_size < 0) { kernel_size = load_image_targphys(kernel_filename, KERNEL_LOAD_ADDR, ram_size - KERNEL_LOAD_ADDR); entry = KERNEL_LOAD_ADDR; } if (kernel_size < 0) { qemu_log("QEMU: couldn't load the kernel '%s'\n", kernel_filename); exit(1); } } cpu->env.pc = entry; } static void openrisc_sim_init(ram_addr_t ram_size, const char *boot_device, const char *kernel_filename, const char *kernel_cmdline, const char *initrd_filename, const char *cpu_model) { OpenRISCCPU *cpu = NULL; MemoryRegion *ram; int n; if (!cpu_model) { cpu_model = "or1200"; } for (n = 0; n < smp_cpus; n++) { cpu = cpu_openrisc_init(cpu_model); if (cpu == NULL) { qemu_log("Unable to find CPU defineition!\n"); exit(1); } qemu_register_reset(main_cpu_reset, cpu); main_cpu_reset(cpu); } ram = g_malloc(sizeof(*ram)); memory_region_init_ram(ram, "openrisc.ram", ram_size); vmstate_register_ram_global(ram); memory_region_add_subregion(get_system_memory(), 0, ram); cpu_openrisc_pic_init(cpu); cpu_openrisc_clock_init(cpu); serial_mm_init(get_system_memory(), 0x90000000, 0, cpu->env.irq[2], 115200, serial_hds[0], DEVICE_NATIVE_ENDIAN); if (nd_table[0].vlan) { openrisc_sim_net_init(get_system_memory(), 0x92000000, 0x92000400, cpu->env.irq[4], nd_table); } cpu_openrisc_load_kernel(ram_size, kernel_filename, cpu); } >> } >> >> static QEMUMachine openrisc_sim_machine = { >> .name = "or32-sim", >> .desc = "or32 simulation", >> .init = openrisc_sim_init, >> .max_cpus = 1, >> .is_default = 1, >> }; >> >> static void openrisc_sim_machine_init(void) >> { >> qemu_register_machine(&openrisc_sim_machine); >> } >> >> machine_init(openrisc_sim_machine_init); >> >> >> Please review and let me the new is OK or not, please. >> Thank you, very much, all of you. >> >> >> On Wed, Jun 27, 2012 at 9:10 PM, Peter Crosthwaite >> <peter.crosthwa...@petalogix.com> wrote: >>> On Wed, Jun 27, 2012 at 11:04 PM, Andreas Färber <afaer...@suse.de> wrote: >>>> Am 27.06.2012 14:25, schrieb Peter Crosthwaite: >>>>> Hi Jia, >>>>> >>>>> On Wed, Jun 27, 2012 at 7:54 PM, Jia Liu <pro...@gmail.com> wrote: >>>>>> +static void openrisc_sim_init(ram_addr_t ram_size, >>>>>> + const char *boot_device, >>>>>> + const char *kernel_filename, >>>>>> + const char *kernel_cmdline, >>>>>> + const char *initrd_filename, >>>>>> + const char *cpu_model) >>>>>> +{ >>>>>> + CPUOpenRISCState *env; >>>>>> + MemoryRegion *ram = g_new(MemoryRegion, 1); >>>>>> + >>>>>> + if (!cpu_model) { >>>>>> + cpu_model = "or1200"; >>>>>> + } >>>>>> + env = cpu_init(cpu_model); >>>>>> + if (!env) { >>>>>> + fprintf(stderr, "Unable to find CPU definition!\n"); >>>>>> + exit(1); >>>>>> + } >>>>>> + >>>>>> + qemu_register_reset(main_cpu_reset, env); >>>>>> + main_cpu_reset(env); >>>>>> + >>>>> >>>>> I think this needs rebasing. Andreas a while back abstracted back to >>>>> the CPU level instead for resets. Andreas can you confirm? should this >>>>> be changed to pass the CPU QOM object to the reset instead? cc >>>>> andreas. >>>> >>>> Thought I had commented that already... maybe I'm starting to confuse >>>> uc32 and or32? :) Yes please, cpu_or32_init() should be called and >>>> return an OpenRISCCPU *cpu. main_cpu_reset() should be passed the cpu, >>>> too. All new APIs (static helpers etc.) should use OpenRISCCPU, not >>>> CPUOpenRISCState. >>> >>> That rule has significant impact on patches 9-10. Andreas, you may >>> wish to check these out - they are psuedo device-models tightly >>> coupled to the cpu env. >>> >>> Regards, >>> Peter >>> >>> That will greatly simplify moving forward. >>>> >>>> Thanks for catching this, Peter. >>>> >>>> Andreas >>>> >>>> -- >>>> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany >>>> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg >> >> >> Regards, >> Jia. Regards, Jia.