On 02.05.2011, at 14:05, Edgar E. Iglesias wrote:
> On Mon, May 02, 2011 at 01:04:04PM +0200, Alexander Graf wrote:
>> The MPC8544DS board emulation was only used with KVM so far, so some
>> parts of the code didn't provide proper values for non-KVM execution.
>>
>> This patch makes the machine work without KVM enabled. To actually use
>> this, you also need proper e500v2 MMU emulation.
>
>
> Just a few inline questions...
>
>
>>
>> Signed-off-by: Alexander Graf <ag...@suse.de>
>> ---
>> hw/ppce500_mpc8544ds.c | 56 +++++++++++++++++++++++++++++++++++++++--------
>> 1 files changed, 46 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
>> index 1b8a1c4..b5dd937 100644
>> --- a/hw/ppce500_mpc8544ds.c
>> +++ b/hw/ppce500_mpc8544ds.c
>> @@ -28,6 +28,7 @@
>> #include "kvm_ppc.h"
>> #include "device_tree.h"
>> #include "openpic.h"
>> +#include "ppc.h"
>> #include "ppce500.h"
>> #include "loader.h"
>> #include "elf.h"
>> @@ -82,7 +83,7 @@ static int mpc8544_load_device_tree(target_phys_addr_t
>> addr,
>> {
>> int ret = -1;
>> #ifdef CONFIG_FDT
>> - uint32_t mem_reg_property[] = {0, ramsize};
>> + uint32_t mem_reg_property[] = {0, cpu_to_be32(ramsize)};
>> char *filename;
>> int fdt_size;
>> void *fdt;
>> @@ -103,15 +104,19 @@ static int mpc8544_load_device_tree(target_phys_addr_t
>> addr,
>> if (ret < 0)
>> fprintf(stderr, "couldn't set /memory/reg\n");
>>
>> - ret = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-start",
>> - initrd_base);
>> - if (ret < 0)
>> - fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
>> + if (initrd_size) {
>> + ret = qemu_devtree_setprop_cell(fdt, "/chosen",
>> "linux,initrd-start",
>> + initrd_base);
>> + if (ret < 0) {
>> + fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
>> + }
>>
>> - ret = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-end",
>> - (initrd_base + initrd_size));
>> - if (ret < 0)
>> - fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
>> + ret = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-end",
>> + (initrd_base + initrd_size));
>> + if (ret < 0) {
>> + fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
>> + }
>> + }
>>
>> ret = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs",
>> kernel_cmdline);
>> @@ -145,6 +150,13 @@ static int mpc8544_load_device_tree(target_phys_addr_t
>> addr,
>>
>> mpc8544_copy_soc_cell(fdt, buf, "clock-frequency");
>> mpc8544_copy_soc_cell(fdt, buf, "timebase-frequency");
>> + } else {
>> + const uint32_t freq = 400000000;
>> +
>> + qemu_devtree_setprop_cell(fdt, "/cpus/PowerPC,8544@0",
>> + "clock-frequency", freq);
>> + qemu_devtree_setprop_cell(fdt, "/cpus/PowerPC,8544@0",
>> + "timebase-frequency", freq);
>> }
>>
>> ret = rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
>> @@ -156,6 +168,21 @@ out:
>> return ret;
>> }
>>
>> +/* Create reset TLB entries for BookE, spanning the 32bit addr space. */
>> +static void mmubooke_create_initial_mapping(CPUState *env,
>> + target_ulong va,
>> + target_phys_addr_t pa)
>> +{
>> + ppcemb_tlb_t *tlb = &env->tlb[512].tlbe;
>> +
>> + tlb->attr = 0;
>> + tlb->prot = PAGE_VALID | ((PAGE_READ | PAGE_WRITE | PAGE_EXEC) << 4);
>> + tlb->size = 256 * 1024 * 1024;
>
>
> Is this code really mapping the entire 32bit addr space as the comment says?
You're right - the comment is wrong :).
>
>
>> + tlb->EPN = va & TARGET_PAGE_MASK;
>> + tlb->RPN = pa & TARGET_PAGE_MASK;
>> + tlb->PID = 0;
>> +}
>> +
>> static void mpc8544ds_init(ram_addr_t ram_size,
>> const char *boot_device,
>> const char *kernel_filename,
>> @@ -188,6 +215,10 @@ static void mpc8544ds_init(ram_addr_t ram_size,
>> exit(1);
>> }
>>
>> + /* XXX register timer? */
>> + ppc_emb_timers_init(env, 400000000, PPC_INTERRUPT_DECR);
>> + ppc_dcr_init(env, NULL, NULL);
>> +
>> /* Fixup Memory size on a alignment boundary */
>> ram_size &= ~(RAM_SIZES_ALIGN - 1);
>>
>> @@ -265,6 +296,9 @@ static void mpc8544ds_init(ram_addr_t ram_size,
>>
>> /* If we're loading a kernel directly, we must load the device tree too.
>> */
>> if (kernel_filename) {
>> +#ifndef CONFIG_FDT
>> + cpu_abort(env, "Compiled without FDT support - can't load
>> kernel\n");
>> +#endif
>> dt_base = (kernel_size + DTC_LOAD_PAD) & ~DTC_PAD_MASK;
>> if (mpc8544_load_device_tree(dt_base, ram_size,
>> initrd_base, initrd_size, kernel_cmdline) < 0) {
>> @@ -272,11 +306,13 @@ static void mpc8544ds_init(ram_addr_t ram_size,
>> exit(1);
>> }
>>
>> + cpu_reset(env);
>> +
>> /* Set initial guest state. */
>> env->gpr[1] = (16<<20) - 8;
>> env->gpr[3] = dt_base;
>> env->nip = entry;
>> - /* XXX we currently depend on KVM to create some initial TLB
>> entries. */
>> + mmubooke_create_initial_mapping(env, 0, 0);
>> }
>>
>
> Should the cpu_reset & friends maybe be done from a qemu_register_reset
> registerd callback?
That would definitely be better, yes! Will fix and send out v3.
Alex
