PPC 85xx: Add support for QEMU's ppce500 PV machine

With the qemu-ppce500 machine type we can run the same board with either an e500v2 or an e500mc core plugged in.

This means that the IVOR setup can't be based on compile time decisions, so instead we have to do a runtime check which CPU generation we're running on.

Signed-off-by Alexander Graf <[email protected]>

---
 arch/powerpc/cpu/mpc85xx/fixed_ivor.S |   21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

arch/powerpc/cpu/mpc85xx/fixed_ivor.S +10 -2

index ebbb8c0..635a97e 100644
@@ -36,17 +36,25 @@
 	SET_IVOR(14, 0x1e0) /* Instruction TLB Error */
 	SET_IVOR(15, 0x040) /* Debug */
 
-/* e500v1 & e500v2 only */
-#ifndef CONFIG_E500MC
+	/* Check for CPU */
+	mfpvr	r3
+	srwi	r3, r3, 16
+	/* Compare with e500mc PVR major number */
+	li	r4, 0
+	ori	r4, r4, 0x8023
+	cmpw	r3, r4
+
+	/* e500v1 & e500v2 only */
+	bge	1f
 	SET_IVOR(32, 0x200) /* SPE Unavailable */
 	SET_IVOR(33, 0x220) /* Embedded FP Data */

With the qemu-ppce500 machine type we can run the same board with either an e500v2 or an e500mc core plugged in.

This means that the IVOR setup can't be based on compile time decisions, so instead we have to do a runtime check which CPU generation we're running on.

Signed-off-by Alexander Graf <[email protected]>

---
 arch/powerpc/cpu/mpc85xx/fixed_ivor.S |   21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

arch/powerpc/cpu/mpc85xx/fixed_ivor.S +15 -5

index ebbb8c0..635a97e 100644
@@ -36,17 +36,25 @@
 	SET_IVOR(14, 0x1e0) /* Instruction TLB Error */
 	SET_IVOR(15, 0x040) /* Debug */
 
-/* e500v1 & e500v2 only */
-#ifndef CONFIG_E500MC
+	/* Check for CPU */
+	mfpvr	r3
+	srwi	r3, r3, 16
+	/* Compare with e500mc PVR major number */
+	li	r4, 0
+	ori	r4, r4, 0x8023
+	cmpw	r3, r4
+
+	/* e500v1 & e500v2 only */
+	bge	1f
 	SET_IVOR(32, 0x200) /* SPE Unavailable */
 	SET_IVOR(33, 0x220) /* Embedded FP Data */
 	SET_IVOR(34, 0x240) /* Embedded FP Round */
-#endif
+1:
 
 	SET_IVOR(35, 0x260) /* Performance monitor */
 
-/* e500mc only */
-#ifdef CONFIG_E500MC
+	/* e500mc only */
+	blt	2f
 	SET_IVOR(36, 0x280) /* Processor doorbell */
 	SET_IVOR(37, 0x2a0) /* Processor doorbell critical */
 	SET_IVOR(38, 0x2c0) /* Guest Processor doorbell */

@@ -54,6 +62,8 @@
 	SET_IVOR(40, 0x300) /* Hypervisor system call */
 	SET_IVOR(41, 0x320) /* Hypervisor Priviledge */
 
+#ifndef CONFIG_QEMU_E500
+	/* QEMU guests runs in guest mode and can't access GIVORs */
 	SET_GIVOR(2, 0x060) /* Guest Data Storage */
 	SET_GIVOR(3, 0x080) /* Guest Instruction Storage */
 	SET_GIVOR(4, 0x0a0) /* Guest External Input */

Quoted 51 lines · Alexander Graf

With the qemu-ppce500 machine type we can run the same board with either an e500v2 or an e500mc core plugged in.

This means that the IVOR setup can't be based on compile time decisions, so instead we have to do a runtime check which CPU generation we're running on.

Signed-off-by Alexander Graf <[email protected]>

---
arch/powerpc/cpu/mpc85xx/fixed_ivor.S | 21 ++++++++++++++++-----

1 file changed, 16 insertions(+), 5 deletions(-)

arch/powerpc/cpu/mpc85xx/fixed_ivor.S +13 -5

index ebbb8c0..635a97e 100644
@@ -36,17 +36,25 @@
 	SET_IVOR(14, 0x1e0) /* Instruction TLB Error */
 	SET_IVOR(15, 0x040) /* Debug */
 
-/* e500v1 & e500v2 only */
-#ifndef CONFIG_E500MC
+	/* Check for CPU */
+	mfpvr	r3
+	srwi	r3, r3, 16
+	/* Compare with e500mc PVR major number */
+	li	r4, 0
+	ori	r4, r4, 0x8023
+	cmpw	r3, r4
+
+	/* e500v1 & e500v2 only */
+	bge	1f
 	SET_IVOR(32, 0x200) /* SPE Unavailable */
 	SET_IVOR(33, 0x220) /* Embedded FP Data */
 	SET_IVOR(34, 0x240) /* Embedded FP Round */
-#endif
+1:
 
 	SET_IVOR(35, 0x260) /* Performance monitor */
 
-/* e500mc only */
-#ifdef CONFIG_E500MC
+	/* e500mc only */
+	blt	2f
 	SET_IVOR(36, 0x280) /* Processor doorbell */
 	SET_IVOR(37, 0x2a0) /* Processor doorbell critical */
 	SET_IVOR(38, 0x2c0) /* Guest Processor doorbell */

@@ -54,6 +62,8 @@
 	SET_IVOR(40, 0x300) /* Hypervisor system call */
 	SET_IVOR(41, 0x320) /* Hypervisor Priviledge */
 

arch/powerpc/cpu/mpc85xx/fixed_ivor.S line 74

Quoted 10 lines

arch/powerpc/cpu/mpc85xx/fixed_ivor.S +3 -0

+#ifndef CONFIG_QEMU_E500
+	/* QEMU guests runs in guest mode and can't access GIVORs */
 	SET_GIVOR(2, 0x060) /* Guest Data Storage */
 	SET_GIVOR(3, 0x080) /* Guest Instruction Storage */
 	SET_GIVOR(4, 0x0a0) /* Guest External Input */

@@ -61,3 +71,4 @@
 	SET_GIVOR(13, 0x1c0) /* Guest Data TLB Error */
 	SET_GIVOR(14, 0x1e0) /* Guest Instruction TLB Error */
 #endif
+2:

Again, let's please just remove this entire file.

Quoted 52 lines · Scott Wood

Quoted 51 lines · Alexander Graf

With the qemu-ppce500 machine type we can run the same board with either an e500v2 or an e500mc core plugged in.

This means that the IVOR setup can't be based on compile time decisions, so instead we have to do a runtime check which CPU generation we're running on.

Signed-off-by Alexander Graf <[email protected]>

---
arch/powerpc/cpu/mpc85xx/fixed_ivor.S | 21 ++++++++++++++++-----

1 file changed, 16 insertions(+), 5 deletions(-)

arch/powerpc/cpu/mpc85xx/fixed_ivor.S +13 -5

index ebbb8c0..635a97e 100644
@@ -36,17 +36,25 @@
 	SET_IVOR(14, 0x1e0) /* Instruction TLB Error */
 	SET_IVOR(15, 0x040) /* Debug */
 
-/* e500v1 & e500v2 only */
-#ifndef CONFIG_E500MC
+	/* Check for CPU */
+	mfpvr	r3
+	srwi	r3, r3, 16
+	/* Compare with e500mc PVR major number */
+	li	r4, 0
+	ori	r4, r4, 0x8023
+	cmpw	r3, r4
+
+	/* e500v1 & e500v2 only */
+	bge	1f
 	SET_IVOR(32, 0x200) /* SPE Unavailable */
 	SET_IVOR(33, 0x220) /* Embedded FP Data */
 	SET_IVOR(34, 0x240) /* Embedded FP Round */
-#endif
+1:
 
 	SET_IVOR(35, 0x260) /* Performance monitor */
 
-/* e500mc only */
-#ifdef CONFIG_E500MC
+	/* e500mc only */
+	blt	2f
 	SET_IVOR(36, 0x280) /* Processor doorbell */
 	SET_IVOR(37, 0x2a0) /* Processor doorbell critical */
 	SET_IVOR(38, 0x2c0) /* Guest Processor doorbell */

@@ -54,6 +62,8 @@
 	SET_IVOR(40, 0x300) /* Hypervisor system call */
 	SET_IVOR(41, 0x320) /* Hypervisor Priviledge */
 

Quoted 12 lines

arch/powerpc/cpu/mpc85xx/fixed_ivor.S line 74

Quoted 10 lines

arch/powerpc/cpu/mpc85xx/fixed_ivor.S +3 -0

+#ifndef CONFIG_QEMU_E500
+	/* QEMU guests runs in guest mode and can't access GIVORs */
 	SET_GIVOR(2, 0x060) /* Guest Data Storage */
 	SET_GIVOR(3, 0x080) /* Guest Instruction Storage */
 	SET_GIVOR(4, 0x0a0) /* Guest External Input */

@@ -61,3 +71,4 @@
 	SET_GIVOR(13, 0x1c0) /* Guest Data TLB Error */
 	SET_GIVOR(14, 0x1e0) /* Guest Instruction TLB Error */
 #endif
+2:

Again, let's please just remove this entire file.

I can remove it, but it's a pretty drastic change from the original behavior that was introduced 4 1/2 years ago:

http://lists.denx.de/pipermail/u-boot/2009-August/058670.html

I assume the idea was to give OSs one thing less to worry about (IVOR setting). If any OS in between 2009 and now relied on that fact, we'll break it by removing this code.

Quoted 72 lines · Alexander Graf

Quoted 52 lines · Scott Wood

Quoted 51 lines · Alexander Graf

With the qemu-ppce500 machine type we can run the same board with either an e500v2 or an e500mc core plugged in.

This means that the IVOR setup can't be based on compile time decisions, so instead we have to do a runtime check which CPU generation we're running on.

Signed-off-by Alexander Graf <[email protected]>

---
arch/powerpc/cpu/mpc85xx/fixed_ivor.S | 21 ++++++++++++++++-----

1 file changed, 16 insertions(+), 5 deletions(-)

arch/powerpc/cpu/mpc85xx/fixed_ivor.S +13 -5

index ebbb8c0..635a97e 100644
@@ -36,17 +36,25 @@
 	SET_IVOR(14, 0x1e0) /* Instruction TLB Error */
 	SET_IVOR(15, 0x040) /* Debug */
 
-/* e500v1 & e500v2 only */
-#ifndef CONFIG_E500MC
+	/* Check for CPU */
+	mfpvr	r3
+	srwi	r3, r3, 16
+	/* Compare with e500mc PVR major number */
+	li	r4, 0
+	ori	r4, r4, 0x8023
+	cmpw	r3, r4
+
+	/* e500v1 & e500v2 only */
+	bge	1f
 	SET_IVOR(32, 0x200) /* SPE Unavailable */
 	SET_IVOR(33, 0x220) /* Embedded FP Data */
 	SET_IVOR(34, 0x240) /* Embedded FP Round */
-#endif
+1:
 
 	SET_IVOR(35, 0x260) /* Performance monitor */
 
-/* e500mc only */
-#ifdef CONFIG_E500MC
+	/* e500mc only */
+	blt	2f
 	SET_IVOR(36, 0x280) /* Processor doorbell */
 	SET_IVOR(37, 0x2a0) /* Processor doorbell critical */
 	SET_IVOR(38, 0x2c0) /* Guest Processor doorbell */

@@ -54,6 +62,8 @@
 	SET_IVOR(40, 0x300) /* Hypervisor system call */
 	SET_IVOR(41, 0x320) /* Hypervisor Priviledge */
 

Quoted 12 lines

arch/powerpc/cpu/mpc85xx/fixed_ivor.S line 74

Quoted 10 lines

arch/powerpc/cpu/mpc85xx/fixed_ivor.S +3 -0

+#ifndef CONFIG_QEMU_E500
+	/* QEMU guests runs in guest mode and can't access GIVORs */
 	SET_GIVOR(2, 0x060) /* Guest Data Storage */
 	SET_GIVOR(3, 0x080) /* Guest Instruction Storage */
 	SET_GIVOR(4, 0x0a0) /* Guest External Input */

@@ -61,3 +71,4 @@
 	SET_GIVOR(13, 0x1c0) /* Guest Data TLB Error */
 	SET_GIVOR(14, 0x1e0) /* Guest Instruction TLB Error */
 #endif
+2:

Again, let's please just remove this entire file.

I can remove it, but it's a pretty drastic change from the original behavior that was introduced 4 1/2 years ago:

http://lists.denx.de/pipermail/u-boot/2009-August/058670.html

Quoted 3 lines

I assume the idea was to give OSs one thing less to worry about (IVOR setting). If any OS in between 2009 and now relied on that fact, we'll break it by removing this code.

Any such OS would already be broken on pre-2009 U-Boot. Linux doesn't rely on it. Neither the code nor the commit message gives a sufficient rationale, and Kumar didn't answer when asked. It's incomplete (doesn't include e6500 IVORs). It doesn't work on e6500 secondary threads. The

reset value of these registers is documented as zero, so it's not like we're just putting things back the way they were. It only happens (except on secondary cores) when the bootm/bootz commands are used, not when using the ELF loader or the go command, nor when using bootm with IH_TYPE_STANDALONE.

Does anyone know of an OS that actually depends on this?

-Scott

Quoted 77 lines · Scott Wood

Quoted 75 lines · Alexander Graf

Quoted 52 lines · Scott Wood

Quoted 51 lines · Alexander Graf

With the qemu-ppce500 machine type we can run the same board with either an e500v2 or an e500mc core plugged in.

This means that the IVOR setup can't be based on compile time decisions, so instead we have to do a runtime check which CPU generation we're running on.

Signed-off-by Alexander Graf <[email protected]>

---
arch/powerpc/cpu/mpc85xx/fixed_ivor.S | 21 ++++++++++++++++-----

1 file changed, 16 insertions(+), 5 deletions(-)

arch/powerpc/cpu/mpc85xx/fixed_ivor.S +13 -5

index ebbb8c0..635a97e 100644
@@ -36,17 +36,25 @@
 	SET_IVOR(14, 0x1e0) /* Instruction TLB Error */
 	SET_IVOR(15, 0x040) /* Debug */
 
-/* e500v1 & e500v2 only */
-#ifndef CONFIG_E500MC
+	/* Check for CPU */
+	mfpvr	r3
+	srwi	r3, r3, 16
+	/* Compare with e500mc PVR major number */
+	li	r4, 0
+	ori	r4, r4, 0x8023
+	cmpw	r3, r4
+
+	/* e500v1 & e500v2 only */
+	bge	1f
 	SET_IVOR(32, 0x200) /* SPE Unavailable */
 	SET_IVOR(33, 0x220) /* Embedded FP Data */
 	SET_IVOR(34, 0x240) /* Embedded FP Round */
-#endif
+1:
 
 	SET_IVOR(35, 0x260) /* Performance monitor */
 
-/* e500mc only */
-#ifdef CONFIG_E500MC
+	/* e500mc only */
+	blt	2f
 	SET_IVOR(36, 0x280) /* Processor doorbell */
 	SET_IVOR(37, 0x2a0) /* Processor doorbell critical */
 	SET_IVOR(38, 0x2c0) /* Guest Processor doorbell */

@@ -54,6 +62,8 @@
 	SET_IVOR(40, 0x300) /* Hypervisor system call */
 	SET_IVOR(41, 0x320) /* Hypervisor Priviledge */
 

Quoted 12 lines

arch/powerpc/cpu/mpc85xx/fixed_ivor.S line 74

Quoted 10 lines

arch/powerpc/cpu/mpc85xx/fixed_ivor.S +3 -0

+#ifndef CONFIG_QEMU_E500
+	/* QEMU guests runs in guest mode and can't access GIVORs */
 	SET_GIVOR(2, 0x060) /* Guest Data Storage */
 	SET_GIVOR(3, 0x080) /* Guest Instruction Storage */
 	SET_GIVOR(4, 0x0a0) /* Guest External Input */

@@ -61,3 +71,4 @@
 	SET_GIVOR(13, 0x1c0) /* Guest Data TLB Error */
 	SET_GIVOR(14, 0x1e0) /* Guest Instruction TLB Error */
 #endif
+2:

Again, let's please just remove this entire file.

I can remove it, but it's a pretty drastic change from the original behavior that was introduced 4 1/2 years ago:

http://lists.denx.de/pipermail/u-boot/2009-August/058670.html

I assume the idea was to give OSs one thing less to worry about (IVOR setting). If any OS in between 2009 and now relied on that fact, we'll break it by removing this code.

Quoted 4 lines

Any such OS would already be broken on pre-2009 U-Boot. Linux doesn't rely on it. Neither the code nor the commit message gives a sufficient rationale, and Kumar didn't answer when asked. It's incomplete (doesn't include e6500 IVORs). It doesn't work on e6500 secondary threads. The

I thought e6500 has hard coded IVORs which is the whole point of this exercise? Or am I wrong there?

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 <[email protected]>

---

v1 -> v2:

- Write device tree offset directly into global variable
- use r4 rather than r2 for that
- access fdt directly from in-memory copy
- remove unneeded header includes
- clean up pci enumeration
- coding style fixes
- populate and only use fdt_addr_r
- remove unused exported functions
- remove unused TLB0 entries
- make TLB1 I/O maps non-executable
- remove unused defines in board header
- make -kernel boot variables more clear
- remove TLB0 invalidation
- use tlb1.14 for temporary as=1 map
- use CONFIG_SYS_MPC85XX_NO_RESETVEC
- store fdt pointer in gd through cpu_init_early_f()
- replace fixup_tlb1() with dynamic TLB creation hook
---

arch/powerpc/cpu/mpc85xx/tlb.c +0 -0

 	return ;
 }
 

include/configs/qemu-ppce500.h new file +47 -0

new file mode 100644
index 0000000..1ebaf51
@@ -0,0 +1,209 @@
+/*
+ * Copyright 2011-2014 Freescale Semiconductor, Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+/*
+ * Corenet DS style board configuration file
+ */
+#ifndef __QEMU_PPCE500_H
+#define __QEMU_PPCE500_H
+
+#define CONFIG_CMD_REGINFO
+
+/* High Level Configuration Options */
+#define CONFIG_BOOKE
+#define CONFIG_E500			/* BOOKE e500 family */
+#define CONFIG_MPC85xx			/* MPC85xx/PQ3 platform */
+#define CONFIG_QEMU_E500
+
+#undef CONFIG_SYS_TEXT_BASE
+#define CONFIG_SYS_TEXT_BASE	0xf01000 /* 15 MB */
+
+#define CONFIG_SYS_MPC85XX_NO_RESETVEC
+
+#define CONFIG_SYS_RAMBOOT
+
+#define CONFIG_PCI			/* Enable PCI/PCIE */
+#define CONFIG_PCI1		1	/* PCI controller 1 */
+#define CONFIG_FSL_PCI_INIT		/* Use common FSL init code */
+#define CONFIG_SYS_PCI_64BIT		/* enable 64-bit PCI resources */
+
+#define CONFIG_ENV_OVERWRITE
+
+#define CONFIG_ENABLE_36BIT_PHYS
+
+#define CONFIG_ADDR_MAP
+#define CONFIG_SYS_NUM_ADDR_MAP		16	/* number of TLB1 entries */
+
+#define CONFIG_USE_DYNAMIC_TLBS
+
+#define CONFIG_SYS_MEMTEST_START	0x00200000	/* memtest works on */
+#define CONFIG_SYS_MEMTEST_END		0x00400000
+#define CONFIG_SYS_ALT_MEMTEST
+#define CONFIG_PANIC_HANG	/* do not reset board on panic */
+
+#define CONFIG_SYS_CCSRBAR		0xe0000000

include/configs/qemu-ppce500.h line 57

Quoted 8 lines · Alexander Graf

include/configs/qemu-ppce500.h new file +8 -0

+/*
+ * DDR Setup
+ */
+#define CONFIG_VERY_BIG_RAM
+#define CONFIG_SYS_DDR_SDRAM_BASE	0x00000000
+#define CONFIG_SYS_SDRAM_BASE		CONFIG_SYS_DDR_SDRAM_BASE
+
+#define CONFIG_CHIP_SELECTS_PER_CTRL	0

I don't know if the qemu has PCI, DDR, etc. Setting the above line to 0 will actually disable DDR controllers. Is that what you want?

include/configs/qemu-ppce500.h line 60

Quoted 3 lines

include/configs/qemu-ppce500.h new file +3 -0

+
+/* Get RAM size from device tree */
+#define CONFIG_DDR_SPD

You enabled SPD but I don't see the I2C address. Did you miss something, or you don't really use SPD?

board/freescale/qemu-ppce500/qemu-ppce500.c new file +9 -0

+void pci_init_board(void)
+{
+	struct fsl_pci_info pci_info;
+	const void *fdt = get_fdt();
+	int pci_node;
+
+	puts("\n");
+
+	pci_node = fdt_path_offset(fdt, "/pci");

board/freescale/qemu-ppce500/qemu-ppce500.c new file +9 -0

+void init_tlbs_dynamic(void)
+{
+	unsigned long fdt_tlb = (unsigned long)get_fdt() & ~0xffffful;
+	u32 mas0, mas1, mas2, mas3, mas7;
+	phys_size_t ram_size;
+
+	/*
+	 * Create a temporary AS=1 map for the fdt
+	 *

Quoted 14 lines · Alexander Graf

board/freescale/qemu-ppce500/qemu-ppce500.c new file +14 -0

+void pci_init_board(void)
+{
+	struct fsl_pci_info pci_info;
+	const void *fdt = get_fdt();
+	int pci_node;
+
+	puts("\n");
+
+	pci_node = fdt_path_offset(fdt, "/pci");
+	if (pci_node < 0) {
+		printf("PCI: disabled\n\n");
+		return;
+	}
+

board/freescale/qemu-ppce500/qemu-ppce500.c line 52

Quoted 5 lines

board/freescale/qemu-ppce500/qemu-ppce500.c new file +5 -0

+	SET_STD_PCI_INFO(pci_info, 1);
+
+	fsl_setup_hose(&pci1_hose, pci_info.regs);
+	printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
+		pci_info.regs);

Why hardcode these things in a message? Just don't print anything if you don't have the info.

Is QEMU's PCI emulation 32-bit-only?

Quoted 14 lines

board/freescale/qemu-ppce500/qemu-ppce500.c new file +14 -0

+void init_tlbs_dynamic(void)
+{
+	unsigned long fdt_tlb = (unsigned long)get_fdt() & ~0xffffful;
+	u32 mas0, mas1, mas2, mas3, mas7;
+	phys_size_t ram_size;
+
+	/*
+	 * Create a temporary AS=1 map for the fdt
+	 *
+	 * We use ESEL=0 here to overwrite the previous AS=0 map for ourselves
+	 * which was only 4k big. This way we don't have to clear any other maps.
+	 */
+	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(0);
+	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TS | MAS1_TSIZE(BOOKE_PAGESZ_1M);

board/freescale/qemu-ppce500/qemu-ppce500.c line 144

Quoted 5 lines

board/freescale/qemu-ppce500/qemu-ppce500.c new file +5 -0

+	mas2 = FSL_BOOKE_MAS2(fdt_tlb, 0);
+	mas3 = FSL_BOOKE_MAS3(fdt_tlb, 0, MAS3_SW|MAS3_SR|MAS3_SX);
+	mas7 = FSL_BOOKE_MAS7(fdt_tlb);
+
+	write_tlb(mas0, mas1, mas2, mas3, mas7);

How do you know you're not creating an overlapping TLB entry? You should map this to a fixed virtual address that you know is safe.

board/freescale/qemu-ppce500/qemu-ppce500.c line 157

Quoted 7 lines

board/freescale/qemu-ppce500/qemu-ppce500.c new file +6 -0

+	/* Create a dynamic AS=0 CCSRBAR mapping */
+	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(13);
+	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TSIZE(BOOKE_PAGESZ_1M);
+	mas2 = FSL_BOOKE_MAS2(CONFIG_SYS_CCSRBAR, MAS2_I|MAS2_G);
+	mas3 = FSL_BOOKE_MAS3(CONFIG_SYS_CCSRBAR_PHYS, 0, MAS3_SW|MAS3_SR);
+	mas7 = FSL_BOOKE_MAS7(CONFIG_SYS_CCSRBAR_PHYS);

CCSRBAR may be 1M or 16M (assuming qemu-ppce500 sticks with a CCSR-ish layout at all). Really we should be creating explicit maps for everything we find in the device tree that we care about.

include/configs/qemu-ppce500.h line 48

Quoted 3 lines

include/configs/qemu-ppce500.h new file +2 -0

+#define CONFIG_SYS_CCSRBAR		0xe0000000
+#define CONFIG_SYS_CCSRBAR_PHYS_LOW	CONFIG_SYS_CCSRBAR

Set CONFIG_SYS_CCSR_DO_NOT_RELOCATE instead of
CONFIG_SYS_CCSRBAR_PHYS_LOW.

include/configs/qemu-ppce500.h line 60

Quoted 3 lines

include/configs/qemu-ppce500.h new file +2 -0

+/* Get RAM size from device tree */
+#define CONFIG_DDR_SPD

That's not what CONFIG_DDR_SPD means.

Quoted 15 lines · Scott Wood

Quoted 14 lines · Alexander Graf

board/freescale/qemu-ppce500/qemu-ppce500.c new file +14 -0

+void pci_init_board(void)
+{
+	struct fsl_pci_info pci_info;
+	const void *fdt = get_fdt();
+	int pci_node;
+
+	puts("\n");
+
+	pci_node = fdt_path_offset(fdt, "/pci");
+	if (pci_node < 0) {
+		printf("PCI: disabled\n\n");
+		return;
+	}
+

Quoted 8 lines

board/freescale/qemu-ppce500/qemu-ppce500.c line 52

Quoted 5 lines

board/freescale/qemu-ppce500/qemu-ppce500.c new file +5 -0

+	SET_STD_PCI_INFO(pci_info, 1);
+
+	fsl_setup_hose(&pci1_hose, pci_info.regs);
+	printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
+		pci_info.regs);

Why hardcode these things in a message? Just don't print anything if you don't have the info.

To make it look more akin to a real e500 board. But I'll change it.

Quoted 14 lines

Quoted 14 lines

board/freescale/qemu-ppce500/qemu-ppce500.c new file +14 -0

+void init_tlbs_dynamic(void)
+{
+	unsigned long fdt_tlb = (unsigned long)get_fdt() & ~0xffffful;
+	u32 mas0, mas1, mas2, mas3, mas7;
+	phys_size_t ram_size;
+
+	/*
+	 * Create a temporary AS=1 map for the fdt
+	 *
+	 * We use ESEL=0 here to overwrite the previous AS=0 map for ourselves
+	 * which was only 4k big. This way we don't have to clear any other maps.
+	 */
+	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(0);
+	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TS | MAS1_TSIZE(BOOKE_PAGESZ_1M);

Quoted 8 lines

board/freescale/qemu-ppce500/qemu-ppce500.c line 144

Quoted 5 lines

board/freescale/qemu-ppce500/qemu-ppce500.c new file +5 -0

+	mas2 = FSL_BOOKE_MAS2(fdt_tlb, 0);
+	mas3 = FSL_BOOKE_MAS3(fdt_tlb, 0, MAS3_SW|MAS3_SR|MAS3_SX);
+	mas7 = FSL_BOOKE_MAS7(fdt_tlb);
+
+	write_tlb(mas0, mas1, mas2, mas3, mas7);

How do you know you're not creating an overlapping TLB entry? You should map this to a fixed virtual address that you know is safe.

Ok. I'll map it behind CCSRBAR.

Quoted 10 lines

board/freescale/qemu-ppce500/qemu-ppce500.c line 157

Quoted 6 lines

board/freescale/qemu-ppce500/qemu-ppce500.c new file +6 -0

+	/* Create a dynamic AS=0 CCSRBAR mapping */
+	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(13);
+	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TSIZE(BOOKE_PAGESZ_1M);
+	mas2 = FSL_BOOKE_MAS2(CONFIG_SYS_CCSRBAR, MAS2_I|MAS2_G);
+	mas3 = FSL_BOOKE_MAS3(CONFIG_SYS_CCSRBAR_PHYS, 0, MAS3_SW|MAS3_SR);
+	mas7 = FSL_BOOKE_MAS7(CONFIG_SYS_CCSRBAR_PHYS);

CCSRBAR may be 1M or 16M (assuming qemu-ppce500 sticks with a CCSR-ish layout at all). Really we should be creating explicit maps for everything we find in the device tree that we care about.

I don't understand that part. We do create explicit maps for everything we care about, no?

Quoted 26 lines · Alexander Graf

Quoted 23 lines · Scott Wood

Quoted 9 lines · Alexander Graf

board/freescale/qemu-ppce500/qemu-ppce500.c new file +9 -0

+void pci_init_board(void)
+{
+	struct fsl_pci_info pci_info;
+	const void *fdt = get_fdt();
+	int pci_node;
+
+	puts("\n");
+
+	pci_node = fdt_path_offset(fdt, "/pci");

board/freescale/qemu-ppce500/qemu-ppce500.c line 52

Quoted 10 lines

board/freescale/qemu-ppce500/qemu-ppce500.c new file +10 -0

+	if (pci_node < 0) {
+		printf("PCI: disabled\n\n");
+		return;
+	}
+
+	SET_STD_PCI_INFO(pci_info, 1);
+
+	fsl_setup_hose(&pci1_hose, pci_info.regs);
+	printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
+		pci_info.regs);

Why hardcode these things in a message? Just don't print anything if you don't have the info.

Quoted 1 line

To make it look more akin to a real e500 board. But I'll change it.

It's a paravirt target...

Quoted 24 lines

Quoted 23 lines

Quoted 10 lines

board/freescale/qemu-ppce500/qemu-ppce500.c new file +9 -0

+void init_tlbs_dynamic(void)
+{
+	unsigned long fdt_tlb = (unsigned long)get_fdt() & ~0xffffful;
+	u32 mas0, mas1, mas2, mas3, mas7;
+	phys_size_t ram_size;
+
+	/*
+	 * Create a temporary AS=1 map for the fdt
+	 *

board/freescale/qemu-ppce500/qemu-ppce500.c line 144

Quoted 10 lines

board/freescale/qemu-ppce500/qemu-ppce500.c new file +10 -0

+	 * We use ESEL=0 here to overwrite the previous AS=0 map for ourselves
+	 * which was only 4k big. This way we don't have to clear any other maps.
+	 */
+	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(0);
+	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TS | MAS1_TSIZE(BOOKE_PAGESZ_1M);
+	mas2 = FSL_BOOKE_MAS2(fdt_tlb, 0);
+	mas3 = FSL_BOOKE_MAS3(fdt_tlb, 0, MAS3_SW|MAS3_SR|MAS3_SX);
+	mas7 = FSL_BOOKE_MAS7(fdt_tlb);
+
+	write_tlb(mas0, mas1, mas2, mas3, mas7);

How do you know you're not creating an overlapping TLB entry? You should map this to a fixed virtual address that you know is safe.

Quoted 1 line

Ok. I'll map it behind CCSRBAR.

I'd give it its own CONFIG symbol to be explicitly located in the header file.

Quoted 12 lines

Quoted 11 lines

board/freescale/qemu-ppce500/qemu-ppce500.c line 157

Quoted 7 lines

board/freescale/qemu-ppce500/qemu-ppce500.c new file +6 -0

+	/* Create a dynamic AS=0 CCSRBAR mapping */
+	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(13);
+	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TSIZE(BOOKE_PAGESZ_1M);
+	mas2 = FSL_BOOKE_MAS2(CONFIG_SYS_CCSRBAR, MAS2_I|MAS2_G);
+	mas3 = FSL_BOOKE_MAS3(CONFIG_SYS_CCSRBAR_PHYS, 0, MAS3_SW|MAS3_SR);
+	mas7 = FSL_BOOKE_MAS7(CONFIG_SYS_CCSRBAR_PHYS);

CCSRBAR may be 1M or 16M (assuming qemu-ppce500 sticks with a CCSR-ish layout at all). Really we should be creating explicit maps for everything we find in the device tree that we care about.

Quoted 1 line

I don't understand that part. We do create explicit maps for everything we care about, no?

No, you map CCSR as a whole, rather than the UART, PCI controller regs, etc. If you want to that, ideally the size of CCSR should come from the device tree as well as the address.

Quoted 27 lines · Scott Wood

Quoted 26 lines · Alexander Graf

Quoted 23 lines · Scott Wood

Quoted 9 lines · Alexander Graf

board/freescale/qemu-ppce500/qemu-ppce500.c new file +9 -0

+void pci_init_board(void)
+{
+	struct fsl_pci_info pci_info;
+	const void *fdt = get_fdt();
+	int pci_node;
+
+	puts("\n");
+
+	pci_node = fdt_path_offset(fdt, "/pci");

board/freescale/qemu-ppce500/qemu-ppce500.c line 52

Quoted 10 lines

board/freescale/qemu-ppce500/qemu-ppce500.c new file +10 -0

+	if (pci_node < 0) {
+		printf("PCI: disabled\n\n");
+		return;
+	}
+
+	SET_STD_PCI_INFO(pci_info, 1);
+
+	fsl_setup_hose(&pci1_hose, pci_info.regs);
+	printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
+		pci_info.regs);

Why hardcode these things in a message? Just don't print anything if you don't have the info.

Quoted 3 lines

Quoted 1 line

To make it look more akin to a real e500 board. But I'll change it.

It's a paravirt target...

So? It should still look "normal" to someone who runs it.

Quoted 7 lines

Quoted 4 lines

Quoted 2 lines

How do you know you're not creating an overlapping TLB entry? You should map this to a fixed virtual address that you know is safe.

Ok. I'll map it behind CCSRBAR.

I'd give it its own CONFIG symbol to be explicitly located in the header file.

It's only a temporary map that is alive for a few dozen instructions. I think a new CONFIG symbol is excessive here :).

The definition of our ppce500 PV machine is that every address is dynamically determined through device tree bindings.

So don't hardcode where CCSR is in our physical memory layout but instead read it dynamically from the device tree we get passed on boot.

Signed-off-by Alexander Graf <[email protected]>

---
 arch/powerpc/cpu/mpc85xx/start.S            |    2 +
 board/freescale/qemu-ppce500/qemu-ppce500.c |   83 +++++++++++++++++++++++++++
 include/configs/qemu-ppce500.h              |   12 +++-
 3 files changed, 96 insertions(+), 1 deletion(-)

arch/powerpc/cpu/mpc85xx/start.S +2 -0

index 0e593d2..2672c20 100644
@@ -519,6 +519,7 @@ nexti:	mflr	r1		/* R1 = our PC */
  * As a general rule, TLB0 is used for short-term TLBs, and TLB1 is used for
  * long-term TLBs, so we use TLB0 here.
  */
+#if !defined(CONFIG_DYNAMIC_CCSRBAR)
 #if (CONFIG_SYS_CCSRBAR_DEFAULT != CONFIG_SYS_CCSRBAR_PHYS)
 
 #if !defined(CONFIG_SYS_CCSRBAR_PHYS_HIGH) || !defined(CONFIG_SYS_CCSRBAR_PHYS_LOW)

@@ -703,6 +704,7 @@ delete_temp_tlbs:
 	delete_tlb0_entry 1, CONFIG_SYS_CCSRBAR + 0x1000, MAS2_I|MAS2_G, r3
 
 #endif /* #if (CONFIG_SYS_CCSRBAR_DEFAULT != CONFIG_SYS_CCSRBAR_PHYS) */
+#endif /* #if !defined(CONFIG_DYNAMIC_CCSRBAR) */
 
 #if defined(CONFIG_SYS_FSL_QORIQ_CHASSIS2) && defined(CONFIG_E6500)
 create_ccsr_l2_tlb:

board/freescale/qemu-ppce500/qemu-ppce500.c +76 -0

index b33b6b1..6491ae9 100644
@@ -26,6 +26,89 @@ static const void *get_fdt(void)
 	return gd->fdt_blob;
 }
 
+uint64_t get_phys_ccsrbar_addr(void)
+{
+	const void *fdt = get_fdt();
+	int root_node = fdt_path_offset(fdt, "/");
+	int soc_node = fdt_path_offset(fdt, "/soc");
+	const uint32_t *prop;
+	const uint64_t *prop64;
+	int len;
+	int root_address_cells = 1;
+	int address_cells = 1;
+	uint64_t r = 0;
+
+	/* Read CCSRBAR address length and size from device tree */
+	prop = fdt_getprop(fdt, root_node, "#address-cells", &len);
+	if (prop && (len >= 4))
+		root_address_cells = prop[0];
+
+	prop = fdt_getprop(fdt, soc_node, "#address-cells", &len);
+	if (prop && (len >= 4))
+		address_cells = prop[0];
+
+	/* Read CCSRBAR address from device tree */
+	prop = fdt_getprop(fdt, soc_node, "ranges", &len);
+	if (prop && (len >= ((address_cells + root_address_cells) * 4))) {
+		/* The physical address starts after the child's offset */
+		prop += address_cells;
+		prop64 = (const uint64_t *)prop;
+
+		if (root_address_cells == 1)
+			r = prop[0];
+		else if (root_address_cells == 2)
+			r = prop64[0];
+	}
+
+	if (!r)
+		panic("Couldn't find CCSR in FDT");
+
+	return r;
+}
+
+uint64_t get_phys_ccsrbar_addr_early(void)
+{
+	u32 mas0, mas1, mas2, mas3, mas7;
+	ulong fdt = (ulong)get_fdt();
+	uint64_t r;
+
+	/*
+	 * To be able to read the FDT we need to create a temporary TLB
+	 * map for it.
+	 */
+
+	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(10);
+	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TS | MAS1_TSIZE(BOOKE_PAGESZ_1M);
+	mas2 = FSL_BOOKE_MAS2(fdt, 0);
+	mas3 = FSL_BOOKE_MAS3(fdt, 0, MAS3_SW|MAS3_SR);
+	mas7 = FSL_BOOKE_MAS7(fdt);
+
+	write_tlb(mas0, mas1, mas2, mas3, mas7);
+
+	r = get_phys_ccsrbar_addr();
+
+	disable_tlb(10);
+
+	return r;
+}
+
+int board_early_init_f(void)
+{
+	u32 mas0, mas1, mas2, mas3, mas7;
+	uint64_t phys_ccsr_address = get_phys_ccsrbar_addr();
+
+	/* Extend the CCSR map from cpu_init_early_f() */
+	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(13);
+	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TSIZE(BOOKE_PAGESZ_64M);
+	mas2 = FSL_BOOKE_MAS2(CONFIG_SYS_CCSRBAR, MAS2_I|MAS2_G);
+	mas3 = FSL_BOOKE_MAS3(phys_ccsr_address, 0, MAS3_SW|MAS3_SR);

include/configs/qemu-ppce500.h +4 -0

index 1ebaf51..7ef7235 100644
@@ -44,8 +44,18 @@
 #define CONFIG_SYS_ALT_MEMTEST
 #define CONFIG_PANIC_HANG	/* do not reset board on panic */
 
+/* Needed to fill the ccsrbar pointer */
+#define CONFIG_BOARD_EARLY_INIT_F
+
+/* Virtual address to CCSRBAR */

Quoted 122 lines · Alexander Graf

The definition of our ppce500 PV machine is that every address is dynamically determined through device tree bindings.

So don't hardcode where CCSR is in our physical memory layout but instead read it dynamically from the device tree we get passed on boot.

Signed-off-by Alexander Graf <[email protected]>

---

arch/powerpc/cpu/mpc85xx/start.S            |    2 +
board/freescale/qemu-ppce500/qemu-ppce500.c |   83 +++++++++++++++++++++++++++
include/configs/qemu-ppce500.h              |   12 +++-
3 files changed, 96 insertions(+), 1 deletion(-)

arch/powerpc/cpu/mpc85xx/start.S +2 -0

index 0e593d2..2672c20 100644
@@ -519,6 +519,7 @@ nexti:	mflr	r1		/* R1 = our PC */
 * As a general rule, TLB0 is used for short-term TLBs, and TLB1 is used for
 * long-term TLBs, so we use TLB0 here.
 */
+#if !defined(CONFIG_DYNAMIC_CCSRBAR)
 #if (CONFIG_SYS_CCSRBAR_DEFAULT != CONFIG_SYS_CCSRBAR_PHYS)
 
 #if !defined(CONFIG_SYS_CCSRBAR_PHYS_HIGH) || !defined(CONFIG_SYS_CCSRBAR_PHYS_LOW)

@@ -703,6 +704,7 @@ delete_temp_tlbs:
 	delete_tlb0_entry 1, CONFIG_SYS_CCSRBAR + 0x1000, MAS2_I|MAS2_G, r3
 
 #endif /* #if (CONFIG_SYS_CCSRBAR_DEFAULT != CONFIG_SYS_CCSRBAR_PHYS) */
+#endif /* #if !defined(CONFIG_DYNAMIC_CCSRBAR) */
 
 #if defined(CONFIG_SYS_FSL_QORIQ_CHASSIS2) && defined(CONFIG_E6500)

create_ccsr_l2_tlb:

board/freescale/qemu-ppce500/qemu-ppce500.c +81 -0

index b33b6b1..6491ae9 100644
@@ -26,6 +26,89 @@ static const void *get_fdt(void)
 	return gd->fdt_blob;
 }
 
+uint64_t get_phys_ccsrbar_addr(void)
+{
+	const void *fdt = get_fdt();
+	int root_node = fdt_path_offset(fdt, "/");
+	int soc_node = fdt_path_offset(fdt, "/soc");
+	const uint32_t *prop;
+	const uint64_t *prop64;
+	int len;
+	int root_address_cells = 1;
+	int address_cells = 1;
+	uint64_t r = 0;
+
+	/* Read CCSRBAR address length and size from device tree */
+	prop = fdt_getprop(fdt, root_node, "#address-cells", &len);
+	if (prop && (len >= 4))
+		root_address_cells = prop[0];
+
+	prop = fdt_getprop(fdt, soc_node, "#address-cells", &len);
+	if (prop && (len >= 4))
+		address_cells = prop[0];
+
+	/* Read CCSRBAR address from device tree */
+	prop = fdt_getprop(fdt, soc_node, "ranges", &len);
+	if (prop && (len >= ((address_cells + root_address_cells) * 4))) {
+		/* The physical address starts after the child's offset */
+		prop += address_cells;
+		prop64 = (const uint64_t *)prop;
+
+		if (root_address_cells == 1)
+			r = prop[0];
+		else if (root_address_cells == 2)
+			r = prop64[0];
+	}
+
+	if (!r)
+		panic("Couldn't find CCSR in FDT");
+
+	return r;
+}
+
+uint64_t get_phys_ccsrbar_addr_early(void)
+{
+	u32 mas0, mas1, mas2, mas3, mas7;
+	ulong fdt = (ulong)get_fdt();
+	uint64_t r;
+
+	/*
+	 * To be able to read the FDT we need to create a temporary TLB
+	 * map for it.
+	 */
+
+	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(10);
+	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TS | MAS1_TSIZE(BOOKE_PAGESZ_1M);
+	mas2 = FSL_BOOKE_MAS2(fdt, 0);
+	mas3 = FSL_BOOKE_MAS3(fdt, 0, MAS3_SW|MAS3_SR);
+	mas7 = FSL_BOOKE_MAS7(fdt);
+
+	write_tlb(mas0, mas1, mas2, mas3, mas7);
+
+	r = get_phys_ccsrbar_addr();
+
+	disable_tlb(10);
+
+	return r;
+}
+
+int board_early_init_f(void)
+{
+	u32 mas0, mas1, mas2, mas3, mas7;
+	uint64_t phys_ccsr_address = get_phys_ccsrbar_addr();
+
+	/* Extend the CCSR map from cpu_init_early_f() */
+	mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(13);
+	mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TSIZE(BOOKE_PAGESZ_64M);
+	mas2 = FSL_BOOKE_MAS2(CONFIG_SYS_CCSRBAR, MAS2_I|MAS2_G);
+	mas3 = FSL_BOOKE_MAS3(phys_ccsr_address, 0, MAS3_SW|MAS3_SR);
+	mas7 = FSL_BOOKE_MAS7(phys_ccsr_address);
+
+	write_tlb(mas0, mas1, mas2, mas3, mas7);
+
+	return 0;

board/freescale/qemu-ppce500/qemu-ppce500.c static const void *get_fdt(void) · line 114

Quoted 5 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +2 -0

+}
+
 int checkboard(void)
 {
 	return 0;

You're adding new code to map CCSR and FDT, but not removing the code to do the same thing you added in patch 3/6.

Why 64M?

Quoted 20 lines

include/configs/qemu-ppce500.h +9 -1

index 1ebaf51..7ef7235 100644
@@ -44,8 +44,18 @@
 #define CONFIG_SYS_ALT_MEMTEST
 #define CONFIG_PANIC_HANG	/* do not reset board on panic */
 
+/* Needed to fill the ccsrbar pointer */
+#define CONFIG_BOARD_EARLY_INIT_F
+
+/* Virtual address to CCSRBAR */
 #define CONFIG_SYS_CCSRBAR		0xe0000000
-#define CONFIG_SYS_CCSRBAR_PHYS_LOW	CONFIG_SYS_CCSRBAR
+/* Physical address should be a function call */
+#ifndef __ASSEMBLY__
+extern unsigned long long get_phys_ccsrbar_addr_early(void);
+#endif
+#define CONFIG_SYS_CCSRBAR_PHYS_LOW	((uint32_t)get_phys_ccsrbar_addr_early())

include/configs/qemu-ppce500.h line 59

Quoted 3 lines

include/configs/qemu-ppce500.h +2 -0

+#define CONFIG_SYS_CCSRBAR_PHYS_HIGH	(get_phys_ccsrbar_addr_early() >> 32)
+#define CONFIG_DYNAMIC_CCSRBAR
 

What cares about CONFIG_SYS_CCSRBAR_PHYS* other than the code you skipped with an ifndef in start.S, and related header ifdeffery (which could be skipped by the existing CONFIG_SYS_CCSR_DO_NOT_RELOCATE instead)?

The definition of our ppce500 PV machine is that every address is dynamically determined through device tree bindings.

So don't hardcode where PCI devices are in our physical memory layout but instead read them dynamically from the device tree we get passed on boot.

Signed-off-by Alexander Graf <[email protected]>

---
 board/freescale/qemu-ppce500/qemu-ppce500.c |  193 ++++++++++++++++++++++++---
 board/freescale/qemu-ppce500/tlb.c          |   13 --
 include/configs/qemu-ppce500.h              |   12 --
 3 files changed, 175 insertions(+), 43 deletions(-)

board/freescale/qemu-ppce500/qemu-ppce500.c +0 -0

index 6491ae9..5d4dd64 100644

board/freescale/qemu-ppce500/qemu-ppce500.c +11 -0

+{
+	int len;
+	const uint32_t *prop;
+
+	prop = fdt_getprop(fdt, node, property, &len);
+
+	if (!prop)
+		return defval;
+
+	if (len < ((off + num) * sizeof(uint32_t)))
+		panic("Invalid fdt");

board/freescale/qemu-ppce500/qemu-ppce500.c +15 -0

+static void map_tlb1_io(ulong virt_addr, uint64_t phys_addr, uint64_t size)
+{
+	unsigned int max_cam, tsize_mask;
+	int i;
+
+	if ((mfspr(SPRN_MMUCFG) & MMUCFG_MAVN) == MMUCFG_MAVN_V1) {
+		/* Convert (4^max) kB to (2^max) bytes */
+		max_cam = ((mfspr(SPRN_TLB1CFG) >> 16) & 0xf) * 2 + 10;
+		tsize_mask = ~1U;
+	} else {
+		/* Convert (2^max) kB to (2^max) bytes */
+		max_cam = __ilog2(mfspr(SPRN_TLB1PS)) + 10;
+		tsize_mask = ~0U;
+	}
+

board/freescale/qemu-ppce500/qemu-ppce500.c +73 -10

+
 void pci_init_board(void)
 {
-	struct fsl_pci_info pci_info;
+	struct pci_controller *pci_hoses;
 	const void *fdt = get_fdt();
 	int pci_node;
+	int pci_num = 0;
+	int pci_count;
+	const char *compat = "fsl,mpc8540-pci";
+	ulong map_addr;
 
 	puts("\n");
 
-	pci_node = fdt_path_offset(fdt, "/pci");
-	if (pci_node < 0) {
+	/* Start MMIO and PIO range maps above RAM */
+	map_addr = CONFIG_MAX_MEM_MAPPED;
+
+	/* Count and allocate PCI buses */
+	pci_count = myfdt_count_compatibles(fdt, compat);
+
+	if (pci_count) {
+		pci_hoses = malloc(sizeof(struct pci_controller) * pci_count);
+	} else {
 		printf("PCI: disabled\n\n");
 		return;
 	}
 
-	SET_STD_PCI_INFO(pci_info, 1);
-
-	fsl_setup_hose(&pci1_hose, pci_info.regs);
-	printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
-		pci_info.regs);
-
-	fsl_pci_init_port(&pci_info, &pci1_hose, 0);
+	/* Spawn PCI buses based on device tree */
+	pci_node = fdt_node_offset_by_compatible(fdt, -1, compat);
+	while (pci_node != -FDT_ERR_NOTFOUND) {
+		struct fsl_pci_info pci_info = { };
+		uint64_t phys_addr;
+		int pnode = fdt_parent_offset(fdt, pci_node);
+		int paddress_cells;
+		int address_cells;
+		int size_cells;
+		int off = 0;
+
+		paddress_cells = myfdt_one_cell(fdt, pnode, "#address-cells", 1);
+		address_cells = myfdt_one_cell(fdt, pci_node, "#address-cells", 1);
+		size_cells = myfdt_one_cell(fdt, pci_node, "#size-cells", 1);
+
+		pci_info.regs = myfdt_readcells(fdt, pci_node, "reg",
+						paddress_cells, 0, 0);
+
+		/* MMIO range */
+		off += address_cells;
+		phys_addr = myfdt_readcells(fdt, pci_node, "ranges",
+					    paddress_cells, off, 0);
+		off += paddress_cells;
+		pci_info.mem_size = myfdt_readcells(fdt, pci_node, "ranges",
+			size_cells, off, 0);
+		off += size_cells;
+
+		/* Align virtual region */
+		map_addr += pci_info.mem_size - 1;
+		map_addr &= ~(pci_info.mem_size - 1);
+		/* Map virtual memory for MMIO range */
+		map_tlb1_io(map_addr, phys_addr, pci_info.mem_size);
+		pci_info.mem_bus = phys_addr;
+		pci_info.mem_phys = phys_addr;
+		map_addr += pci_info.mem_size;
+
+		/* PIO range */
+		off += address_cells;
+		pci_info.io_phys = myfdt_readcells(fdt, pci_node, "ranges",
+			paddress_cells, off, 0);
+		off += paddress_cells;
+		pci_info.io_size = myfdt_readcells(fdt, pci_node, "ranges",
+			size_cells, off, 0);
+
+		/* Align virtual region */
+		map_addr += pci_info.io_size - 1;
+		map_addr &= ~(pci_info.io_size - 1);
+		/* Map virtual memory for MMIO range */
+		map_tlb1_io(map_addr, pci_info.io_phys, pci_info.io_size);
+		pci_info.io_bus = map_addr;
+		map_addr += pci_info.io_size;
+
+		/* Instantiate */
+		pci_info.pci_num = pci_num + 1;
+
+		fsl_setup_hose(&pci_hoses[pci_num], pci_info.regs);
+		printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
+			pci_info.regs);

Quoted 5 lines · Alexander Graf

The definition of our ppce500 PV machine is that every address is dynamically determined through device tree bindings.

So don't hardcode where PCI devices are in our physical memory layout but instead read them dynamically from the device tree we get passed on boot.

Would it be difficult to make the QEMU emulation properly implement access windows?

Quoted 15 lines

Signed-off-by Alexander Graf <[email protected]>

---

board/freescale/qemu-ppce500/qemu-ppce500.c |  193 ++++++++++++++++++++++++---
board/freescale/qemu-ppce500/tlb.c          |   13 --
include/configs/qemu-ppce500.h              |   12 --
3 files changed, 175 insertions(+), 43 deletions(-)

board/freescale/qemu-ppce500/qemu-ppce500.c +0 -0

index 6491ae9..5d4dd64 100644
@@ -19,7 +19,51 @@
 #include <malloc.h>
 

board/freescale/qemu-ppce500/qemu-ppce500.c line 24

Quoted 5 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +3 -1

 DECLARE_GLOBAL_DATA_PTR;
-static struct pci_controller pci1_hose;
+
+static uint64_t myfdt_readcells(const void *fdt, int node, const char *property,
+				int num, int off, uint64_t defval)

"my" fdt?

Quoted 16 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +15 -0

+{
+	int len;
+	const uint32_t *prop;
+
+	prop = fdt_getprop(fdt, node, property, &len);
+
+	if (!prop)
+		return defval;
+
+	if (len < ((off + num) * sizeof(uint32_t)))
+		panic("Invalid fdt");
+
+	prop += off;
+
+	switch (num) {

board/freescale/qemu-ppce500/qemu-ppce500.c line 45

Quoted 6 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +6 -0

+	case 1:
+		return *prop;
+	case 2:
+		return *(const uint64_t *)prop;
+	}
+

What about this function is specific to qemu-ppce500?

(quoted diff) +2 -0

+	panic("Invalid cell size");
+}

s/cell size/cell count/

board/freescale/qemu-ppce500/qemu-ppce500.c line 53

Quoted 5 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +5 -0

+static uint32_t myfdt_one_cell(const void *fdt, int node, const char *property,
+			       uint32_t defval)
+{
+	return myfdt_readcells(fdt, node, property, 1, 0, defval);
+}

This looks a lot like fdt_getprop_u32_default(), except for "int node" versus path.

Quoted 23 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +22 -0

+static void map_tlb1_io(ulong virt_addr, uint64_t phys_addr, uint64_t size)
+{
+	unsigned int max_cam, tsize_mask;
+	int i;
+
+	if ((mfspr(SPRN_MMUCFG) & MMUCFG_MAVN) == MMUCFG_MAVN_V1) {
+		/* Convert (4^max) kB to (2^max) bytes */
+		max_cam = ((mfspr(SPRN_TLB1CFG) >> 16) & 0xf) * 2 + 10;
+		tsize_mask = ~1U;
+	} else {
+		/* Convert (2^max) kB to (2^max) bytes */
+		max_cam = __ilog2(mfspr(SPRN_TLB1PS)) + 10;
+		tsize_mask = ~0U;
+	}
+
+	for (i = 0; size && i < 8; i++) {
+		int tlb_index = find_free_tlbcam();
+		u32 camsize = __ilog2_u64(size) & tsize_mask;
+		u32 align = __ilog2(virt_addr) & tsize_mask;
+		unsigned int tlb_size;
+
+		if (tlb_index == -1)

board/freescale/qemu-ppce500/qemu-ppce500.c int checkboard(void) · line 181

Quoted 3 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +3 -0

+			break;
+
+		if (align == -2) align = max_cam;

-2? Besides align being unsigned, if this is meant to handle the case where virt_addr is zero, that's undefined for __ilog2() (don't rely on it being implemented with cntlzw), and you're not handling the MMUv2 case.

Plus, whitespace.

board/freescale/qemu-ppce500/qemu-ppce500.c int checkboard(void) · line 186

Quoted 5 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +5 -0

+		if (camsize > align)
+			camsize = align;
+
+		if (camsize > max_cam)
+			camsize = max_cam;

What about min_cam?

board/freescale/qemu-ppce500/qemu-ppce500.c line 47

Quoted 3 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +3 -0

+	}
+
+}

Whitespace.

Quoted 102 lines

+
void pci_init_board(void)
{

board/freescale/qemu-ppce500/qemu-ppce500.c +1 -1

-	struct fsl_pci_info pci_info;
+	struct pci_controller *pci_hoses;

	const void *fdt = get_fdt();
	int pci_node;

board/freescale/qemu-ppce500/qemu-ppce500.c +4 -0

+	int pci_num = 0;
+	int pci_count;
+	const char *compat = "fsl,mpc8540-pci";
+	ulong map_addr;

	puts("\n");

board/freescale/qemu-ppce500/qemu-ppce500.c +9 -2

-	pci_node = fdt_path_offset(fdt, "/pci");
-	if (pci_node < 0) {
+	/* Start MMIO and PIO range maps above RAM */
+	map_addr = CONFIG_MAX_MEM_MAPPED;
+
+	/* Count and allocate PCI buses */
+	pci_count = myfdt_count_compatibles(fdt, compat);
+
+	if (pci_count) {
+		pci_hoses = malloc(sizeof(struct pci_controller) * pci_count);
+	} else {

		printf("PCI: disabled\n\n");
		return;
	}

board/freescale/qemu-ppce500/qemu-ppce500.c +65 -7

-	SET_STD_PCI_INFO(pci_info, 1);
-
-	fsl_setup_hose(&pci1_hose, pci_info.regs);
-	printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
-		pci_info.regs);
-
-	fsl_pci_init_port(&pci_info, &pci1_hose, 0);
+	/* Spawn PCI buses based on device tree */
+	pci_node = fdt_node_offset_by_compatible(fdt, -1, compat);
+	while (pci_node != -FDT_ERR_NOTFOUND) {
+		struct fsl_pci_info pci_info = { };
+		uint64_t phys_addr;
+		int pnode = fdt_parent_offset(fdt, pci_node);
+		int paddress_cells;
+		int address_cells;
+		int size_cells;
+		int off = 0;
+
+		paddress_cells = myfdt_one_cell(fdt, pnode, "#address-cells", 1);
+		address_cells = myfdt_one_cell(fdt, pci_node, "#address-cells", 1);
+		size_cells = myfdt_one_cell(fdt, pci_node, "#size-cells", 1);
+
+		pci_info.regs = myfdt_readcells(fdt, pci_node, "reg",
+						paddress_cells, 0, 0);
+
+		/* MMIO range */
+		off += address_cells;
+		phys_addr = myfdt_readcells(fdt, pci_node, "ranges",
+					    paddress_cells, off, 0);
+		off += paddress_cells;
+		pci_info.mem_size = myfdt_readcells(fdt, pci_node, "ranges",
+			size_cells, off, 0);
+		off += size_cells;
+
+		/* Align virtual region */
+		map_addr += pci_info.mem_size - 1;
+		map_addr &= ~(pci_info.mem_size - 1);
+		/* Map virtual memory for MMIO range */
+		map_tlb1_io(map_addr, phys_addr, pci_info.mem_size);
+		pci_info.mem_bus = phys_addr;
+		pci_info.mem_phys = phys_addr;
+		map_addr += pci_info.mem_size;
+
+		/* PIO range */
+		off += address_cells;
+		pci_info.io_phys = myfdt_readcells(fdt, pci_node, "ranges",
+			paddress_cells, off, 0);
+		off += paddress_cells;
+		pci_info.io_size = myfdt_readcells(fdt, pci_node, "ranges",
+			size_cells, off, 0);
+
+		/* Align virtual region */
+		map_addr += pci_info.io_size - 1;
+		map_addr &= ~(pci_info.io_size - 1);
+		/* Map virtual memory for MMIO range */
+		map_tlb1_io(map_addr, pci_info.io_phys, pci_info.io_size);
+		pci_info.io_bus = map_addr;
+		map_addr += pci_info.io_size;
+
+		/* Instantiate */
+		pci_info.pci_num = pci_num + 1;
+
+		fsl_setup_hose(&pci_hoses[pci_num], pci_info.regs);
+		printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
+			pci_info.regs);
+
+		fsl_pci_init_port(&pci_info, &pci_hoses[pci_num], pci_num);
+
+		/* Jump to next PCI node */
+		pci_node = fdt_node_offset_by_compatible(fdt, pci_node, compat);
+		pci_num++;
+	}

Quoted 2 lines

	puts("\n");
}

How about using fdt_translate_address() or other existing functionality?

Quoted 4 lines · Scott Wood

Quoted 3 lines · Alexander Graf

The definition of our ppce500 PV machine is that every address is dynamically determined through device tree bindings.

Quoted 5 lines

Quoted 2 lines

So don't hardcode where PCI devices are in our physical memory layout but instead read them dynamically from the device tree we get passed on boot.

Would it be difficult to make the QEMU emulation properly implement access windows?

What are access windows? You mean BAR region offsets? Not too hard I suppose, but it adds complexity which we were trying to avoid, no?

Quoted 15 lines

Quoted 15 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +15 -0

+{
+	int len;
+	const uint32_t *prop;
+
+	prop = fdt_getprop(fdt, node, property, &len);
+
+	if (!prop)
+		return defval;
+
+	if (len < ((off + num) * sizeof(uint32_t)))
+		panic("Invalid fdt");
+
+	prop += off;
+
+	switch (num) {

Quoted 8 lines

board/freescale/qemu-ppce500/qemu-ppce500.c line 45

Quoted 6 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +6 -0

+	case 1:
+		return *prop;
+	case 2:
+		return *(const uint64_t *)prop;
+	}
+

What about this function is specific to qemu-ppce500?

Nothing. But the less common code I touch the less I can break.

There seems to be an fdt helper framework that's only targeted at a few ARM
devices - not sure what to make of that.

Quoted 5 lines

(quoted diff) +2 -0

+	panic("Invalid cell size");
+}

s/cell size/cell count/

Quoted 8 lines

board/freescale/qemu-ppce500/qemu-ppce500.c line 53

Quoted 5 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +5 -0

+static uint32_t myfdt_one_cell(const void *fdt, int node, const char *property,
+			       uint32_t defval)
+{
+	return myfdt_readcells(fdt, node, property, 1, 0, defval);
+}

This looks a lot like fdt_getprop_u32_default(), except for "int node" versus path.

Well, I use it inside of a loop where I don't have the path :).

Quoted 22 lines

Quoted 22 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +22 -0

+static void map_tlb1_io(ulong virt_addr, uint64_t phys_addr, uint64_t size)
+{
+	unsigned int max_cam, tsize_mask;
+	int i;
+
+	if ((mfspr(SPRN_MMUCFG) & MMUCFG_MAVN) == MMUCFG_MAVN_V1) {
+		/* Convert (4^max) kB to (2^max) bytes */
+		max_cam = ((mfspr(SPRN_TLB1CFG) >> 16) & 0xf) * 2 + 10;
+		tsize_mask = ~1U;
+	} else {
+		/* Convert (2^max) kB to (2^max) bytes */
+		max_cam = __ilog2(mfspr(SPRN_TLB1PS)) + 10;
+		tsize_mask = ~0U;
+	}
+
+	for (i = 0; size && i < 8; i++) {
+		int tlb_index = find_free_tlbcam();
+		u32 camsize = __ilog2_u64(size) & tsize_mask;
+		u32 align = __ilog2(virt_addr) & tsize_mask;
+		unsigned int tlb_size;
+
+		if (tlb_index == -1)

Quoted 8 lines

board/freescale/qemu-ppce500/qemu-ppce500.c int checkboard(void) · line 181

Quoted 3 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +3 -0

+			break;
+
+		if (align == -2) align = max_cam;

-2? Besides align being unsigned, if this is meant to handle the case where virt_addr is zero, that's undefined for __ilog2() (don't rely on it being implemented with cntlzw), and you're not handling the MMUv2 case.

I merely copied this from tlb.c's setup_ddr_tlbs_phys() and adjusted it slightly to let me choose the target virt address.

Would you prefer if I generalize setup_ddr_tlbs_phys() inside tlb.c and export that function there?

Quoted 101 lines

Quoted 101 lines

void pci_init_board(void)
{

board/freescale/qemu-ppce500/qemu-ppce500.c +1 -1

-	struct fsl_pci_info pci_info;
+	struct pci_controller *pci_hoses;

	const void *fdt = get_fdt();
	int pci_node;

board/freescale/qemu-ppce500/qemu-ppce500.c +4 -0

+	int pci_num = 0;
+	int pci_count;
+	const char *compat = "fsl,mpc8540-pci";
+	ulong map_addr;

	puts("\n");

board/freescale/qemu-ppce500/qemu-ppce500.c +9 -2

-	pci_node = fdt_path_offset(fdt, "/pci");
-	if (pci_node < 0) {
+	/* Start MMIO and PIO range maps above RAM */
+	map_addr = CONFIG_MAX_MEM_MAPPED;
+
+	/* Count and allocate PCI buses */
+	pci_count = myfdt_count_compatibles(fdt, compat);
+
+	if (pci_count) {
+		pci_hoses = malloc(sizeof(struct pci_controller) * pci_count);
+	} else {

		printf("PCI: disabled\n\n");
		return;
	}

board/freescale/qemu-ppce500/qemu-ppce500.c +65 -7

-	SET_STD_PCI_INFO(pci_info, 1);
-
-	fsl_setup_hose(&pci1_hose, pci_info.regs);
-	printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
-		pci_info.regs);
-
-	fsl_pci_init_port(&pci_info, &pci1_hose, 0);
+	/* Spawn PCI buses based on device tree */
+	pci_node = fdt_node_offset_by_compatible(fdt, -1, compat);
+	while (pci_node != -FDT_ERR_NOTFOUND) {
+		struct fsl_pci_info pci_info = { };
+		uint64_t phys_addr;
+		int pnode = fdt_parent_offset(fdt, pci_node);
+		int paddress_cells;
+		int address_cells;
+		int size_cells;
+		int off = 0;
+
+		paddress_cells = myfdt_one_cell(fdt, pnode, "#address-cells", 1);
+		address_cells = myfdt_one_cell(fdt, pci_node, "#address-cells", 1);
+		size_cells = myfdt_one_cell(fdt, pci_node, "#size-cells", 1);
+
+		pci_info.regs = myfdt_readcells(fdt, pci_node, "reg",
+						paddress_cells, 0, 0);
+
+		/* MMIO range */
+		off += address_cells;
+		phys_addr = myfdt_readcells(fdt, pci_node, "ranges",
+					    paddress_cells, off, 0);
+		off += paddress_cells;
+		pci_info.mem_size = myfdt_readcells(fdt, pci_node, "ranges",
+			size_cells, off, 0);
+		off += size_cells;
+
+		/* Align virtual region */
+		map_addr += pci_info.mem_size - 1;
+		map_addr &= ~(pci_info.mem_size - 1);
+		/* Map virtual memory for MMIO range */
+		map_tlb1_io(map_addr, phys_addr, pci_info.mem_size);
+		pci_info.mem_bus = phys_addr;
+		pci_info.mem_phys = phys_addr;
+		map_addr += pci_info.mem_size;
+
+		/* PIO range */
+		off += address_cells;
+		pci_info.io_phys = myfdt_readcells(fdt, pci_node, "ranges",
+			paddress_cells, off, 0);
+		off += paddress_cells;
+		pci_info.io_size = myfdt_readcells(fdt, pci_node, "ranges",
+			size_cells, off, 0);
+
+		/* Align virtual region */
+		map_addr += pci_info.io_size - 1;
+		map_addr &= ~(pci_info.io_size - 1);
+		/* Map virtual memory for MMIO range */
+		map_tlb1_io(map_addr, pci_info.io_phys, pci_info.io_size);
+		pci_info.io_bus = map_addr;
+		map_addr += pci_info.io_size;
+
+		/* Instantiate */
+		pci_info.pci_num = pci_num + 1;
+
+		fsl_setup_hose(&pci_hoses[pci_num], pci_info.regs);
+		printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
+			pci_info.regs);
+
+		fsl_pci_init_port(&pci_info, &pci_hoses[pci_num], pci_num);
+
+		/* Jump to next PCI node */
+		pci_node = fdt_node_offset_by_compatible(fdt, pci_node, compat);
+		pci_num++;
+	}

Quoted 4 lines

Quoted 2 lines

	puts("\n");
}

How about using fdt_translate_address() or other existing functionality?

Mind to show exactly how?

Quoted 12 lines · Alexander Graf

Quoted 9 lines · Scott Wood

Quoted 5 lines · Alexander Graf

The definition of our ppce500 PV machine is that every address is dynamically determined through device tree bindings.

So don't hardcode where PCI devices are in our physical memory layout but instead read them dynamically from the device tree we get passed on boot.

Would it be difficult to make the QEMU emulation properly implement access windows?

Quoted 2 lines

What are access windows? You mean BAR region offsets? Not too hard I suppose, but it adds complexity which we were trying to avoid, no?

It would remove U-Boot complexity (unlike the LAW stuff where we just skip it) because we wouldn't need to care about QEMU's default settings. It should be easier to do than LAW support, and more useful (e.g. Linux currently programs this as well, we just get lucky that it misuses the device tree as configuration information).

Quoted 25 lines

Quoted 16 lines

Quoted 16 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +15 -0

+{
+	int len;
+	const uint32_t *prop;
+
+	prop = fdt_getprop(fdt, node, property, &len);
+
+	if (!prop)
+		return defval;
+
+	if (len < ((off + num) * sizeof(uint32_t)))
+		panic("Invalid fdt");
+
+	prop += off;
+
+	switch (num) {

Quoted 8 lines

board/freescale/qemu-ppce500/qemu-ppce500.c line 45

Quoted 6 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +6 -0

+	case 1:
+		return *prop;
+	case 2:
+		return *(const uint64_t *)prop;
+	}
+

What about this function is specific to qemu-ppce500?

Quoted 1 line

Nothing. But the less common code I touch the less I can break.

The more that line of thought is applied, the uglier the codebase we'll end up with. :-)

Quoted 2 lines

There seems to be an fdt helper framework that's only targeted at a few ARM devices - not sure what to make of that.

Make use of whatever parts you can, and extend it with the missing bits you need. There's also common/fdt_support.c which is definitely not just used by ARM.

Quoted 14 lines

Quoted 5 lines

(quoted diff) +2 -0

+	panic("Invalid cell size");
+}

s/cell size/cell count/

Quoted 8 lines

board/freescale/qemu-ppce500/qemu-ppce500.c line 53

Quoted 5 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +5 -0

+static uint32_t myfdt_one_cell(const void *fdt, int node, const char *property,
+			       uint32_t defval)
+{
+	return myfdt_readcells(fdt, node, property, 1, 0, defval);
+}

This looks a lot like fdt_getprop_u32_default(), except for "int node" versus path.

Quoted 1 line

Well, I use it inside of a loop where I don't have the path :).

It still indicates where the proper place for code like this is. :-)

Quoted 34 lines

Quoted 22 lines

Quoted 22 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +22 -0

+static void map_tlb1_io(ulong virt_addr, uint64_t phys_addr, uint64_t size)
+{
+	unsigned int max_cam, tsize_mask;
+	int i;
+
+	if ((mfspr(SPRN_MMUCFG) & MMUCFG_MAVN) == MMUCFG_MAVN_V1) {
+		/* Convert (4^max) kB to (2^max) bytes */
+		max_cam = ((mfspr(SPRN_TLB1CFG) >> 16) & 0xf) * 2 + 10;
+		tsize_mask = ~1U;
+	} else {
+		/* Convert (2^max) kB to (2^max) bytes */
+		max_cam = __ilog2(mfspr(SPRN_TLB1PS)) + 10;
+		tsize_mask = ~0U;
+	}
+
+	for (i = 0; size && i < 8; i++) {
+		int tlb_index = find_free_tlbcam();
+		u32 camsize = __ilog2_u64(size) & tsize_mask;
+		u32 align = __ilog2(virt_addr) & tsize_mask;
+		unsigned int tlb_size;
+
+		if (tlb_index == -1)

Quoted 8 lines

board/freescale/qemu-ppce500/qemu-ppce500.c int checkboard(void) · line 181

Quoted 3 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +3 -0

+			break;
+
+		if (align == -2) align = max_cam;

-2? Besides align being unsigned, if this is meant to handle the case where virt_addr is zero, that's undefined for __ilog2() (don't rely on it being implemented with cntlzw), and you're not handling the MMUv2 case.

I merely copied this from tlb.c's setup_ddr_tlbs_phys() and adjusted it slightly to let me choose the target virt address.

Quoted 2 lines

Would you prefer if I generalize setup_ddr_tlbs_phys() inside tlb.c and export that function there?

Yes.

And maybe fix that align == -2 bug while you're at it. :-)

Quoted 16 lines · Scott Wood

Quoted 14 lines · Alexander Graf

Quoted 9 lines · Scott Wood

Quoted 5 lines · Alexander Graf

The definition of our ppce500 PV machine is that every address is dynamically determined through device tree bindings.

So don't hardcode where PCI devices are in our physical memory layout but instead read them dynamically from the device tree we get passed on boot.

Would it be difficult to make the QEMU emulation properly implement access windows?

What are access windows? You mean BAR region offsets? Not too hard I suppose, but it adds complexity which we were trying to avoid, no?

Quoted 5 lines

It would remove U-Boot complexity (unlike the LAW stuff where we just skip it) because we wouldn't need to care about QEMU's default settings. It should be easier to do than LAW support, and more useful (e.g. Linux currently programs this as well, we just get lucky that it misuses the device tree as configuration information).

What complexity would it remove?

We would still need to find the configuration space for the access windows,

configure them

and then even go as far as modifying the original device tree so we expose the new windows.

Quoted 6 lines · Alexander Graf

Quoted 4 lines · Scott Wood

Quoted 3 lines · Alexander Graf

The definition of our ppce500 PV machine is that every address is dynamically determined through device tree bindings.

Quoted 8 lines

Quoted 5 lines

Quoted 2 lines

So don't hardcode where PCI devices are in our physical memory layout but instead read them dynamically from the device tree we get passed on boot.

Would it be difficult to make the QEMU emulation properly implement access windows?

What are access windows? You mean BAR region offsets? Not too hard I suppose, but it adds complexity which we were trying to avoid, no?

It would remove U-Boot complexity (unlike the LAW stuff where we just skip it) because we wouldn't need to care about QEMU's default settings. It should be easier to do than LAW support, and more useful (e.g. Linux currently programs this as well, we just get lucky that it misuses the device tree as configuration information).

Quoted 24 lines

Quoted 15 lines

Quoted 15 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +15 -0

+{
+	int len;
+	const uint32_t *prop;
+
+	prop = fdt_getprop(fdt, node, property, &len);
+
+	if (!prop)
+		return defval;
+
+	if (len < ((off + num) * sizeof(uint32_t)))
+		panic("Invalid fdt");
+
+	prop += off;
+
+	switch (num) {

Quoted 8 lines

board/freescale/qemu-ppce500/qemu-ppce500.c line 45

Quoted 6 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +6 -0

+	case 1:
+		return *prop;
+	case 2:
+		return *(const uint64_t *)prop;
+	}
+

What about this function is specific to qemu-ppce500?

Quoted 1 line

Nothing. But the less common code I touch the less I can break.

The more that line of thought is applied, the uglier the codebase we'll end up with. :-)

Quoted 2 lines

There seems to be an fdt helper framework that's only targeted at a few ARM devices - not sure what to make of that.

Make use of whatever parts you can, and extend it with the missing bits you need. There's also common/fdt_support.c which is definitely not just used by ARM.

Quoted 14 lines

Quoted 5 lines

(quoted diff) +2 -0

+	panic("Invalid cell size");
+}

s/cell size/cell count/

Quoted 8 lines

board/freescale/qemu-ppce500/qemu-ppce500.c line 53

Quoted 5 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +5 -0

+static uint32_t myfdt_one_cell(const void *fdt, int node, const char *property,
+			       uint32_t defval)
+{
+	return myfdt_readcells(fdt, node, property, 1, 0, defval);
+}

This looks a lot like fdt_getprop_u32_default(), except for "int node" versus path.

Quoted 1 line

Well, I use it inside of a loop where I don't have the path :).

It still indicates where the proper place for code like this is. :-)

Quoted 31 lines

Quoted 22 lines

Quoted 22 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +22 -0

+static void map_tlb1_io(ulong virt_addr, uint64_t phys_addr, uint64_t size)
+{
+	unsigned int max_cam, tsize_mask;
+	int i;
+
+	if ((mfspr(SPRN_MMUCFG) & MMUCFG_MAVN) == MMUCFG_MAVN_V1) {
+		/* Convert (4^max) kB to (2^max) bytes */
+		max_cam = ((mfspr(SPRN_TLB1CFG) >> 16) & 0xf) * 2 + 10;
+		tsize_mask = ~1U;
+	} else {
+		/* Convert (2^max) kB to (2^max) bytes */
+		max_cam = __ilog2(mfspr(SPRN_TLB1PS)) + 10;
+		tsize_mask = ~0U;
+	}
+
+	for (i = 0; size && i < 8; i++) {
+		int tlb_index = find_free_tlbcam();
+		u32 camsize = __ilog2_u64(size) & tsize_mask;
+		u32 align = __ilog2(virt_addr) & tsize_mask;
+		unsigned int tlb_size;
+
+		if (tlb_index == -1)

Quoted 8 lines

board/freescale/qemu-ppce500/qemu-ppce500.c int checkboard(void) · line 181

Quoted 3 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +3 -0

+			break;
+
+		if (align == -2) align = max_cam;

-2? Besides align being unsigned, if this is meant to handle the case where virt_addr is zero, that's undefined for __ilog2() (don't rely on it being implemented with cntlzw), and you're not handling the MMUv2 case.

Quoted 5 lines

I merely copied this from tlb.c's setup_ddr_tlbs_phys() and adjusted it slightly to let me choose the target virt address.

Would you prefer if I generalize setup_ddr_tlbs_phys() inside tlb.c and export that function there?

Yes.

And maybe fix that align == -2 bug while you're at it. :-)

Quoted 106 lines

Quoted 105 lines

Quoted 101 lines

void pci_init_board(void)
{

board/freescale/qemu-ppce500/qemu-ppce500.c +1 -1

-	struct fsl_pci_info pci_info;
+	struct pci_controller *pci_hoses;

	const void *fdt = get_fdt();
	int pci_node;

board/freescale/qemu-ppce500/qemu-ppce500.c +4 -0

+	int pci_num = 0;
+	int pci_count;
+	const char *compat = "fsl,mpc8540-pci";
+	ulong map_addr;

	puts("\n");

board/freescale/qemu-ppce500/qemu-ppce500.c +9 -2

-	pci_node = fdt_path_offset(fdt, "/pci");
-	if (pci_node < 0) {
+	/* Start MMIO and PIO range maps above RAM */
+	map_addr = CONFIG_MAX_MEM_MAPPED;
+
+	/* Count and allocate PCI buses */
+	pci_count = myfdt_count_compatibles(fdt, compat);
+
+	if (pci_count) {
+		pci_hoses = malloc(sizeof(struct pci_controller) * pci_count);
+	} else {

		printf("PCI: disabled\n\n");
		return;
	}

board/freescale/qemu-ppce500/qemu-ppce500.c +65 -7

-	SET_STD_PCI_INFO(pci_info, 1);
-
-	fsl_setup_hose(&pci1_hose, pci_info.regs);
-	printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
-		pci_info.regs);
-
-	fsl_pci_init_port(&pci_info, &pci1_hose, 0);
+	/* Spawn PCI buses based on device tree */
+	pci_node = fdt_node_offset_by_compatible(fdt, -1, compat);
+	while (pci_node != -FDT_ERR_NOTFOUND) {
+		struct fsl_pci_info pci_info = { };
+		uint64_t phys_addr;
+		int pnode = fdt_parent_offset(fdt, pci_node);
+		int paddress_cells;
+		int address_cells;
+		int size_cells;
+		int off = 0;
+
+		paddress_cells = myfdt_one_cell(fdt, pnode, "#address-cells", 1);
+		address_cells = myfdt_one_cell(fdt, pci_node, "#address-cells", 1);
+		size_cells = myfdt_one_cell(fdt, pci_node, "#size-cells", 1);
+
+		pci_info.regs = myfdt_readcells(fdt, pci_node, "reg",
+						paddress_cells, 0, 0);
+
+		/* MMIO range */
+		off += address_cells;
+		phys_addr = myfdt_readcells(fdt, pci_node, "ranges",
+					    paddress_cells, off, 0);
+		off += paddress_cells;
+		pci_info.mem_size = myfdt_readcells(fdt, pci_node, "ranges",
+			size_cells, off, 0);
+		off += size_cells;
+
+		/* Align virtual region */
+		map_addr += pci_info.mem_size - 1;
+		map_addr &= ~(pci_info.mem_size - 1);
+		/* Map virtual memory for MMIO range */
+		map_tlb1_io(map_addr, phys_addr, pci_info.mem_size);
+		pci_info.mem_bus = phys_addr;
+		pci_info.mem_phys = phys_addr;
+		map_addr += pci_info.mem_size;
+
+		/* PIO range */
+		off += address_cells;
+		pci_info.io_phys = myfdt_readcells(fdt, pci_node, "ranges",
+			paddress_cells, off, 0);
+		off += paddress_cells;
+		pci_info.io_size = myfdt_readcells(fdt, pci_node, "ranges",
+			size_cells, off, 0);
+
+		/* Align virtual region */
+		map_addr += pci_info.io_size - 1;
+		map_addr &= ~(pci_info.io_size - 1);
+		/* Map virtual memory for MMIO range */
+		map_tlb1_io(map_addr, pci_info.io_phys, pci_info.io_size);
+		pci_info.io_bus = map_addr;
+		map_addr += pci_info.io_size;
+
+		/* Instantiate */
+		pci_info.pci_num = pci_num + 1;
+
+		fsl_setup_hose(&pci_hoses[pci_num], pci_info.regs);
+		printf("PCI: 32 bit, 66 MHz, async, host, base address %lx\n",
+			pci_info.regs);
+
+		fsl_pci_init_port(&pci_info, &pci_hoses[pci_num], pci_num);
+
+		/* Jump to next PCI node */
+		pci_node = fdt_node_offset_by_compatible(fdt, pci_node, compat);
+		pci_num++;
+	}

Quoted 2 lines

	puts("\n");
}

How about using fdt_translate_address() or other existing functionality?

Quoted 1 line

Mind to show exactly how?

I guess you can't use that when you don't know the bus address, but still this is the wrong place to implement it. If getting PCI range info from the device tree is something we want to do, then it should be a new common code helper.

Quoted 6 lines · Scott Wood

Quoted 4 lines · Alexander Graf

Quoted 2 lines

How about using fdt_translate_address() or other existing functionality?

Mind to show exactly how?

Quoted 4 lines

I guess you can't use that when you don't know the bus address, but still this is the wrong place to implement it. If getting PCI range info from the device tree is something we want to do, then it should be a new common code helper.

The more I think about this the less of an idea I have how to do any generic API for this at all. And I'm not convinced anything generic is going to help anyone. Do a git grep on fdt_translate_address over the code base and you will find a _single_ caller.

So even if we come up with something now, nobody's going to use it.

Quoted 14 lines · Alexander Graf

Quoted 10 lines · Scott Wood

Quoted 4 lines · Alexander Graf

Quoted 2 lines

How about using fdt_translate_address() or other existing functionality?

Mind to show exactly how?

I guess you can't use that when you don't know the bus address, but still this is the wrong place to implement it. If getting PCI range info from the device tree is something we want to do, then it should be a new common code helper.

Quoted 3 lines

The more I think about this the less of an idea I have how to do any generic API for this at all. And I'm not convinced anything generic is going to help anyone. Do a git grep on fdt_translate_address over the code base and you will find a _single_ caller.

So even if we come up with something now, nobody's going to use it.

Hrm. Maybe fdt_get_base_address() is the answer?

The only thing we know in our PV machine through device tree is the clock speed of the CPUs. Take that as CPU speed, system speed and ddr speed so that we have some meaningful values there at all.

The CPU speed is important because our timing loops get determined based on it.

Signed-off-by Alexander Graf <[email protected]>

---
 arch/powerpc/cpu/mpc85xx/Makefile           |    2 ++
 board/freescale/qemu-ppce500/qemu-ppce500.c |   43 +++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

arch/powerpc/cpu/mpc85xx/Makefile +2 -0

index ef7637a..4094785 100644
@@ -102,7 +102,9 @@ obj-y	+= cpu.o
 obj-y	+= cpu_init.o
 obj-y	+= cpu_init_early.o
 obj-y	+= interrupts.o
+ifneq ($(CONFIG_QEMU_E500),y)
 obj-y	+= speed.o
+endif
 obj-y	+= tlb.o
 obj-y	+= traps.o
 

board/freescale/qemu-ppce500/qemu-ppce500.c +22 -0

index 5d4dd64..9e9d688 100644
@@ -407,3 +407,46 @@ void init_laws(void)
 {
 	/* We don't emulate LAWs yet */
 }
+
+static uint32_t get_cpu_freq(void)
+{
+	const void *fdt = get_fdt();
+	int cpus_node = fdt_path_offset(fdt, "/cpus");
+	int cpu_node = fdt_first_subnode(fdt, cpus_node);
+	return myfdt_one_cell(fdt, cpu_node, "clock-frequency", 0);
+}
+
+void get_sys_info(sys_info_t *sys_info)
+{
+	int freq = get_cpu_freq();
+
+	memset(sys_info, 0, sizeof(sys_info_t));
+	sys_info->freq_systembus = freq;
+	sys_info->freq_ddrbus = freq;
+	sys_info->freq_processor[0] = freq;
+}
+
+int get_clocks (void)
+{
+	sys_info_t sys_info;

Quoted 56 lines · Alexander Graf

The only thing we know in our PV machine through device tree is the clock speed of the CPUs. Take that as CPU speed, system speed and ddr speed so that we have some meaningful values there at all.

The CPU speed is important because our timing loops get determined based on it.

Signed-off-by Alexander Graf <[email protected]>

---
arch/powerpc/cpu/mpc85xx/Makefile | 2 ++
board/freescale/qemu-ppce500/qemu-ppce500.c | 43 +++++++++++++++++++++++++++

2 files changed, 45 insertions(+)

arch/powerpc/cpu/mpc85xx/Makefile +0 -0

index ef7637a..4094785 100644
@@ -102,7 +102,9 @@ obj-y	+= cpu.o

obj-y += cpu_init.o
obj-y += cpu_init_early.o
obj-y += interrupts.o
+ifneq ($(CONFIG_QEMU_E500),y)
obj-y += speed.o
+endif
obj-y += tlb.o
obj-y += traps.o

board/freescale/qemu-ppce500/qemu-ppce500.c +22 -0

index 5d4dd64..9e9d688 100644
@@ -407,3 +407,46 @@ void init_laws(void)
 {
 	/* We don't emulate LAWs yet */
 }
+
+static uint32_t get_cpu_freq(void)
+{
+	const void *fdt = get_fdt();
+	int cpus_node = fdt_path_offset(fdt, "/cpus");
+	int cpu_node = fdt_first_subnode(fdt, cpus_node);
+	return myfdt_one_cell(fdt, cpu_node, "clock-frequency", 0);
+}
+
+void get_sys_info(sys_info_t *sys_info)
+{
+	int freq = get_cpu_freq();
+
+	memset(sys_info, 0, sizeof(sys_info_t));
+	sys_info->freq_systembus = freq;
+	sys_info->freq_ddrbus = freq;
+	sys_info->freq_processor[0] = freq;
+}
+
+int get_clocks (void)
+{
+	sys_info_t sys_info;

board/freescale/qemu-ppce500/qemu-ppce500.c void init_laws(void) · line 441

Quoted 10 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +10 -0

+
+	get_sys_info(&sys_info);
+
+	gd->cpu_clk = sys_info.freq_processor[0];
+	gd->bus_clk = sys_info.freq_systembus;
+	gd->mem_clk = sys_info.freq_ddrbus;
+	gd->arch.lbc_clk = sys_info.freq_ddrbus;
+
+	return 0;
+}

This probably decreases the accuracy of the timebase frequency, since you'll be basing it on the CPU frequency rather than the bus frequency.

If you're doing this, why not override get_tbclk() as well?

Quoted 71 lines · Scott Wood

Quoted 56 lines · Alexander Graf

The only thing we know in our PV machine through device tree is the clock speed of the CPUs. Take that as CPU speed, system speed and ddr speed so that we have some meaningful values there at all.

The CPU speed is important because our timing loops get determined based on it.

Signed-off-by Alexander Graf <[email protected]>

---
arch/powerpc/cpu/mpc85xx/Makefile | 2 ++
board/freescale/qemu-ppce500/qemu-ppce500.c | 43 +++++++++++++++++++++++++++

2 files changed, 45 insertions(+)

arch/powerpc/cpu/mpc85xx/Makefile +0 -0

index ef7637a..4094785 100644
@@ -102,7 +102,9 @@ obj-y	+= cpu.o

obj-y += cpu_init.o
obj-y += cpu_init_early.o
obj-y += interrupts.o
+ifneq ($(CONFIG_QEMU_E500),y)
obj-y += speed.o
+endif
obj-y += tlb.o
obj-y += traps.o

board/freescale/qemu-ppce500/qemu-ppce500.c +22 -0

index 5d4dd64..9e9d688 100644
@@ -407,3 +407,46 @@ void init_laws(void)
 {
 	/* We don't emulate LAWs yet */
 }
+
+static uint32_t get_cpu_freq(void)
+{
+	const void *fdt = get_fdt();
+	int cpus_node = fdt_path_offset(fdt, "/cpus");
+	int cpu_node = fdt_first_subnode(fdt, cpus_node);
+	return myfdt_one_cell(fdt, cpu_node, "clock-frequency", 0);
+}
+
+void get_sys_info(sys_info_t *sys_info)
+{
+	int freq = get_cpu_freq();
+
+	memset(sys_info, 0, sizeof(sys_info_t));
+	sys_info->freq_systembus = freq;
+	sys_info->freq_ddrbus = freq;
+	sys_info->freq_processor[0] = freq;
+}
+
+int get_clocks (void)
+{
+	sys_info_t sys_info;

board/freescale/qemu-ppce500/qemu-ppce500.c void init_laws(void) · line 441

Quoted 10 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +10 -0

+
+	get_sys_info(&sys_info);
+
+	gd->cpu_clk = sys_info.freq_processor[0];
+	gd->bus_clk = sys_info.freq_systembus;
+	gd->mem_clk = sys_info.freq_ddrbus;
+	gd->arch.lbc_clk = sys_info.freq_ddrbus;
+
+	return 0;
+}

This probably decreases the accuracy of the timebase frequency, since you'll be basing it on the CPU frequency rather than the bus frequency.

Quoted 1 line

If you're doing this, why not override get_tbclk() as well?

That one only uses variables I already control, no?

unsigned long get_tbclk (void)

{
        unsigned long tbclk_div = CONFIG_SYS_FSL_TBCLK_DIV;

        return (gd->bus_clk + (tbclk_div >> 1)) / tbclk_div;
}

Quoted 77 lines · Alexander Graf

Quoted 71 lines · Scott Wood

Quoted 56 lines · Alexander Graf

The only thing we know in our PV machine through device tree is the clock speed of the CPUs. Take that as CPU speed, system speed and ddr speed so that we have some meaningful values there at all.

The CPU speed is important because our timing loops get determined based on it.

Signed-off-by Alexander Graf <[email protected]>

---
arch/powerpc/cpu/mpc85xx/Makefile | 2 ++
board/freescale/qemu-ppce500/qemu-ppce500.c | 43 +++++++++++++++++++++++++++

2 files changed, 45 insertions(+)

arch/powerpc/cpu/mpc85xx/Makefile +0 -0

index ef7637a..4094785 100644
@@ -102,7 +102,9 @@ obj-y	+= cpu.o

obj-y += cpu_init.o
obj-y += cpu_init_early.o
obj-y += interrupts.o
+ifneq ($(CONFIG_QEMU_E500),y)
obj-y += speed.o
+endif
obj-y += tlb.o
obj-y += traps.o

board/freescale/qemu-ppce500/qemu-ppce500.c +22 -0

index 5d4dd64..9e9d688 100644
@@ -407,3 +407,46 @@ void init_laws(void)
 {
 	/* We don't emulate LAWs yet */
 }
+
+static uint32_t get_cpu_freq(void)
+{
+	const void *fdt = get_fdt();
+	int cpus_node = fdt_path_offset(fdt, "/cpus");
+	int cpu_node = fdt_first_subnode(fdt, cpus_node);
+	return myfdt_one_cell(fdt, cpu_node, "clock-frequency", 0);
+}
+
+void get_sys_info(sys_info_t *sys_info)
+{
+	int freq = get_cpu_freq();
+
+	memset(sys_info, 0, sizeof(sys_info_t));
+	sys_info->freq_systembus = freq;
+	sys_info->freq_ddrbus = freq;
+	sys_info->freq_processor[0] = freq;
+}
+
+int get_clocks (void)
+{
+	sys_info_t sys_info;

board/freescale/qemu-ppce500/qemu-ppce500.c void init_laws(void) · line 441

Quoted 10 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +10 -0

+
+	get_sys_info(&sys_info);
+
+	gd->cpu_clk = sys_info.freq_processor[0];
+	gd->bus_clk = sys_info.freq_systembus;
+	gd->mem_clk = sys_info.freq_ddrbus;
+	gd->arch.lbc_clk = sys_info.freq_ddrbus;
+
+	return 0;
+}

This probably decreases the accuracy of the timebase frequency, since you'll be basing it on the CPU frequency rather than the bus frequency.

Quoted 1 line

If you're doing this, why not override get_tbclk() as well?

That one only uses variables I already control, no?

Quoted 6 lines

unsigned long get_tbclk (void)

{
       unsigned long tbclk_div = CONFIG_SYS_FSL_TBCLK_DIV;

       return (gd->bus_clk + (tbclk_div >> 1)) / tbclk_div;
}

What did you set CONFIG_SYS_FSL_TBCLK_DIV to?

Quoted 82 lines · Scott Wood

Quoted 81 lines · Alexander Graf

Quoted 71 lines · Scott Wood

Quoted 56 lines · Alexander Graf

The only thing we know in our PV machine through device tree is the clock speed of the CPUs. Take that as CPU speed, system speed and ddr speed so that we have some meaningful values there at all.

The CPU speed is important because our timing loops get determined based on it.

Signed-off-by Alexander Graf <[email protected]>

---
arch/powerpc/cpu/mpc85xx/Makefile | 2 ++
board/freescale/qemu-ppce500/qemu-ppce500.c | 43 +++++++++++++++++++++++++++

2 files changed, 45 insertions(+)

arch/powerpc/cpu/mpc85xx/Makefile +0 -0

index ef7637a..4094785 100644
@@ -102,7 +102,9 @@ obj-y	+= cpu.o

obj-y += cpu_init.o
obj-y += cpu_init_early.o
obj-y += interrupts.o
+ifneq ($(CONFIG_QEMU_E500),y)
obj-y += speed.o
+endif
obj-y += tlb.o
obj-y += traps.o

board/freescale/qemu-ppce500/qemu-ppce500.c +22 -0

index 5d4dd64..9e9d688 100644
@@ -407,3 +407,46 @@ void init_laws(void)
 {
 	/* We don't emulate LAWs yet */
 }
+
+static uint32_t get_cpu_freq(void)
+{
+	const void *fdt = get_fdt();
+	int cpus_node = fdt_path_offset(fdt, "/cpus");
+	int cpu_node = fdt_first_subnode(fdt, cpus_node);
+	return myfdt_one_cell(fdt, cpu_node, "clock-frequency", 0);
+}
+
+void get_sys_info(sys_info_t *sys_info)
+{
+	int freq = get_cpu_freq();
+
+	memset(sys_info, 0, sizeof(sys_info_t));
+	sys_info->freq_systembus = freq;
+	sys_info->freq_ddrbus = freq;
+	sys_info->freq_processor[0] = freq;
+}
+
+int get_clocks (void)
+{
+	sys_info_t sys_info;

board/freescale/qemu-ppce500/qemu-ppce500.c void init_laws(void) · line 441

Quoted 10 lines

board/freescale/qemu-ppce500/qemu-ppce500.c +10 -0

+
+	get_sys_info(&sys_info);
+
+	gd->cpu_clk = sys_info.freq_processor[0];
+	gd->bus_clk = sys_info.freq_systembus;
+	gd->mem_clk = sys_info.freq_ddrbus;
+	gd->arch.lbc_clk = sys_info.freq_ddrbus;
+
+	return 0;
+}

This probably decreases the accuracy of the timebase frequency, since you'll be basing it on the CPU frequency rather than the bus frequency.

Quoted 1 line

If you're doing this, why not override get_tbclk() as well?

That one only uses variables I already control, no?

unsigned long get_tbclk (void)

{
       unsigned long tbclk_div = CONFIG_SYS_FSL_TBCLK_DIV;

Quoted 4 lines

Quoted 2 lines

       return (gd->bus_clk + (tbclk_div >> 1)) / tbclk_div;
}

What did you set CONFIG_SYS_FSL_TBCLK_DIV to?

Nothing which is why it's defaulting to 8.