[U-Boot] [PATCH 0/3] PPC 85xx: Add support for QEMU's ppce500 PV machine

[U-Boot] [PATCH 0/3] PPC 85xx: Add support for QEMU's ppce500 PV machine

[Alexander Graf]

In QEMU we implement a PV machine type called "ppce500". That board is able
to run any e500+ FSL cores (e500v2, e500mc, e5500, e6500).

It is heavily inspired by the MPC8544DS SoC and board combination, but
implements only the bare minimum to make Linux happy enough to drive a
virtual machine.

This patch set implements support for this PV machine type in U-Boot, enabling
users to run their virtual machines with netboot, u-boot payload binaries or
other fun things they come up with.

Alexander Graf (3):
  PPC 85xx: Detect e500v2 / e500mc during runtime
  PPC 85xx: Add ELF entry point
  PPC 85xx: Add qemu-ppce500 machine

 arch/powerpc/cpu/mpc85xx/fixed_ivor.S       |   21 ++-
 arch/powerpc/cpu/mpc85xx/start.S            |    7 +
 arch/powerpc/cpu/mpc85xx/u-boot.lds         |    1 +
 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 ++++++++++++++++++++++++
 9 files changed, 593 insertions(+), 5 deletions(-)
 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

-- 
1.7.10.4

[U-Boot] [PATCH 1/3] PPC 85xx: Detect e500v2 / e500mc during runtime

[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 <agraf@suse.de>
---
 arch/powerpc/cpu/mpc85xx/fixed_ivor.S |   21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/cpu/mpc85xx/fixed_ivor.S b/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
index ebbb8c0..635a97e 100644
--- a/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
+++ b/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
@@ -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 */
@@ -61,3 +71,4 @@
 	SET_GIVOR(13, 0x1c0) /* Guest Data TLB Error */
 	SET_GIVOR(14, 0x1e0) /* Guest Instruction TLB Error */
 #endif
+2:
-- 
1.7.10.4

[U-Boot] [PATCH 1/3] PPC 85xx: Detect e500v2 / e500mc during runtime

[Scott Wood]

On Sun, 2014-01-19 at 16:19 +0100, Alexander Graf wrote:
> 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.

Is this really the only place where you ran into this?

Also consider that you'll be adding extra size, and some of our 85xx
targets are pretty close to the limit as is (though at least this code
isn't used in SPLs).

I guess nobody ever bothered to set IVORs for e6500-specific exceptions.

For that matter, I don't see why we need this code at all.  These aren't
the addresses that U-Boot keeps its exception vectors at; it's setting
them up for the OS, apparently trying to imitate some other type of
book3e chip that has fixed ivors.  Apparently U-Boot has done this only
since 2009 (commit 26f4cdba6b51deab4ec99d60be381244068ef950), so it's
not even something that an OS could depend on (and certainly Linux
doesn't).  So I don't see the point.

-Scott

[U-Boot] [PATCH 1/3] PPC 85xx: Detect e500v2 / e500mc during runtime

[Alexander Graf]

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:
>> 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.
> 
> Is this really the only place where you ran into this?

Yup. At least the only place where the difference actually matters for a VM.

> Also consider that you'll be adding extra size, and some of our 85xx
> targets are pretty close to the limit as is (though at least this code
> isn't used in SPLs).
> 
> I guess nobody ever bothered to set IVORs for e6500-specific exceptions.
> 
> For that matter, I don't see why we need this code at all.  These aren't
> the addresses that U-Boot keeps its exception vectors at; it's setting
> them up for the OS, apparently trying to imitate some other type of
> book3e chip that has fixed ivors.  Apparently U-Boot has done this only
> since 2009 (commit 26f4cdba6b51deab4ec99d60be381244068ef950), so it's
> not even something that an OS could depend on (and certainly Linux
> doesn't).  So I don't see the point.

Kumar, do you remember why you put this in? Was it only for prototyping purposes?

I certainly wouldn't mind removing the whole thing altogether.


Alex

[U-Boot] [PATCH 1/3] PPC 85xx: Detect e500v2 / e500mc during runtime

[Kumar Gala]

On Jan 23, 2014, at 6:11 AM, Alexander Graf <agraf@suse.de> 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:
>>> 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.
>> 
>> Is this really the only place where you ran into this?
> 
> Yup. At least the only place where the difference actually matters for a VM.
> 
>> Also consider that you'll be adding extra size, and some of our 85xx
>> targets are pretty close to the limit as is (though at least this code
>> isn't used in SPLs).
>> 
>> I guess nobody ever bothered to set IVORs for e6500-specific exceptions.
>> 
>> For that matter, I don't see why we need this code at all.  These aren't
>> the addresses that U-Boot keeps its exception vectors at; it's setting
>> them up for the OS, apparently trying to imitate some other type of
>> book3e chip that has fixed ivors.  Apparently U-Boot has done this only
>> since 2009 (commit 26f4cdba6b51deab4ec99d60be381244068ef950), so it's
>> not even something that an OS could depend on (and certainly Linux
>> doesn't).  So I don't see the point.
> 
> Kumar, do you remember why you put this in? Was it only for prototyping purposes?
> 
> I certainly wouldn't mind removing the whole thing altogether.
> 
> 
> Alex


I feel like we did have some support for timer & external interrupts in u-boot.  Its been a while since I looked.

- k

[U-Boot] [PATCH 1/3] PPC 85xx: Detect e500v2 / e500mc during runtime

[Scott Wood]

On Thu, 2014-03-06 at 09:48 -0600, Kumar Gala wrote:
> On Jan 23, 2014, at 6:11 AM, Alexander Graf <agraf@suse.de> 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:
> >>> 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.
> >> 
> >> Is this really the only place where you ran into this?
> > 
> > Yup. At least the only place where the difference actually matters for a VM.
> > 
> >> Also consider that you'll be adding extra size, and some of our 85xx
> >> targets are pretty close to the limit as is (though at least this code
> >> isn't used in SPLs).
> >> 
> >> I guess nobody ever bothered to set IVORs for e6500-specific exceptions.
> >> 
> >> For that matter, I don't see why we need this code at all.  These aren't
> >> the addresses that U-Boot keeps its exception vectors at; it's setting
> >> them up for the OS, apparently trying to imitate some other type of
> >> book3e chip that has fixed ivors.  Apparently U-Boot has done this only
> >> since 2009 (commit 26f4cdba6b51deab4ec99d60be381244068ef950), so it's
> >> not even something that an OS could depend on (and certainly Linux
> >> doesn't).  So I don't see the point.
> > 
> > Kumar, do you remember why you put this in? Was it only for prototyping purposes?
> > 
> > I certainly wouldn't mind removing the whole thing altogether.
> > 
> > 
> > Alex
> 
> 
> I feel like we did have some support for timer & external interrupts in u-boot.  Its been a while since I looked.

This has nothing to do with U-Boot's own exception handlers -- this is
what U-Boot is currently doing just prior to entering the OS.

-Scott

[U-Boot] [PATCH 1/3] PPC 85xx: Detect e500v2 / e500mc during runtime

[Kumar Gala]

On Mar 6, 2014, at 9:50 AM, Scott Wood <scottwood@freescale.com> wrote:

> On Thu, 2014-03-06 at 09:48 -0600, Kumar Gala wrote:
>> On Jan 23, 2014, at 6:11 AM, Alexander Graf <agraf@suse.de> 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:
>>>>> 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.
>>>> 
>>>> Is this really the only place where you ran into this?
>>> 
>>> Yup. At least the only place where the difference actually matters for a VM.
>>> 
>>>> Also consider that you'll be adding extra size, and some of our 85xx
>>>> targets are pretty close to the limit as is (though at least this code
>>>> isn't used in SPLs).
>>>> 
>>>> I guess nobody ever bothered to set IVORs for e6500-specific exceptions.
>>>> 
>>>> For that matter, I don't see why we need this code at all.  These aren't
>>>> the addresses that U-Boot keeps its exception vectors at; it's setting
>>>> them up for the OS, apparently trying to imitate some other type of
>>>> book3e chip that has fixed ivors.  Apparently U-Boot has done this only
>>>> since 2009 (commit 26f4cdba6b51deab4ec99d60be381244068ef950), so it's
>>>> not even something that an OS could depend on (and certainly Linux
>>>> doesn't).  So I don't see the point.
>>> 
>>> Kumar, do you remember why you put this in? Was it only for prototyping purposes?
>>> 
>>> I certainly wouldn't mind removing the whole thing altogether.
>>> 
>>> 
>>> Alex
>> 
>> 
>> I feel like we did have some support for timer & external interrupts in u-boot.  Its been a while since I looked.
> 
> This has nothing to do with U-Boot's own exception handlers -- this is
> what U-Boot is currently doing just prior to entering the OS.
> 
> -Scott
> 

Oh, right.  Did e6500 move to fixed IVORs?  If not this can probably be removed as I doubt there will be a new core from FSL that would require this.  It was meant as a transition for any OS that cared (if my memory serves me).

- k

[U-Boot] [PATCH 1/3] PPC 85xx: Detect e500v2 / e500mc during runtime

[Scott Wood]

On Thu, 2014-03-06 at 09:56 -0600, Kumar Gala wrote:
> On Mar 6, 2014, at 9:50 AM, Scott Wood <scottwood@freescale.com> wrote:
> 
> > This has nothing to do with U-Boot's own exception handlers -- this is
> > what U-Boot is currently doing just prior to entering the OS.
> > 
> > -Scott
> > 
> 
> Oh, right.  Did e6500 move to fixed IVORs?  If not this can probably be
> removed as I doubt there will be a new core from FSL that would require
> this.  It was meant as a transition for any OS that cared (if my memory
> serves me).

e6500 does not have fixed IVORs.  I'm not sure why it would matter even
if it did.  An OS should know whether it is running on a chip with fixed
IVORs and behave appropriately, rather than assuming the bootloader
fixes things up on older chips to look like newer chips.

-Scott

[U-Boot] [PATCH 2/3] PPC 85xx: Add ELF entry point

[Alexander Graf]

We want to be able to directly execute the ELF binary without going
through the u-boot.bin one.

To know where we have to start executing this ELF binary  we have to
tell the linker where our entry point is.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/powerpc/cpu/mpc85xx/u-boot.lds |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/cpu/mpc85xx/u-boot.lds b/arch/powerpc/cpu/mpc85xx/u-boot.lds
index 2af4c80..b34d212 100644
--- a/arch/powerpc/cpu/mpc85xx/u-boot.lds
+++ b/arch/powerpc/cpu/mpc85xx/u-boot.lds
@@ -13,6 +13,7 @@
 #endif
 
 OUTPUT_ARCH(powerpc)
+ENTRY(_start_e500)
 
 PHDRS
 {
-- 
1.7.10.4

[U-Boot] [PATCH 3/3] PPC 85xx: Add qemu-ppce500 machine

[Alexander Graf]

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 at h
+	ori	r2, r2, CONFIG_QEMU_DT_ADDR at l
+	stw	r3, 0(r2)
+#endif
+
 #ifdef CONFIG_SYS_FSL_ERRATUM_A004510
 	mfspr	r3,SPRN_SVR
 	rlwinm	r3,r3,0,0xff
diff --git a/arch/powerpc/include/asm/config_mpc85xx.h b/arch/powerpc/include/asm/config_mpc85xx.h
index 54ce2f0..a0ab453 100644
--- a/arch/powerpc/include/asm/config_mpc85xx.h
+++ b/arch/powerpc/include/asm/config_mpc85xx.h
@@ -776,6 +776,10 @@ defined(CONFIG_PPC_T1020) || defined(CONFIG_PPC_T1022)
 #define CONFIG_SYS_CCSRBAR_DEFAULT	0xff700000
 #define CONFIG_SYS_FSL_ERRATUM_A005125
 
+#elif defined(CONFIG_QEMU_E500)
+#define CONFIG_MAX_CPUS			1
+#define CONFIG_SYS_CCSRBAR_DEFAULT	0xe0000000
+
 #else
 #error Processor type not defined for this platform
 #endif
diff --git a/board/freescale/qemu-ppce500/Makefile b/board/freescale/qemu-ppce500/Makefile
new file mode 100644
index 0000000..193b160
--- /dev/null
+++ b/board/freescale/qemu-ppce500/Makefile
@@ -0,0 +1,10 @@
+#
+# Copyright 2007 Freescale Semiconductor, Inc.
+# (C) Copyright 2001-2006
+# Wolfgang Denk, DENX Software Engineering, wd at denx.de.
+#
+# SPDX-License-Identifier:	GPL-2.0+
+#
+
+obj-y	+= qemu-ppce500.o
+obj-y	+= tlb.o
diff --git a/board/freescale/qemu-ppce500/qemu-ppce500.c b/board/freescale/qemu-ppce500/qemu-ppce500.c
new file mode 100644
index 0000000..c6c4b4a
--- /dev/null
+++ b/board/freescale/qemu-ppce500/qemu-ppce500.c
@@ -0,0 +1,260 @@
+/*
+ * Copyright 2007,2009-2014 Freescale Semiconductor, Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <command.h>
+#include <pci.h>
+#include <asm/processor.h>
+#include <asm/mmu.h>
+#include <asm/immap_85xx.h>
+#include <asm/fsl_pci.h>
+#include <asm/io.h>
+#include <miiphy.h>
+#include <libfdt.h>
+#include <fdt_support.h>
+#include <netdev.h>
+#include <fdtdec.h>
+#include <errno.h>
+#include <malloc.h>
+
+int checkboard(void)
+{
+	return 0;
+}
+
+static struct pci_controller pci1_hose;
+
+void pci_init_board(void)
+{
+	volatile ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
+	struct fsl_pci_info pci_info;
+	u32 devdisr, pordevsr;
+	u32 porpllsr, pci_agent, pci_speed, pci_32, pci_arb, pci_clk_sel;
+	int first_free_busno = 0;
+
+	devdisr = in_be32(&gur->devdisr);
+	pordevsr = in_be32(&gur->pordevsr);
+	porpllsr = in_be32(&gur->porpllsr);
+
+	puts("\n");
+
+	pci_speed = 66666000;
+	pci_32 = 1;
+	pci_arb = pordevsr & MPC85xx_PORDEVSR_PCI1_ARB;
+	pci_clk_sel = porpllsr & MPC85xx_PORDEVSR_PCI1_SPD;
+
+	if (!(devdisr & MPC85xx_DEVDISR_PCI1)) {
+		SET_STD_PCI_INFO(pci_info, 1);
+
+		pci_agent = fsl_setup_hose(&pci1_hose, pci_info.regs);
+		printf("PCI: %d bit, %s MHz, %s, %s, %s (base address %lx)\n",
+			(pci_32) ? 32 : 64,
+			(pci_speed == 33333000) ? "33" :
+			(pci_speed == 66666000) ? "66" : "unknown",
+			pci_clk_sel ? "sync" : "async",
+			pci_agent ? "agent" : "host",
+			pci_arb ? "arbiter" : "external-arbiter",
+			pci_info.regs);
+
+		first_free_busno = fsl_pci_init_port(&pci_info,
+					&pci1_hose, first_free_busno);
+	} else {
+		printf("PCI: disabled\n");
+	}
+
+	puts("\n");
+}
+
+int last_stage_init(void)
+{
+	int ret;
+	int len = 0;
+	const void *prop;
+	int chosen;
+        uint32_t *dt_base_ptr = (void*)CONFIG_QEMU_DT_ADDR;
+	uint32_t dt_base = *dt_base_ptr;
+	uint32_t dt_size;
+	void *fdt;
+
+	dt_size = fdt_totalsize((void*)dt_base);
+
+	/* Reserve 4k for dtb tweaks */
+	dt_size += 4096;
+	fdt = malloc(dt_size);
+
+	/* Open device tree */
+	ret = fdt_open_into((void*)dt_base, fdt, dt_size);
+	if (ret) {
+		printf("Couldn't open fdt at %x\n", dt_base);
+		return -EIO;
+	}
+
+	chosen = fdt_path_offset(fdt, "/chosen");
+	if (chosen < 0) {
+		printf("Couldn't find /chosen node in fdt\n");
+		return -EIO;
+	}
+
+	prop = fdt_getprop(fdt, chosen, "qemu,boot-kernel", &len);
+	if (prop && (len >= 8)) {
+		/* -kernel boot */
+		setenv_hex("kernel_addr", *(uint64_t*)prop);
+
+		/*
+		 * We already know where the initrd is inside the dtb, so no
+		 * need to override it
+		 */
+			setenv("ramdisk_addr", "-");
+	}
+
+	/* Give the user a variable for the host fdt */
+	setenv_hex("fdt_addr", (int)dt_base);
+
+	free(fdt);
+
+	return 0;
+}
+
+static uint64_t get_linear_ram_size(void)
+{
+	const void *prop;
+	int memory;
+	int len;
+        uint32_t *dt_base_ptr = (void*)CONFIG_QEMU_DT_ADDR;
+	void *fdt = &dt_base_ptr[1];
+	int ret;
+	uint32_t dt_base = *dt_base_ptr;
+	uint32_t dt_size = 1024 * 1024;
+
+	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");
+}
+
+unsigned long
+get_board_sys_clk(ulong dummy)
+{
+	/* The actual clock doesn't matter in a PV machine */
+	return 33333333;
+}
+
+
+int board_phy_config(struct phy_device *phydev)
+{
+	return 0;
+}
+
+
+int board_eth_init(bd_t *bis)
+{
+	return pci_eth_init(bis);
+}
+
+#if defined(CONFIG_OF_BOARD_SETUP)
+void ft_board_setup(void *blob, bd_t *bd)
+{
+	FT_FSL_PCI_SETUP;
+}
+#endif
+
+void print_laws(void)
+{
+	/* We don't emulate LAWs yet */
+}
+
+static void fixup_tlb1(uint64_t ram_size)
+{
+	u32 mas0, mas1, mas2, mas3, mas7;
+	long tmp;
+
+	/* Flush TLB0 - it only contains stale maps by now */
+	invalidate_tlb(0);
+
+	/* Create a temporary AS=1 map for ourselves */
+        mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(13);
+        mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TS | MAS1_TSIZE(BOOKE_PAGESZ_64M);
+        mas2 = FSL_BOOKE_MAS2(0, 0);
+        mas3 = FSL_BOOKE_MAS3(CONFIG_SYS_DDR_SDRAM_BASE, 0,
+			      MAS3_SW|MAS3_SR|MAS3_SX);
+        mas7 = FSL_BOOKE_MAS7(CONFIG_SYS_DDR_SDRAM_BASE);
+
+        write_tlb(mas0, mas1, mas2, mas3, mas7);
+
+	/* 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);
+}
+
+phys_size_t fixed_sdram(void)
+{
+	uint64_t ram_size;
+
+	ram_size = get_linear_ram_size();
+
+	/*
+	 * At this point we know that we're running in RAM, but within the
+	 * first 64MB because we don't have a mapping for anything else.
+	 *
+	 * Replace that mapping with real maps for our full RAM range.
+	 */
+	fixup_tlb1(ram_size);
+
+	return ram_size;
+}
+
+phys_size_t fsl_ddr_sdram_size(void)
+{
+	return fixed_sdram();
+}
+
+void init_laws(void)
+{
+	/* We don't emulate LAWs yet */
+}
+
+int set_next_law(phys_addr_t addr, enum law_size sz, enum law_trgt_if id)
+{
+	/* We don't emulate LAWs yet */
+	return 0;
+}
diff --git a/board/freescale/qemu-ppce500/tlb.c b/board/freescale/qemu-ppce500/tlb.c
new file mode 100644
index 0000000..cdc7013
--- /dev/null
+++ b/board/freescale/qemu-ppce500/tlb.c
@@ -0,0 +1,59 @@
+/*
+ * Copyright 2008 Freescale Semiconductor, Inc.
+ *
+ * (C) Copyright 2000
+ * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <asm/mmu.h>
+
+struct fsl_e_tlb_entry tlb_table[] = {
+	/* TLB 0 - for temp stack in cache */
+	SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR, CONFIG_SYS_INIT_RAM_ADDR,
+		      MAS3_SX|MAS3_SW|MAS3_SR, 0,
+		      0, 0, BOOKE_PAGESZ_4K, 0),
+	SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR + 4 * 1024 , CONFIG_SYS_INIT_RAM_ADDR + 4 * 1024,
+		      MAS3_SX|MAS3_SW|MAS3_SR, 0,
+		      0, 0, BOOKE_PAGESZ_4K, 0),
+	SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR + 8 * 1024 , CONFIG_SYS_INIT_RAM_ADDR + 8 * 1024,
+		      MAS3_SX|MAS3_SW|MAS3_SR, 0,
+		      0, 0, BOOKE_PAGESZ_4K, 0),
+	SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR + 12 * 1024 , CONFIG_SYS_INIT_RAM_ADDR + 12 * 1024,
+		      MAS3_SX|MAS3_SW|MAS3_SR, 0,
+		      0, 0, BOOKE_PAGESZ_4K, 0),
+	/*
+	 * TLB 0:	64M	Cacheable
+	 * 0x00000000	64M	Covers RAM at 0x00000000
+	 */
+	SET_TLB_ENTRY(1, CONFIG_SYS_BOOT_BLOCK, CONFIG_SYS_BOOT_BLOCK,
+		      MAS3_SX|MAS3_SW|MAS3_SR, 0,
+		      0, 0, BOOKE_PAGESZ_64M, 1),
+
+	/*
+	 * TLB 1:	256M	Non-cacheable, guarded
+	 */
+	SET_TLB_ENTRY(1, CONFIG_SYS_PCI_VIRT, CONFIG_SYS_PCI_PHYS,
+		      MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
+		      0, 1, BOOKE_PAGESZ_256M, 1),
+
+	/*
+	 * TLB 2:	256M	Non-cacheable, guarded
+	 */
+	SET_TLB_ENTRY(1, CONFIG_SYS_PCI_VIRT + 0x10000000, CONFIG_SYS_PCI_PHYS + 0x10000000,
+		      MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
+		      0, 2, BOOKE_PAGESZ_256M, 1),
+
+	/*
+	 * TLB 3:	64M	Non-cacheable, guarded
+	 * 0xe000_0000	1M	CCSRBAR
+	 * 0xe100_0000	255M	PCI IO range
+	 */
+	SET_TLB_ENTRY(1, CONFIG_SYS_CCSRBAR, CONFIG_SYS_CCSRBAR_PHYS,
+		      MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
+		      0, 3, BOOKE_PAGESZ_64M, 1),
+};
+
+int num_tlb_entries = ARRAY_SIZE(tlb_table);
diff --git a/boards.cfg b/boards.cfg
index d177f82..ab50982 100644
--- a/boards.cfg
+++ b/boards.cfg
@@ -986,6 +986,7 @@ Active  powerpc     mpc85xx        -           freescale       t2080qds
 Active  powerpc     mpc85xx        -           freescale       t2080qds            T2080QDS_SPIFLASH     T2080QDS:PPC_T2080,RAMBOOT_PBL,SPIFLASH,SYS_TEXT_BASE=0xFFF80000
 Active  powerpc     mpc85xx        -           freescale       t2080qds            T2080QDS_NAND         T2080QDS:PPC_T2080,RAMBOOT_PBL,NAND,SYS_TEXT_BASE=0xFFF80000
 Active  powerpc     mpc85xx        -           freescale       t2080qds            T2080QDS_SRIO_PCIE_BOOT  T2080QDS:PPC_T2080,SRIO_PCIE_BOOT_SLAVE,SYS_TEXT_BASE=0xFFF80000
+Active  powerpc     mpc85xx        -           freescale       qemu-ppce500        qemu-ppce500                         -                                                                                                                                 Alexander Graf <agraf@suse.de>
 Active  powerpc     mpc85xx        -           gdsys           p1022               controlcenterd_36BIT_SDCARD          controlcenterd:36BIT,SDCARD                                                                                                       Dirk Eibach <eibach@gdsys.de>
 Active  powerpc     mpc85xx        -           gdsys           p1022               controlcenterd_36BIT_SDCARD_DEVELOP  controlcenterd:36BIT,SDCARD,DEVELOP                                                                                               Dirk Eibach <eibach@gdsys.de>
 Active  powerpc     mpc85xx        -           gdsys           p1022               controlcenterd_TRAILBLAZER           controlcenterd:TRAILBLAZER,SPIFLASH                                                                                               Dirk Eibach <eibach@gdsys.de>
diff --git a/include/configs/qemu-ppce500.h b/include/configs/qemu-ppce500.h
new file mode 100644
index 0000000..981b016
--- /dev/null
+++ b/include/configs/qemu-ppce500.h
@@ -0,0 +1,235 @@
+/*
+ * 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	0xf00000 /* 15 MB */
+
+#undef CONFIG_RESET_VECTOR_ADDRESS
+#define CONFIG_RESET_VECTOR_ADDRESS	(0x1000000 - 4) /* 16 MB */
+
+#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_INTERRUPTS		/* enable pci, srio, ddr interrupts */
+
+#define CONFIG_ENABLE_36BIT_PHYS
+
+#define CONFIG_ADDR_MAP
+#define CONFIG_SYS_NUM_ADDR_MAP		16	/* number of TLB1 entries */
+
+#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
+#define CONFIG_SYS_CCSRBAR_PHYS_LOW	CONFIG_SYS_CCSRBAR
+
+#define CONFIG_FW_CFG_ADDR		0xFF7FFFFC
+#define CONFIG_QEMU_DT_ADDR		((2 * 1024 * 1024) - 4) /* below 2MB */
+
+/*
+ * DDR Setup
+ */
+#define CONFIG_VERY_BIG_RAM
+#define CONFIG_SYS_DDR_SDRAM_BASE	0x00000000
+#define CONFIG_SYS_SDRAM_BASE		CONFIG_SYS_DDR_SDRAM_BASE
+
+/* 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
+
+/* Get RAM size from device tree */
+#define CONFIG_DDR_SPD
+
+#define CONFIG_SYS_CLK_FREQ        33000000
+
+
+/*
+ * IFC Definitions
+ */
+
+#define CONFIG_SYS_NO_FLASH
+
+#define CONFIG_SYS_BOOT_BLOCK		0x00000000	/* boot TLB */
+#define CONFIG_SYS_FLASH_BASE		0xff800000	/* start of FLASH 8M */
+#define CONFIG_SYS_FLASH_BASE_PHYS	(0xf00000000ull | CONFIG_SYS_FLASH_BASE)
+#define CONFIG_SYS_MAX_FLASH_BANKS	1		/* number of banks */
+#define CONFIG_SYS_MAX_FLASH_SECT	128		/* sectors per device */
+
+#define CONFIG_SYS_MONITOR_BASE		(CONFIG_RESET_VECTOR_ADDRESS - 0x100000 + 4)
+#define CONFIG_SYS_SDRAM_SIZE           64
+
+#define CONFIG_ENV_IS_NOWHERE
+
+#define CONFIG_HWCONFIG
+
+#define CONFIG_SYS_INIT_RAM_ADDR		0x00100000
+#define CONFIG_SYS_INIT_RAM_ADDR_PHYS_HIGH	0x0
+#define CONFIG_SYS_INIT_RAM_ADDR_PHYS_LOW	0x00100000
+/* The assembler doesn't like typecast */
+#define CONFIG_SYS_INIT_RAM_ADDR_PHYS \
+	((CONFIG_SYS_INIT_RAM_ADDR_PHYS_HIGH * 1ull << 32) | \
+	  CONFIG_SYS_INIT_RAM_ADDR_PHYS_LOW)
+#define CONFIG_SYS_INIT_RAM_SIZE		0x00004000
+
+#define CONFIG_SYS_GBL_DATA_OFFSET	(CONFIG_SYS_INIT_RAM_SIZE - \
+					GENERATED_GBL_DATA_SIZE)
+#define CONFIG_SYS_INIT_SP_OFFSET	CONFIG_SYS_GBL_DATA_OFFSET
+
+#define CONFIG_SYS_MONITOR_LEN		(512 * 1024)
+#define CONFIG_SYS_MALLOC_LEN		(4 * 1024 * 1024)
+
+#define CONFIG_CONS_INDEX	1
+#define CONFIG_SYS_NS16550
+#define CONFIG_SYS_NS16550_SERIAL
+#define CONFIG_SYS_NS16550_REG_SIZE	1
+#define CONFIG_SYS_NS16550_CLK		(get_bus_freq(0))
+
+#define CONFIG_SYS_BAUDRATE_TABLE	\
+	{300, 600, 1200, 2400, 4800, 9600, 19200, 38400, 57600, 115200}
+
+#define CONFIG_SYS_NS16550_COM1	(CONFIG_SYS_CCSRBAR+0x4500)
+#define CONFIG_SYS_NS16550_COM2	(CONFIG_SYS_CCSRBAR+0x4600)
+
+/* Use the HUSH parser */
+#define CONFIG_SYS_HUSH_PARSER
+#define CONFIG_SYS_PROMPT_HUSH_PS2 "> "
+
+/* pass open firmware flat tree */
+#define CONFIG_OF_LIBFDT
+#define CONFIG_OF_BOARD_SETUP
+#define CONFIG_OF_STDOUT_VIA_ALIAS
+
+/* new uImage format support */
+#define CONFIG_FIT
+#define CONFIG_FIT_VERBOSE	/* enable fit_format_{error,warning}() */
+
+/*
+ * General PCI
+ * Memory space is mapped 1-1, but I/O space must start from 0.
+ */
+
+#define CONFIG_SYS_PCI_VIRT		0xc0000000	/* 512M PCI TLB */
+#define CONFIG_SYS_PCI_PHYS		0xc0000000	/* 512M PCI TLB */
+
+#define CONFIG_SYS_PCI1_MEM_VIRT	0xc0000000
+#define CONFIG_SYS_PCI1_MEM_BUS	0xc0000000
+#define CONFIG_SYS_PCI1_MEM_PHYS	0xc0000000
+#define CONFIG_SYS_PCI1_MEM_SIZE	0x20000000	/* 512M */
+#define CONFIG_SYS_PCI1_IO_VIRT	0xe1000000
+#define CONFIG_SYS_PCI1_IO_BUS	0x00000000
+#define CONFIG_SYS_PCI1_IO_PHYS	0xe1000000
+#define CONFIG_SYS_PCI1_IO_SIZE	0x00010000	/* 64k */
+
+#ifdef CONFIG_PCI
+#define CONFIG_PCI_INDIRECT_BRIDGE
+#define CONFIG_NET_MULTI
+#define CONFIG_PCI_PNP			/* do pci plug-and-play */
+#define CONFIG_E1000
+
+#define CONFIG_PCI_SCAN_SHOW		/* show pci devices on startup */
+#define CONFIG_DOS_PARTITION
+#endif	/* CONFIG_PCI */
+
+#define CONFIG_LBA48
+#define CONFIG_DOS_PARTITION
+#define CONFIG_CMD_EXT2
+
+/*
+ * Environment
+ */
+#define CONFIG_ENV_SIZE		0x2000
+#define CONFIG_ENV_SECT_SIZE	0x10000 /* 64K (one sector) */
+
+#define CONFIG_LOADS_ECHO		/* echo on for serial download */
+#define CONFIG_SYS_LOADS_BAUD_CHANGE	/* allow baudrate change */
+
+#define CONFIG_LAST_STAGE_INIT
+
+/*
+ * Command line configuration.
+ */
+#include <config_cmd_default.h>
+
+#define CONFIG_CMD_DHCP
+#define CONFIG_CMD_ELF
+#define CONFIG_CMD_BOOTZ
+#define CONFIG_CMD_ERRATA
+#define CONFIG_CMD_GREPENV
+#define CONFIG_CMD_IRQ
+#define CONFIG_CMD_PING
+#define CONFIG_CMD_SETEXPR
+
+#ifdef CONFIG_PCI
+#define CONFIG_CMD_PCI
+#define CONFIG_CMD_NET
+#endif
+
+/*
+ * Miscellaneous configurable options
+ */
+#define CONFIG_SYS_LONGHELP			/* undef to save memory	*/
+#define CONFIG_CMDLINE_EDITING			/* Command-line editing */
+#define CONFIG_AUTO_COMPLETE			/* add autocompletion support */
+#define CONFIG_SYS_LOAD_ADDR	0x2000000	/* default load address */
+#ifdef CONFIG_CMD_KGDB
+#define CONFIG_SYS_CBSIZE	1024		/* Console I/O Buffer Size */
+#else
+#define CONFIG_SYS_CBSIZE	256		/* Console I/O Buffer Size */
+#endif
+#define CONFIG_SYS_PBSIZE (CONFIG_SYS_CBSIZE+sizeof(CONFIG_SYS_PROMPT)+16)
+#define CONFIG_SYS_MAXARGS	16		/* max number of command args */
+#define CONFIG_SYS_BARGSIZE	CONFIG_SYS_CBSIZE/* Boot Argument Buffer Size */
+
+/*
+ * For booting Linux, the board info and command line data
+ * have to be in the first 64 MB of memory, since this is
+ * the maximum mapped by the Linux kernel during initialization.
+ */
+#define CONFIG_SYS_BOOTMAPSZ	(64 << 20)	/* Initial map for Linux*/
+#define CONFIG_SYS_BOOTM_LEN	(64 << 20)	/* Increase max gunzip size */
+
+#ifdef CONFIG_CMD_KGDB
+#define CONFIG_KGDB_BAUDRATE	230400	/* speed to run kgdb serial port */
+#endif
+
+/*
+ * Environment Configuration
+ */
+#define CONFIG_ROOTPATH		"/opt/nfsroot"
+#define CONFIG_BOOTFILE		"uImage"
+#define CONFIG_UBOOTPATH	"u-boot.bin"	/* U-Boot image on TFTP server*/
+
+/* default location for tftp and bootm */
+#define CONFIG_LOADADDR		1000000
+
+#define CONFIG_BAUDRATE	115200
+
+#define CONFIG_BOOTDELAY        1
+#define CONFIG_BOOTCOMMAND		\
+	"test -n \"$kernel_addr\" && bootm $kernel_addr $ramdisk_addr $fdt_addr\0"
+
+#endif	/* __QEMU_PPCE500_H */
-- 
1.7.10.4

[U-Boot] [PATCH 3/3] PPC 85xx: Add qemu-ppce500 machine

[Scott Wood]

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 at h
> +	ori	r2, r2, CONFIG_QEMU_DT_ADDR at 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.

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?

> diff --git a/arch/powerpc/include/asm/config_mpc85xx.h b/arch/powerpc/include/asm/config_mpc85xx.h
> index 54ce2f0..a0ab453 100644
> --- a/arch/powerpc/include/asm/config_mpc85xx.h
> +++ b/arch/powerpc/include/asm/config_mpc85xx.h
> @@ -776,6 +776,10 @@ defined(CONFIG_PPC_T1020) || defined(CONFIG_PPC_T1022)
>  #define CONFIG_SYS_CCSRBAR_DEFAULT	0xff700000
>  #define CONFIG_SYS_FSL_ERRATUM_A005125
>  
> +#elif defined(CONFIG_QEMU_E500)
> +#define CONFIG_MAX_CPUS			1
> +#define CONFIG_SYS_CCSRBAR_DEFAULT	0xe0000000

This is supposed to come from the device tree.  We will want to change
that address eventually to support larger guest memory.

> diff --git a/board/freescale/qemu-ppce500/qemu-ppce500.c b/board/freescale/qemu-ppce500/qemu-ppce500.c
> new file mode 100644
> index 0000000..c6c4b4a
> --- /dev/null
> +++ b/board/freescale/qemu-ppce500/qemu-ppce500.c
> @@ -0,0 +1,260 @@
> +/*
> + * Copyright 2007,2009-2014 Freescale Semiconductor, Inc.
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <pci.h>
> +#include <asm/processor.h>
> +#include <asm/mmu.h>
> +#include <asm/immap_85xx.h>
> +#include <asm/fsl_pci.h>
> +#include <asm/io.h>
> +#include <miiphy.h>
> +#include <libfdt.h>
> +#include <fdt_support.h>
> +#include <netdev.h>
> +#include <fdtdec.h>
> +#include <errno.h>
> +#include <malloc.h>

Do you really need all these headers?  miiphy?

> +int checkboard(void)
> +{
> +	return 0;
> +}
> +
> +static struct pci_controller pci1_hose;
> +
> +void pci_init_board(void)
> +{
> +	volatile ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
> +	struct fsl_pci_info pci_info;
> +	u32 devdisr, pordevsr;
> +	u32 porpllsr, pci_agent, pci_speed, pci_32, pci_arb, pci_clk_sel;
> +	int first_free_busno = 0;
> +
> +	devdisr = in_be32(&gur->devdisr);
> +	pordevsr = in_be32(&gur->pordevsr);
> +	porpllsr = in_be32(&gur->porpllsr);

Why are you reading registers that don't exist in QEMU?

> +int last_stage_init(void)
> +{
> +	int ret;
> +	int len = 0;
> +	const void *prop;
> +	int chosen;
> +        uint32_t *dt_base_ptr = (void*)CONFIG_QEMU_DT_ADDR;

Whitespace

> +	uint32_t dt_base = *dt_base_ptr;
> +	uint32_t dt_size;
> +	void *fdt;
> +
> +	dt_size = fdt_totalsize((void*)dt_base);

Please put a space before the * in casts.

> +	/* Reserve 4k for dtb tweaks */
> +	dt_size += 4096;
> +	fdt = malloc(dt_size);
> +
> +	/* Open device tree */
> +	ret = fdt_open_into((void*)dt_base, fdt, dt_size);
> +	if (ret) {
> +		printf("Couldn't open fdt at %x\n", dt_base);
> +		return -EIO;
> +	}
> +
> +	chosen = fdt_path_offset(fdt, "/chosen");
> +	if (chosen < 0) {
> +		printf("Couldn't find /chosen node in fdt\n");
> +		return -EIO;
> +	}
> +
> +	prop = fdt_getprop(fdt, chosen, "qemu,boot-kernel", &len);
> +	if (prop && (len >= 8)) {
> +		/* -kernel boot */
> +		setenv_hex("kernel_addr", *(uint64_t*)prop);

Don't cast away the const.  This looks like the only place you use prop;
why not make it uint64_t to begin with?

> +		/*
> +		 * 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.

> +	}
> +
> +	/* Give the user a variable for the host fdt */
> +	setenv_hex("fdt_addr", (int)dt_base);

Unnecessary cast.

> +
> +	free(fdt);
> +
> +	return 0;
> +}

Why is the device tree handling here different from anything else we
already have?  In particular, why do you create a temporary fdt (with
space for tweaks that never happen) just to read from it?

> +static uint64_t get_linear_ram_size(void)
> +{
> +	const void *prop;
> +	int memory;
> +	int len;
> +        uint32_t *dt_base_ptr = (void*)CONFIG_QEMU_DT_ADDR;

Whitespace.

> +	void *fdt = &dt_base_ptr[1];
>
> +	int ret;
> +	uint32_t dt_base = *dt_base_ptr;
> +	uint32_t dt_size = 1024 * 1024;
> +
> +	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)?

> +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?

> +int board_phy_config(struct phy_device *phydev)
> +{
> +	return 0;
> +}

Why?

mpc8544ds is the only board in boards/freescale that has this, so it's
clearly optional, and you clearly don't need it...

Just because QEMU started with mpc8544ds doesn't mean this needs to be a
copy-and-paste of the U-Boot mpc8544ds code.  In fact it's better if it
isn't to reduce the likelihood of accidentally depending on hardcoded
addresses and such.

> +int board_eth_init(bd_t *bis)
> +{
> +	return pci_eth_init(bis);
> +}
> +
> +#if defined(CONFIG_OF_BOARD_SETUP)
> +void ft_board_setup(void *blob, bd_t *bd)
> +{
> +	FT_FSL_PCI_SETUP;
> +}
> +#endif
> +
> +void print_laws(void)
> +{
> +	/* We don't emulate LAWs yet */
> +}
> +
> +static void fixup_tlb1(uint64_t ram_size)
> +{
> +	u32 mas0, mas1, mas2, mas3, mas7;
> +	long tmp;
> +
> +	/* Flush TLB0 - it only contains stale maps by now */
> +	invalidate_tlb(0);
> +
> +	/* Create a temporary AS=1 map for ourselves */
> +        mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(13);
> +        mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TS | MAS1_TSIZE(BOOKE_PAGESZ_64M);
> +        mas2 = FSL_BOOKE_MAS2(0, 0);
> +        mas3 = FSL_BOOKE_MAS3(CONFIG_SYS_DDR_SDRAM_BASE, 0,
> +			      MAS3_SW|MAS3_SR|MAS3_SX);
> +        mas7 = FSL_BOOKE_MAS7(CONFIG_SYS_DDR_SDRAM_BASE);
> +
> +        write_tlb(mas0, mas1, mas2, mas3, mas7);

Whitespace.  Please run scripts/checkpatch.pl.

> +
> +	/* 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?

> +phys_size_t fixed_sdram(void)
> +{
> +	uint64_t ram_size;
> +
> +	ram_size = get_linear_ram_size();
> +
> +	/*
> +	 * At this point we know that we're running in RAM, but within the
> +	 * first 64MB because we don't have a mapping for anything else.
> +	 *
> +	 * Replace that mapping with real maps for our full RAM range.
> +	 */
> +	fixup_tlb1(ram_size);
> +
> +	return ram_size;
> +}
> +
> +phys_size_t fsl_ddr_sdram_size(void)
> +{
> +	return fixed_sdram();
> +}
> +
> +void init_laws(void)
> +{
> +	/* We don't emulate LAWs yet */
> +}
> +
> +int set_next_law(phys_addr_t addr, enum law_size sz, enum law_trgt_if id)
> +{
> +	/* We don't emulate LAWs yet */
> +	return 0;
> +}

What is calling set_next_law()?

> diff --git a/board/freescale/qemu-ppce500/tlb.c b/board/freescale/qemu-ppce500/tlb.c
> new file mode 100644
> index 0000000..cdc7013
> --- /dev/null
> +++ b/board/freescale/qemu-ppce500/tlb.c
> @@ -0,0 +1,59 @@
> +/*
> + * Copyright 2008 Freescale Semiconductor, Inc.
> + *
> + * (C) Copyright 2000
> + * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <asm/mmu.h>
> +
> +struct fsl_e_tlb_entry tlb_table[] = {
> +	/* TLB 0 - for temp stack in cache */
> +	SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR, CONFIG_SYS_INIT_RAM_ADDR,
> +		      MAS3_SX|MAS3_SW|MAS3_SR, 0,
> +		      0, 0, BOOKE_PAGESZ_4K, 0),
> +	SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR + 4 * 1024 , CONFIG_SYS_INIT_RAM_ADDR + 4 * 1024,
> +		      MAS3_SX|MAS3_SW|MAS3_SR, 0,
> +		      0, 0, BOOKE_PAGESZ_4K, 0),
> +	SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR + 8 * 1024 , CONFIG_SYS_INIT_RAM_ADDR + 8 * 1024,
> +		      MAS3_SX|MAS3_SW|MAS3_SR, 0,
> +		      0, 0, BOOKE_PAGESZ_4K, 0),
> +	SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR + 12 * 1024 , CONFIG_SYS_INIT_RAM_ADDR + 12 * 1024,
> +		      MAS3_SX|MAS3_SW|MAS3_SR, 0,
> +		      0, 0, BOOKE_PAGESZ_4K, 0),

Isn't this just normal RAM?  Thus, aren't you creating overlapping TLB
entries here?

> +	/*
> +	 * TLB 0:	64M	Cacheable
> +	 * 0x00000000	64M	Covers RAM at 0x00000000
> +	 */
> +	SET_TLB_ENTRY(1, CONFIG_SYS_BOOT_BLOCK, CONFIG_SYS_BOOT_BLOCK,
> +		      MAS3_SX|MAS3_SW|MAS3_SR, 0,
> +		      0, 0, BOOKE_PAGESZ_64M, 1),
> +
> +	/*
> +	 * TLB 1:	256M	Non-cacheable, guarded
> +	 */
> +	SET_TLB_ENTRY(1, CONFIG_SYS_PCI_VIRT, CONFIG_SYS_PCI_PHYS,
> +		      MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
> +		      0, 1, BOOKE_PAGESZ_256M, 1),
> +
> +	/*
> +	 * TLB 2:	256M	Non-cacheable, guarded
> +	 */
> +	SET_TLB_ENTRY(1, CONFIG_SYS_PCI_VIRT + 0x10000000, CONFIG_SYS_PCI_PHYS + 0x10000000,
> +		      MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
> +		      0, 2, BOOKE_PAGESZ_256M, 1),
> +
> +	/*
> +	 * TLB 3:	64M	Non-cacheable, guarded
> +	 * 0xe000_0000	1M	CCSRBAR
> +	 * 0xe100_0000	255M	PCI IO range
> +	 */
> +	SET_TLB_ENTRY(1, CONFIG_SYS_CCSRBAR, CONFIG_SYS_CCSRBAR_PHYS,
> +		      MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
> +		      0, 3, BOOKE_PAGESZ_64M, 1),
> +};

These should not be executable.


> +
> +int num_tlb_entries = ARRAY_SIZE(tlb_table);
> diff --git a/boards.cfg b/boards.cfg
> index d177f82..ab50982 100644
> --- a/boards.cfg
> +++ b/boards.cfg
> @@ -986,6 +986,7 @@ Active  powerpc     mpc85xx        -           freescale       t2080qds
>  Active  powerpc     mpc85xx        -           freescale       t2080qds            T2080QDS_SPIFLASH     T2080QDS:PPC_T2080,RAMBOOT_PBL,SPIFLASH,SYS_TEXT_BASE=0xFFF80000
>  Active  powerpc     mpc85xx        -           freescale       t2080qds            T2080QDS_NAND         T2080QDS:PPC_T2080,RAMBOOT_PBL,NAND,SYS_TEXT_BASE=0xFFF80000
>  Active  powerpc     mpc85xx        -           freescale       t2080qds            T2080QDS_SRIO_PCIE_BOOT  T2080QDS:PPC_T2080,SRIO_PCIE_BOOT_SLAVE,SYS_TEXT_BASE=0xFFF80000
> +Active  powerpc     mpc85xx        -           freescale       qemu-ppce500        qemu-ppce500                         -                                                                                                                                 Alexander Graf <agraf@suse.de>
>  Active  powerpc     mpc85xx        -           gdsys           p1022               controlcenterd_36BIT_SDCARD          controlcenterd:36BIT,SDCARD                                                                                                       Dirk Eibach <eibach@gdsys.de>
>  Active  powerpc     mpc85xx        -           gdsys           p1022               controlcenterd_36BIT_SDCARD_DEVELOP  controlcenterd:36BIT,SDCARD,DEVELOP                                                                                               Dirk Eibach <eibach@gdsys.de>
>  Active  powerpc     mpc85xx        -           gdsys           p1022               controlcenterd_TRAILBLAZER           controlcenterd:TRAILBLAZER,SPIFLASH                                                                                               Dirk Eibach <eibach@gdsys.de>
> diff --git a/include/configs/qemu-ppce500.h b/include/configs/qemu-ppce500.h
> new file mode 100644
> index 0000000..981b016
> --- /dev/null
> +++ b/include/configs/qemu-ppce500.h
> @@ -0,0 +1,235 @@
> +/*
> + * 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	0xf00000 /* 15 MB */
> +
> +#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?

> +#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_INTERRUPTS		/* enable pci, srio, ddr interrupts */

SRIO and DDR interrupts?

Why do we need CONFIG_INTERRUPTS at all?

> +#define CONFIG_SYS_CCSRBAR		0xe0000000
> +#define CONFIG_SYS_CCSRBAR_PHYS_LOW	CONFIG_SYS_CCSRBAR

Again, shouldn't be hardcoded (and must equal the default since QEMU
doesn't currently support relocating CCSR)

> +#define CONFIG_FW_CFG_ADDR		0xFF7FFFFC

Where did this come from?  If this is a new paravirt device please
advertise it through the device tree rather than a hardcoded address.
And if it must be a hardcoded address, please put it well above 4G.

> +#define CONFIG_QEMU_DT_ADDR		((2 * 1024 * 1024) - 4) /* below 2MB */

This is U-Boot-internal and not something hardcoded in QEMU, right?

> +/* 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?

> +/* 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.

> +
> +
> +/*
> + * IFC Definitions
> + */

There is no IFC.

> +#define CONFIG_SYS_NO_FLASH
> +
> +#define CONFIG_SYS_BOOT_BLOCK		0x00000000	/* boot TLB */
> +#define CONFIG_SYS_FLASH_BASE		0xff800000	/* start of FLASH 8M */
> +#define CONFIG_SYS_FLASH_BASE_PHYS	(0xf00000000ull | CONFIG_SYS_FLASH_BASE)
> +#define CONFIG_SYS_MAX_FLASH_BANKS	1		/* number of banks */
> +#define CONFIG_SYS_MAX_FLASH_SECT	128		/* sectors per device */

NO_FLASH, but flash starts at 0xff800000?

> +#define CONFIG_SYS_MONITOR_BASE		(CONFIG_RESET_VECTOR_ADDRESS - 0x100000 + 4)
> +#define CONFIG_SYS_SDRAM_SIZE           64

Why hardcoded to 64 MiB?

> +/*
> + * General PCI
> + * Memory space is mapped 1-1, but I/O space must start from 0.
> + */
> +
> +#define CONFIG_SYS_PCI_VIRT		0xc0000000	/* 512M PCI TLB */
> +#define CONFIG_SYS_PCI_PHYS		0xc0000000	/* 512M PCI TLB */
> +
> +#define CONFIG_SYS_PCI1_MEM_VIRT	0xc0000000
> +#define CONFIG_SYS_PCI1_MEM_BUS	0xc0000000
> +#define CONFIG_SYS_PCI1_MEM_PHYS	0xc0000000
> +#define CONFIG_SYS_PCI1_MEM_SIZE	0x20000000	/* 512M */
> +#define CONFIG_SYS_PCI1_IO_VIRT	0xe1000000
> +#define CONFIG_SYS_PCI1_IO_BUS	0x00000000
> +#define CONFIG_SYS_PCI1_IO_PHYS	0xe1000000
> +#define CONFIG_SYS_PCI1_IO_SIZE	0x00010000	/* 64k */

I assume U-Boot will reprogram PCI based on this, but does QEMU support
that for I/O (for memory IIRC it completely ignores the ATMU)?  Don't
rely on the QEMU address not changing.

> +/*
> + * Environment
> + */
> +#define CONFIG_ENV_SIZE		0x2000
> +#define CONFIG_ENV_SECT_SIZE	0x10000 /* 64K (one sector) */

Even though ENV_IS_NOWHERE. :-)

> +#define CONFIG_LOADS_ECHO		/* echo on for serial download */
> +#define CONFIG_SYS_LOADS_BAUD_CHANGE	/* allow baudrate change */

Baud rate?

> +#define CONFIG_CMD_DHCP
> +#define CONFIG_CMD_ELF
> +#define CONFIG_CMD_BOOTZ
> +#define CONFIG_CMD_ERRATA
> +#define CONFIG_CMD_GREPENV
> +#define CONFIG_CMD_IRQ
> +#define CONFIG_CMD_PING
> +#define CONFIG_CMD_SETEXPR

Errata?

> +#ifdef CONFIG_CMD_KGDB
> +#define CONFIG_SYS_CBSIZE	1024		/* Console I/O Buffer Size */
> +#else
> +#define CONFIG_SYS_CBSIZE	256		/* Console I/O Buffer Size */
> +#endif

KGDB?

> +#ifdef CONFIG_CMD_KGDB
> +#define CONFIG_KGDB_BAUDRATE	230400	/* speed to run kgdb serial port */
> +#endif

This is copy-and-paste cruft even on the non-QEMU targets.

-Scott

[U-Boot] [PATCH 3/3] PPC 85xx: Add qemu-ppce500 machine

[Alexander Graf]

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 at h
>> +	ori	r2, r2, CONFIG_QEMU_DT_ADDR at 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.

> 
>> diff --git a/arch/powerpc/include/asm/config_mpc85xx.h b/arch/powerpc/include/asm/config_mpc85xx.h
>> index 54ce2f0..a0ab453 100644
>> --- a/arch/powerpc/include/asm/config_mpc85xx.h
>> +++ b/arch/powerpc/include/asm/config_mpc85xx.h
>> @@ -776,6 +776,10 @@ defined(CONFIG_PPC_T1020) || defined(CONFIG_PPC_T1022)
>> #define CONFIG_SYS_CCSRBAR_DEFAULT	0xff700000
>> #define CONFIG_SYS_FSL_ERRATUM_A005125
>> 
>> +#elif defined(CONFIG_QEMU_E500)
>> +#define CONFIG_MAX_CPUS			1
>> +#define CONFIG_SYS_CCSRBAR_DEFAULT	0xe0000000
> 
> This is supposed to come from the device tree.  We will want to change
> that address eventually to support larger guest memory.

That's a lot easier said than done. There is a lot of code in u-boot that puts the CCSRBAR or addresses derived from CCSRBAR into arrays which fails miserably when you make it a variable.

It's certainly a reasonably big task.

> 
>> diff --git a/board/freescale/qemu-ppce500/qemu-ppce500.c b/board/freescale/qemu-ppce500/qemu-ppce500.c
>> new file mode 100644
>> index 0000000..c6c4b4a
>> --- /dev/null
>> +++ b/board/freescale/qemu-ppce500/qemu-ppce500.c
>> @@ -0,0 +1,260 @@
>> +/*
>> + * Copyright 2007,2009-2014 Freescale Semiconductor, Inc.
>> + *
>> + * SPDX-License-Identifier:	GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <command.h>
>> +#include <pci.h>
>> +#include <asm/processor.h>
>> +#include <asm/mmu.h>
>> +#include <asm/immap_85xx.h>
>> +#include <asm/fsl_pci.h>
>> +#include <asm/io.h>
>> +#include <miiphy.h>
>> +#include <libfdt.h>
>> +#include <fdt_support.h>
>> +#include <netdev.h>
>> +#include <fdtdec.h>
>> +#include <errno.h>
>> +#include <malloc.h>
> 
> Do you really need all these headers?  miiphy?

Ah, I missed that one during my header cleaning :). Reved that and immap_85xx.h.

> 
>> +int checkboard(void)
>> +{
>> +	return 0;
>> +}
>> +
>> +static struct pci_controller pci1_hose;
>> +
>> +void pci_init_board(void)
>> +{
>> +	volatile ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
>> +	struct fsl_pci_info pci_info;
>> +	u32 devdisr, pordevsr;
>> +	u32 porpllsr, pci_agent, pci_speed, pci_32, pci_arb, pci_clk_sel;
>> +	int first_free_busno = 0;
>> +
>> +	devdisr = in_be32(&gur->devdisr);
>> +	pordevsr = in_be32(&gur->pordevsr);
>> +	porpllsr = in_be32(&gur->porpllsr);
> 
> Why are you reading registers that don't exist in QEMU?

Dropped

> 
>> +int last_stage_init(void)
>> +{
>> +	int ret;
>> +	int len = 0;
>> +	const void *prop;
>> +	int chosen;
>> +        uint32_t *dt_base_ptr = (void*)CONFIG_QEMU_DT_ADDR;
> 
> Whitespace
> 
>> +	uint32_t dt_base = *dt_base_ptr;
>> +	uint32_t dt_size;
>> +	void *fdt;
>> +
>> +	dt_size = fdt_totalsize((void*)dt_base);
> 
> Please put a space before the * in casts.
> 
>> +	/* Reserve 4k for dtb tweaks */
>> +	dt_size += 4096;
>> +	fdt = malloc(dt_size);
>> +
>> +	/* Open device tree */
>> +	ret = fdt_open_into((void*)dt_base, fdt, dt_size);
>> +	if (ret) {
>> +		printf("Couldn't open fdt at %x\n", dt_base);
>> +		return -EIO;
>> +	}
>> +
>> +	chosen = fdt_path_offset(fdt, "/chosen");
>> +	if (chosen < 0) {
>> +		printf("Couldn't find /chosen node in fdt\n");
>> +		return -EIO;
>> +	}
>> +
>> +	prop = fdt_getprop(fdt, chosen, "qemu,boot-kernel", &len);
>> +	if (prop && (len >= 8)) {
>> +		/* -kernel boot */
>> +		setenv_hex("kernel_addr", *(uint64_t*)prop);
> 
> Don't cast away the const.  This looks like the only place you use prop;
> why not make it uint64_t to begin with?

Good idea. Fixed.

> 
>> +		/*
>> +		 * 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?

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 :).

> 
>> +	}
>> +
>> +	/* Give the user a variable for the host fdt */
>> +	setenv_hex("fdt_addr", (int)dt_base);
> 
> Unnecessary cast.
> 
>> +
>> +	free(fdt);
>> +
>> +	return 0;
>> +}
> 
> Why is the device tree handling here different from anything else we
> already have?  In particular, why do you create a temporary fdt (with
> space for tweaks that never happen) just to read from it?

Because I had no idea :). Changed to directly use the in-memory fdt.

> 
>> +static uint64_t get_linear_ram_size(void)
>> +{
>> +	const void *prop;
>> +	int memory;
>> +	int len;
>> +        uint32_t *dt_base_ptr = (void*)CONFIG_QEMU_DT_ADDR;
> 
> Whitespace.
> 
>> +	void *fdt = &dt_base_ptr[1];
>> 
>> +	int ret;
>> +	uint32_t dt_base = *dt_base_ptr;
>> +	uint32_t dt_size = 1024 * 1024;
>> +
>> +	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?

> 
>> +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.

> 
>> +int board_phy_config(struct phy_device *phydev)
>> +{
>> +	return 0;
>> +}
> 
> Why?
> 
> mpc8544ds is the only board in boards/freescale that has this, so it's
> clearly optional, and you clearly don't need it...
> 
> Just because QEMU started with mpc8544ds doesn't mean this needs to be a
> copy-and-paste of the U-Boot mpc8544ds code.  In fact it's better if it
> isn't to reduce the likelihood of accidentally depending on hardcoded
> addresses and such.

Yup. I just had to start from somewhere :). Removed.

> 
>> +int board_eth_init(bd_t *bis)
>> +{
>> +	return pci_eth_init(bis);
>> +}
>> +
>> +#if defined(CONFIG_OF_BOARD_SETUP)
>> +void ft_board_setup(void *blob, bd_t *bd)
>> +{
>> +	FT_FSL_PCI_SETUP;
>> +}
>> +#endif
>> +
>> +void print_laws(void)
>> +{
>> +	/* We don't emulate LAWs yet */
>> +}
>> +
>> +static void fixup_tlb1(uint64_t ram_size)
>> +{
>> +	u32 mas0, mas1, mas2, mas3, mas7;
>> +	long tmp;
>> +
>> +	/* Flush TLB0 - it only contains stale maps by now */
>> +	invalidate_tlb(0);
>> +
>> +	/* Create a temporary AS=1 map for ourselves */
>> +        mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(13);
>> +        mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TS | MAS1_TSIZE(BOOKE_PAGESZ_64M);
>> +        mas2 = FSL_BOOKE_MAS2(0, 0);
>> +        mas3 = FSL_BOOKE_MAS3(CONFIG_SYS_DDR_SDRAM_BASE, 0,
>> +			      MAS3_SW|MAS3_SR|MAS3_SX);
>> +        mas7 = FSL_BOOKE_MAS7(CONFIG_SYS_DDR_SDRAM_BASE);
>> +
>> +        write_tlb(mas0, mas1, mas2, mas3, mas7);
> 
> Whitespace.  Please run scripts/checkpatch.pl.

I shouldn't copy&paste I suppose :).

> 
>> +
>> +	/* 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.

> 
>> +phys_size_t fixed_sdram(void)
>> +{
>> +	uint64_t ram_size;
>> +
>> +	ram_size = get_linear_ram_size();
>> +
>> +	/*
>> +	 * At this point we know that we're running in RAM, but within the
>> +	 * first 64MB because we don't have a mapping for anything else.
>> +	 *
>> +	 * Replace that mapping with real maps for our full RAM range.
>> +	 */
>> +	fixup_tlb1(ram_size);
>> +
>> +	return ram_size;
>> +}
>> +
>> +phys_size_t fsl_ddr_sdram_size(void)
>> +{
>> +	return fixed_sdram();
>> +}
>> +
>> +void init_laws(void)
>> +{
>> +	/* We don't emulate LAWs yet */
>> +}
>> +
>> +int set_next_law(phys_addr_t addr, enum law_size sz, enum law_trgt_if id)
>> +{
>> +	/* We don't emulate LAWs yet */
>> +	return 0;
>> +}
> 
> What is calling set_next_law()?

Nothing anymore apparently. Removed.

> 
>> diff --git a/board/freescale/qemu-ppce500/tlb.c b/board/freescale/qemu-ppce500/tlb.c
>> new file mode 100644
>> index 0000000..cdc7013
>> --- /dev/null
>> +++ b/board/freescale/qemu-ppce500/tlb.c
>> @@ -0,0 +1,59 @@
>> +/*
>> + * Copyright 2008 Freescale Semiconductor, Inc.
>> + *
>> + * (C) Copyright 2000
>> + * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
>> + *
>> + * SPDX-License-Identifier:	GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <asm/mmu.h>
>> +
>> +struct fsl_e_tlb_entry tlb_table[] = {
>> +	/* TLB 0 - for temp stack in cache */
>> +	SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR, CONFIG_SYS_INIT_RAM_ADDR,
>> +		      MAS3_SX|MAS3_SW|MAS3_SR, 0,
>> +		      0, 0, BOOKE_PAGESZ_4K, 0),
>> +	SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR + 4 * 1024 , CONFIG_SYS_INIT_RAM_ADDR + 4 * 1024,
>> +		      MAS3_SX|MAS3_SW|MAS3_SR, 0,
>> +		      0, 0, BOOKE_PAGESZ_4K, 0),
>> +	SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR + 8 * 1024 , CONFIG_SYS_INIT_RAM_ADDR + 8 * 1024,
>> +		      MAS3_SX|MAS3_SW|MAS3_SR, 0,
>> +		      0, 0, BOOKE_PAGESZ_4K, 0),
>> +	SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR + 12 * 1024 , CONFIG_SYS_INIT_RAM_ADDR + 12 * 1024,
>> +		      MAS3_SX|MAS3_SW|MAS3_SR, 0,
>> +		      0, 0, BOOKE_PAGESZ_4K, 0),
> 
> Isn't this just normal RAM?  Thus, aren't you creating overlapping TLB
> entries here?

Indeed. Removed.

> 
>> +	/*
>> +	 * TLB 0:	64M	Cacheable
>> +	 * 0x00000000	64M	Covers RAM at 0x00000000
>> +	 */
>> +	SET_TLB_ENTRY(1, CONFIG_SYS_BOOT_BLOCK, CONFIG_SYS_BOOT_BLOCK,
>> +		      MAS3_SX|MAS3_SW|MAS3_SR, 0,
>> +		      0, 0, BOOKE_PAGESZ_64M, 1),
>> +
>> +	/*
>> +	 * TLB 1:	256M	Non-cacheable, guarded
>> +	 */
>> +	SET_TLB_ENTRY(1, CONFIG_SYS_PCI_VIRT, CONFIG_SYS_PCI_PHYS,
>> +		      MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
>> +		      0, 1, BOOKE_PAGESZ_256M, 1),
>> +
>> +	/*
>> +	 * TLB 2:	256M	Non-cacheable, guarded
>> +	 */
>> +	SET_TLB_ENTRY(1, CONFIG_SYS_PCI_VIRT + 0x10000000, CONFIG_SYS_PCI_PHYS + 0x10000000,
>> +		      MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
>> +		      0, 2, BOOKE_PAGESZ_256M, 1),
>> +
>> +	/*
>> +	 * TLB 3:	64M	Non-cacheable, guarded
>> +	 * 0xe000_0000	1M	CCSRBAR
>> +	 * 0xe100_0000	255M	PCI IO range
>> +	 */
>> +	SET_TLB_ENTRY(1, CONFIG_SYS_CCSRBAR, CONFIG_SYS_CCSRBAR_PHYS,
>> +		      MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
>> +		      0, 3, BOOKE_PAGESZ_64M, 1),
>> +};
> 
> These should not be executable.

Aww :). Fixed.

> 
> 
>> +
>> +int num_tlb_entries = ARRAY_SIZE(tlb_table);
>> diff --git a/boards.cfg b/boards.cfg
>> index d177f82..ab50982 100644
>> --- a/boards.cfg
>> +++ b/boards.cfg
>> @@ -986,6 +986,7 @@ Active  powerpc     mpc85xx        -           freescale       t2080qds
>> Active  powerpc     mpc85xx        -           freescale       t2080qds            T2080QDS_SPIFLASH     T2080QDS:PPC_T2080,RAMBOOT_PBL,SPIFLASH,SYS_TEXT_BASE=0xFFF80000
>> Active  powerpc     mpc85xx        -           freescale       t2080qds            T2080QDS_NAND         T2080QDS:PPC_T2080,RAMBOOT_PBL,NAND,SYS_TEXT_BASE=0xFFF80000
>> Active  powerpc     mpc85xx        -           freescale       t2080qds            T2080QDS_SRIO_PCIE_BOOT  T2080QDS:PPC_T2080,SRIO_PCIE_BOOT_SLAVE,SYS_TEXT_BASE=0xFFF80000
>> +Active  powerpc     mpc85xx        -           freescale       qemu-ppce500        qemu-ppce500                         -                                                                                                                                 Alexander Graf <agraf@suse.de>
>> Active  powerpc     mpc85xx        -           gdsys           p1022               controlcenterd_36BIT_SDCARD          controlcenterd:36BIT,SDCARD                                                                                                       Dirk Eibach <eibach@gdsys.de>
>> Active  powerpc     mpc85xx        -           gdsys           p1022               controlcenterd_36BIT_SDCARD_DEVELOP  controlcenterd:36BIT,SDCARD,DEVELOP                                                                                               Dirk Eibach <eibach@gdsys.de>
>> Active  powerpc     mpc85xx        -           gdsys           p1022               controlcenterd_TRAILBLAZER           controlcenterd:TRAILBLAZER,SPIFLASH                                                                                               Dirk Eibach <eibach@gdsys.de>
>> diff --git a/include/configs/qemu-ppce500.h b/include/configs/qemu-ppce500.h
>> new file mode 100644
>> index 0000000..981b016
>> --- /dev/null
>> +++ b/include/configs/qemu-ppce500.h
>> @@ -0,0 +1,235 @@
>> +/*
>> + * 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	0xf00000 /* 15 MB */
>> +
>> +#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.

> 
>> +#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_INTERRUPTS		/* enable pci, srio, ddr interrupts */
> 
> SRIO and DDR interrupts?
> 
> Why do we need CONFIG_INTERRUPTS at all?

We don't. Removed.

> 
>> +#define CONFIG_SYS_CCSRBAR		0xe0000000
>> +#define CONFIG_SYS_CCSRBAR_PHYS_LOW	CONFIG_SYS_CCSRBAR
> 
> Again, shouldn't be hardcoded (and must equal the default since QEMU
> doesn't currently support relocating CCSR)

Yes.

> 
>> +#define CONFIG_FW_CFG_ADDR		0xFF7FFFFC
> 
> Where did this come from?  If this is a new paravirt device please
> advertise it through the device tree rather than a hardcoded address.
> And if it must be a hardcoded address, please put it well above 4G.

It's a leftover from a time when I used fw_cfg to fetch the dt pointer. Removed.

> 
>> +#define CONFIG_QEMU_DT_ADDR		((2 * 1024 * 1024) - 4) /* below 2MB */
> 
> This is U-Boot-internal and not something hardcoded in QEMU, right?

Yes, and it's gone now :).

> 
>> +/* 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.

> 
>> +/* 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)

> 
>> +
>> +
>> +/*
>> + * IFC Definitions
>> + */
> 
> There is no IFC.

Removed.

> 
>> +#define CONFIG_SYS_NO_FLASH
>> +
>> +#define CONFIG_SYS_BOOT_BLOCK		0x00000000	/* boot TLB */
>> +#define CONFIG_SYS_FLASH_BASE		0xff800000	/* start of FLASH 8M */
>> +#define CONFIG_SYS_FLASH_BASE_PHYS	(0xf00000000ull | CONFIG_SYS_FLASH_BASE)
>> +#define CONFIG_SYS_MAX_FLASH_BANKS	1		/* number of banks */
>> +#define CONFIG_SYS_MAX_FLASH_SECT	128		/* sectors per device */
> 
> NO_FLASH, but flash starts at 0xff800000?

Yeah, I was experimenting with putting up a flash there. Removed.

> 
>> +#define CONFIG_SYS_MONITOR_BASE		(CONFIG_RESET_VECTOR_ADDRESS - 0x100000 + 4)
>> +#define CONFIG_SYS_SDRAM_SIZE           64
> 
> Why hardcoded to 64 MiB?

The define wasn't needed anymore. Removed.

> 
>> +/*
>> + * General PCI
>> + * Memory space is mapped 1-1, but I/O space must start from 0.
>> + */
>> +
>> +#define CONFIG_SYS_PCI_VIRT		0xc0000000	/* 512M PCI TLB */
>> +#define CONFIG_SYS_PCI_PHYS		0xc0000000	/* 512M PCI TLB */
>> +
>> +#define CONFIG_SYS_PCI1_MEM_VIRT	0xc0000000
>> +#define CONFIG_SYS_PCI1_MEM_BUS	0xc0000000
>> +#define CONFIG_SYS_PCI1_MEM_PHYS	0xc0000000
>> +#define CONFIG_SYS_PCI1_MEM_SIZE	0x20000000	/* 512M */
>> +#define CONFIG_SYS_PCI1_IO_VIRT	0xe1000000
>> +#define CONFIG_SYS_PCI1_IO_BUS	0x00000000
>> +#define CONFIG_SYS_PCI1_IO_PHYS	0xe1000000
>> +#define CONFIG_SYS_PCI1_IO_SIZE	0x00010000	/* 64k */
> 
> I assume U-Boot will reprogram PCI based on this, but does QEMU support
> that for I/O (for memory IIRC it completely ignores the ATMU)?  Don't
> rely on the QEMU address not changing.

It populated its PCI accessor struct based on this. This ideally should be dt based. But for now we have something that works at all :).

> 
>> +/*
>> + * Environment
>> + */
>> +#define CONFIG_ENV_SIZE		0x2000
>> +#define CONFIG_ENV_SECT_SIZE	0x10000 /* 64K (one sector) */
> 
> Even though ENV_IS_NOWHERE. :-)

Removed :)

> 
>> +#define CONFIG_LOADS_ECHO		/* echo on for serial download */
>> +#define CONFIG_SYS_LOADS_BAUD_CHANGE	/* allow baudrate change */
> 
> Baud rate?

Removed.

> 
>> +#define CONFIG_CMD_DHCP
>> +#define CONFIG_CMD_ELF
>> +#define CONFIG_CMD_BOOTZ
>> +#define CONFIG_CMD_ERRATA
>> +#define CONFIG_CMD_GREPENV
>> +#define CONFIG_CMD_IRQ
>> +#define CONFIG_CMD_PING
>> +#define CONFIG_CMD_SETEXPR
> 
> Errata?

Eh - removed :)

> 
>> +#ifdef CONFIG_CMD_KGDB
>> +#define CONFIG_SYS_CBSIZE	1024		/* Console I/O Buffer Size */
>> +#else
>> +#define CONFIG_SYS_CBSIZE	256		/* Console I/O Buffer Size */
>> +#endif
> 
> KGDB?

*shrug* I just copied this from somewhere.

> 
>> +#ifdef CONFIG_CMD_KGDB
>> +#define CONFIG_KGDB_BAUDRATE	230400	/* speed to run kgdb serial port */
>> +#endif
> 
> This is copy-and-paste cruft even on the non-QEMU targets.

Ok, I'll remove it.


Alex

[U-Boot] [PATCH 3/3] PPC 85xx: Add qemu-ppce500 machine

[Scott Wood]

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 at h
> >> +	ori	r2, r2, CONFIG_QEMU_DT_ADDR at 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).

> >> +		/*
> >> +		 * 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.

> 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.
 
> >> +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.
 
> >> +	/* 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().

> >> +#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.
 
> >> +/* 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?

-Scott

[U-Boot] [PATCH 3/3] PPC 85xx: Add qemu-ppce500 machine

[Alexander Graf]

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 at h
>>>> +	ori	r2, r2, CONFIG_QEMU_DT_ADDR at 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

[U-Boot] [PATCH 3/3] PPC 85xx: Add qemu-ppce500 machine

[Scott Wood]

On Fri, 2014-01-24 at 02:25 +0100, Alexander Graf wrote:
> 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 at h
> >>>> +	ori	r2, r2, CONFIG_QEMU_DT_ADDR at 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.

Right.

> And how do I get offsets into the gd structure from asm in the first place?

See lib/asm-offsets.c

> 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 :).

It only works because you are booting directly from RAM, and the
variable you're writing to isn't subject to relocation, and isn't in the
BSS (unlike most uninitialized/zero-initialized data).  It's not good
practice.  What if you later want to simulate NOR flash and boot from
that?

> >> 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?

I don't know what the "_r" is supposed to mean, but the underscore seems
to just be something that varies from board to board.  Our boards use
$fdtaddr and until now I didn't realize other boards do it differently.
 
> >>>> +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.

Incorrectly for U-Boot's own use?  Incorrectly for passing to Linux?

The latter case definitely matters.  The former case would affect the
boot delay, so it needs to be at least reasonably close.
 
> >>> 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.

What sort of messing would you need to do to cpu_init_early_f()?  What
part of the normal workflow breaks under QEMU?

> >>>> +/* 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.

You could stick an ifndef CONFIG_QEMU_E500 in there I guess, or define
it to be zero and make sure you don't call any of those functions that
depend on it.

-Scott

[U-Boot] [PATCH 3/3] PPC 85xx: Add qemu-ppce500 machine

[Alexander Graf]

On 24.01.2014, at 02:39, Scott Wood <scottwood@freescale.com> wrote:

> On Fri, 2014-01-24 at 02:25 +0100, Alexander Graf wrote:
>> 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 at h
>>>>>> +	ori	r2, r2, CONFIG_QEMU_DT_ADDR at 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.
> 
> Right.
> 
>> And how do I get offsets into the gd structure from asm in the first place?
> 
> See lib/asm-offsets.c
> 
>> 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 :).
> 
> It only works because you are booting directly from RAM, and the
> variable you're writing to isn't subject to relocation, and isn't in the
> BSS (unlike most uninitialized/zero-initialized data).  It's not good
> practice.  What if you later want to simulate NOR flash and boot from
> that?

Ok, I'll change it :)

Saving it from asm only doesn't work though because we end up accessing the fdt pointer inside of cpu_early_init_f() which clears the gd. I've made the fdt pointer a function argument now.

> 
>>>> 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?
> 
> I don't know what the "_r" is supposed to mean, but the underscore seems
> to just be something that varies from board to board.  Our boards use
> $fdtaddr and until now I didn't realize other boards do it differently.

Bah - I'll just set all of them.

> 
>>>>>> +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.
> 
> Incorrectly for U-Boot's own use?  Incorrectly for passing to Linux?
> 
> The latter case definitely matters.  The former case would affect the
> boot delay, so it needs to be at least reasonably close.

We don't modify the DT when passing it to Linux. That would mess up quite a number of things (SMP among others).

As for internal use, it's definitely close enough on everything I tested it on.

> 
>>>>> 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.
> 
> What sort of messing would you need to do to cpu_init_early_f()?  What
> part of the normal workflow breaks under QEMU?

#ifndef CONFIG_FSL_CORENET
#if (defined(CONFIG_SYS_RAMBOOT) || defined(CONFIG_SPL)) && \
        !defined(CONFIG_SYS_INIT_L2_ADDR)
phys_size_t initdram(int board_type)
{
#if defined(CONFIG_SPD_EEPROM) || defined(CONFIG_DDR_SPD)
        return fsl_ddr_sdram_size();
#else
        return (phys_size_t)CONFIG_SYS_SDRAM_SIZE * 1024 * 1024;
#endif
}
#else /* CONFIG_SYS_RAMBOOT */
phys_size_t initdram(int board_type)
{
[...]
        dram_size = setup_ddr_tlbs(dram_size / 0x100000);
[...]

So because we set CONFIG_SYS_RAMBOOT we never call setup_ddr_tlbs() but instead have to do it ourselves in fsl_ddr_sdram_size(). I'll move the TLB fixup there.

> 
>>>>>> +/* 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.
> 
> You could stick an ifndef CONFIG_QEMU_E500 in there I guess, or define
> it to be zero and make sure you don't call any of those functions that
> depend on it.

Hrm, let me try that.


Alex

[U-Boot] [PATCH 3/3] PPC 85xx: Add qemu-ppce500 machine

[Scott Wood]

On Fri, 2014-01-24 at 15:19 +0100, Alexander Graf wrote:
> On 24.01.2014, at 02:39, Scott Wood <scottwood@freescale.com> wrote:
> 
> > On Fri, 2014-01-24 at 02:25 +0100, Alexander Graf wrote:
> >> 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:
> >>>> 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?
> > 
> > I don't know what the "_r" is supposed to mean, but the underscore seems
> > to just be something that varies from board to board.  Our boards use
> > $fdtaddr and until now I didn't realize other boards do it differently.
> 
> Bah - I'll just set all of them.

Ugh, no.

It looks like these are documented in README (and our use of fdtaddr is
nonstandard, though the README does state that they're just
suggestions).  fdt_addr is supposed to be the address in flash, and
fdt_addr_r is supposed to be the address in RAM.
 
> > What sort of messing would you need to do to cpu_init_early_f()?  What
> > part of the normal workflow breaks under QEMU?
> 
> #ifndef CONFIG_FSL_CORENET
> #if (defined(CONFIG_SYS_RAMBOOT) || defined(CONFIG_SPL)) && \
>         !defined(CONFIG_SYS_INIT_L2_ADDR)
> phys_size_t initdram(int board_type)
> {
> #if defined(CONFIG_SPD_EEPROM) || defined(CONFIG_DDR_SPD)
>         return fsl_ddr_sdram_size();
> #else
>         return (phys_size_t)CONFIG_SYS_SDRAM_SIZE * 1024 * 1024;
> #endif
> }
> #else /* CONFIG_SYS_RAMBOOT */
> phys_size_t initdram(int board_type)
> {
> [...]
>         dram_size = setup_ddr_tlbs(dram_size / 0x100000);
> [...]
> 
> So because we set CONFIG_SYS_RAMBOOT we never call setup_ddr_tlbs() but
> instead have to do it ourselves in fsl_ddr_sdram_size(). I'll move the
> TLB fixup there.

That looks like a bug in RAMBOOT handling, not specific to the QEMU
target.

You shouldn't be calling fsl_ddr_sdram_size() at all -- there's no
emulated SPD EEPROM.

For that matter, I don't see much in cpu.c that we really want to use
for the QEMU target.

-Scott

[U-Boot] [PATCH 3/3] PPC 85xx: Add qemu-ppce500 machine

[York Sun]

On 01/24/2014 06:19 AM, Alexander Graf wrote:
> Hrm, let me try that.
> 

Looks you got plenty feedback from Scott. I am going to mark this set as "change
requested" so they will drop off from my to-do list. Please submit a v2 when
they are ready (all three patches together) with change log.

York