On 24.01.2014, at 01:49, Scott Wood scottwood@freescale.com wrote:
On Thu, 2014-01-23 at 14:08 +0100, Alexander Graf wrote:
On 21.01.2014, at 03:25, Scott Wood scottwood@freescale.com wrote:
On Sun, 2014-01-19 at 16:19 +0100, Alexander Graf wrote:
For KVM we have a special PV machine type called "ppce500". This machine is inspired by the MPC8544DS board, but implements a lot less features than that one.
It also provides more PCI slots and is supposed to be enumerated by device tree only.
This patch adds support for the current generation ppce500 machine as it is implemented today.
Signed-off-by: Alexander Graf agraf@suse.de
arch/powerpc/cpu/mpc85xx/start.S | 7 + arch/powerpc/include/asm/config_mpc85xx.h | 4 + board/freescale/qemu-ppce500/Makefile | 10 ++ board/freescale/qemu-ppce500/qemu-ppce500.c | 260 +++++++++++++++++++++++++++ board/freescale/qemu-ppce500/tlb.c | 59 ++++++ boards.cfg | 1 + include/configs/qemu-ppce500.h | 235 ++++++++++++++++++++++++ 7 files changed, 576 insertions(+) create mode 100644 board/freescale/qemu-ppce500/Makefile create mode 100644 board/freescale/qemu-ppce500/qemu-ppce500.c create mode 100644 board/freescale/qemu-ppce500/tlb.c create mode 100644 include/configs/qemu-ppce500.h
diff --git a/arch/powerpc/cpu/mpc85xx/start.S b/arch/powerpc/cpu/mpc85xx/start.S index db84d10..ccbcc03 100644 --- a/arch/powerpc/cpu/mpc85xx/start.S +++ b/arch/powerpc/cpu/mpc85xx/start.S @@ -80,6 +80,13 @@ _start_e500: li r1,MSR_DE mtmsr r1
+#ifdef CONFIG_QEMU_E500
- /* Save our ePAPR device tree off before we clobber it */
- lis r2, CONFIG_QEMU_DT_ADDR@h
- ori r2, r2, CONFIG_QEMU_DT_ADDR@l
- stw r3, 0(r2)
+#endif
r2 has a special purpose -- please don't use it for other things, even if it's too early to be a problem and other code is similarly bad.
Heh, ok. I'll use r4 instead.
Instead of saving the DT pointer in some random hardcoded address, how about sticking it in a callee-saved register until you're able to store it in the global data struct?
I did that at first but that didn't survive relocation. However, I just remembered that I had my global variable in bss, so maybe relocation doesn't work properly there. Instead I put it in .data now and that seems to work.
It's certainly the nicer solution, I agree.
I don't mean a global variable, but a field in the global data struct (gd_t). BSS should not be accessed, and global variables should not be written, before relocation (although you may get away with the latter pre-relocation in ramboot cases, you still shouldn't).
But the global data gets cleared in cpu_init_early_f(). So I'd have to shove it to some non-volatile, make sure no other code uses that particular register and then move it into gd. And how do I get offsets into the gd structure from asm in the first place?
Or are you suggesting I move r3 into r2x, leave it there until after cpu_init_early_f(), copy it back to r3 and then call a C helper that puts it into gd? Sounds quite excessive for something that works quite well with a global variable in .data :).
/** We already know where the initrd is inside the dtb, so no* need to override it*/setenv("ramdisk_addr", "-");Indentation.
What if the user wants to specify the initrd from within U-Boot? This should be handled via the default environment, not by overriding the user's choice.
I'm not sure I see the difference. We have the following default boot script:
#define CONFIG_BOOTCOMMAND \ "test -n "$kernel_addr" && bootm $kernel_addr $ramdisk_addr $fdt_addr\0"
which is the only place where we actually use $ramdisk_addr. If a user wants to specify the initrd within U-Boot he can do that easily because he would specify his own boot command anyway, right?
Oh. I was confusing it with the existing $ramdiskaddr. So I'm changing my objection to the fact that it's confusing. :-)
Likewise $fdt_addr versus $fdtaddr.
Mind to explain what the difference is between $fdtaddr, $fdt_addr (pxe boot?) and $fdt_addr_r? Which one should I set?
Maybe I should rename kernel_addr to qemu_kernel_addr and drop ramdisk_addr completely in favor for a - in the bootm command? Yeah, I'll do that :).
OK.
- ret = fdt_open_into((void*)dt_base, fdt, dt_size);
- if (ret)
panic("Couldn't open fdt");- memory = fdt_path_offset(fdt, "/memory");
- prop = fdt_getprop(fdt, memory, "reg", &len);
- if (prop && len >= 16)
return *(uint64_t*)(prop+8);- panic("Couldn't determine RAM size");
+}
Again, there's no need to create a temporary fdt copy every time you want to read from it.
What if memory doesn't start at zero (e.g. for e500v2 VFIO)?
In that case we're pretty broken regardless of determining the correct memory size. Can u-boot handle that case at all? How would this work?
U-Boot can handle this on other architectures. Not sure about the PPC code.
It's handled through preprocessor configuration magic again. I'll leave that for later though.
+unsigned long +get_board_sys_clk(ulong dummy) +{
- /* The actual clock doesn't matter in a PV machine */
- return 33333333;
+}
s/doesn't matter/doesn't exist/
Where is this used from? Can it be skipped for this target? Is the CPU timebase handled properly?
Turns out it's not required at all. Must have been a dependency of something I threw away in between.
OK. I'm still curious about how the timebase frequency is handled.
Incorrectly. The question is whether it matters.
- /* Enter AS=1 */
- asm volatile(
"mfmsr %0 \n""ori %0, %0, 0x30 \n""mtsrr1 %0 \n""bl 1f \n""1: \n""mflr %0 \n""addi %0, %0, 16 \n""mtsrr0 %0 \n""rfi \n"- : "=r"(tmp) : : "lr");
- /* Now we live in AS=1, so we can flush all old maps */
- clear_ddr_tlbs(ram_size >> 20);
- /* and create new ones */
- setup_ddr_tlbs(ram_size >> 20);
- /* Now we can safely go back to AS=0 */
- asm volatile(
"mfmsr %0 \n""subi %0, %0, 0x30 \n""mtsrr1 %0 \n""bl 1f \n""1: \n""mflr %0 \n""addi %0, %0, 16 \n""mtsrr0 %0 \n""rfi \n"- : "=r"(tmp) : : "lr");
- /* And remove our AS=1 map */
- disable_tlb(13);
+}
Why aren't we already in AS1? What code are you replacing with this?
I honestly don't know how it's supposed to work at all usually.
Usually we enter AS1 from start.S during early boot (see switch_as), and return to AS0 later in start.S (see _start_cont) after calling cpu_init_early_f().
Right, but the only function that gets called in AS=1 is cpu_init_early_f() which is CPU, not board specific. I really don't want to mess with common code too much unless I'm 100% sure I don't break it and it doesn't break me next time.
So I need to run this from AS=0 context, which means we need to quickly go out of AS=0, modify the AS=0 map and go back into AS=0.
+#undef CONFIG_RESET_VECTOR_ADDRESS +#define CONFIG_RESET_VECTOR_ADDRESS (0x1000000 - 4) /* 16 MB */
Does QEMU begin execution here, or at the ELF entry point?
At the ELF entry point. But I need to define this to something.
Set CONFIG_SYS_MPC85XX_NO_RESETVEC.
Sounds easy, but doesn't boot anymore now. I guess I'll need to figure out where in that fragile TLB setup mess it fails this time.
+/* CONFIG_NUM_DDR_CONTROLLERS is defined in include/asm/config_mpc85xx.h */ +#define CONFIG_DIMM_SLOTS_PER_CTLR 0 +#define CONFIG_CHIP_SELECTS_PER_CTRL 0
Why are we including code that cares about this?
In file included from cpu.c:25:0: /root/u-boot/include/fsl_ddr_sdram.h:183:7: error: 'CONFIG_CHIP_SELECTS_PER_CTRL' undeclared here (not in a function)
So we don't include code, but we include a header that cares.
I see. Would be nice to clean that up at some point, but OK for now.
+/* Get RAM size from device tree */ +#define CONFIG_DDR_SPD
+#define CONFIG_SYS_CLK_FREQ 33000000
Likewise. BTW, this made up sysclock frequency doesn't match the one you made up elsewhere.
speed.c: In function 'get_sys_info': speed.c:347:42: error: 'CONFIG_SYS_CLK_FREQ' undeclared (first use in this function)
What do we need from speed.c?
Nothing really, but it gets included unconditionally in arch/powerpc/cpu/mpc85xx/Makefile so I can't override it with my own stubs.
Alex