Re: [U-Boot] [PATCH 3/3] PPC 85xx: Add qemu-ppce500 machine
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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/3] PPC 85xx: Add qemu-ppce500 machine
On Fri, 2014-01-24 at 15:19 +0100, Alexander Graf wrote:
> On 24.01.2014, at 02:39, Scott Wood wrote:
>
> > On Fri, 2014-01-24 at 02:25 +0100, Alexander Graf wrote:
> >> On 24.01.2014, at 01:49, Scott Wood 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 / 0x10);
> [...]
>
> 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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/3] PPC 85xx: Add qemu-ppce500 machine
On 24.01.2014, at 02:39, Scott Wood wrote: > On Fri, 2014-01-24 at 02:25 +0100, Alexander Graf wrote: >> On 24.01.2014, at 01:49, Scott Wood wrote: >> >>> On Thu, 2014-01-23 at 14:08 +0100, Alexander Graf wrote: On 21.01.2014, at 03:25, Scott Wood 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 >> --- >> arch/powerpc/cpu/mpc85xx/start.S|7 + >> arch/powerpc/include/asm/config_mpc85xx.h |4 + >> board/freescale/qemu-ppce500/Makefile | 10 ++ >> board/freescale/qemu-ppce500/qemu-ppce500.c | 260 >> +++ >> board/freescale/qemu-ppce500/tlb.c | 59 ++ >> boards.cfg |1 + >> include/configs/qemu-ppce500.h | 235 >> >> 7 files changed, 576 insertions(+) >> create mode 100644 board/freescale/qemu-ppce500/Makefile >> create mode 100644 board/freescale/qemu-ppce500/qemu-ppce500.c >> create mode 100644 board/freescale/qemu-ppce500/tlb.c >> create mode 100644 include/configs/qemu-ppce500.h >> >> diff --git a/arch/powerpc/cpu/mpc85xx/start.S >> b/arch/powerpc/cpu/mpc85xx/start.S >> index db84d10..ccbcc03 100644 >> --- a/arch/powerpc/cpu/mpc85xx/start.S >> +++ b/arch/powerpc/cpu/mpc85xx/start.S >> @@ -80,6 +80,13 @@ _start_e500: >> li r1,MSR_DE >> mtmsr r1 >> >> +#ifdef CONFIG_QEMU_E500 >> +/* Save our ePAPR device tree off before we clobber it */ >> +lis r2, CONFIG_QEMU_DT_ADDR@h >> +ori r2, r2, CONFIG_QEMU_DT_ADDR@l >> +stw r3, 0(r2) >> +#endif > > r2 has a special purpose -- please don't use it for other things, even > if it's too early to be a problem and other code is similarly bad. Heh, ok. I'll use r4 instead. > Instead of saving the DT pointer in some random hardcoded address, how > about sticking it in a callee-saved register until you're able to store > it in the global data struct? I did that at first but that didn't survive relocation. However, I just remembered that I had my global variable in bss, so maybe relocation doesn't work properly there. Instead I put it in .data now and that seems to work. It's certainly the nicer solution, I agree. >>> >>> I don't mean a global variable, but a field in the global data struct >>> (gd_t). BSS should not be accessed, and global variables should not be >>> written, before relocation (although you may get away with the latter >>> pre-relocation in ramboot cases, you still shouldn't). >> >> But the global data gets cleared in cpu_init_early_f(). So I'd have to >> shove it to some non-volatile, make sure no other code uses that >> particular register and then move it into gd. > > 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.
Re: [U-Boot] [PATCH 3/3] PPC 85xx: Add qemu-ppce500 machine
On Fri, 2014-01-24 at 02:25 +0100, Alexander Graf wrote:
> On 24.01.2014, at 01:49, Scott Wood wrote:
>
> > On Thu, 2014-01-23 at 14:08 +0100, Alexander Graf wrote:
> >> On 21.01.2014, at 03:25, Scott Wood 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
> ---
> arch/powerpc/cpu/mpc85xx/start.S|7 +
> arch/powerpc/include/asm/config_mpc85xx.h |4 +
> board/freescale/qemu-ppce500/Makefile | 10 ++
> board/freescale/qemu-ppce500/qemu-ppce500.c | 260
> +++
> board/freescale/qemu-ppce500/tlb.c | 59 ++
> boards.cfg |1 +
> include/configs/qemu-ppce500.h | 235
>
> 7 files changed, 576 insertions(+)
> create mode 100644 board/freescale/qemu-ppce500/Makefile
> create mode 100644 board/freescale/qemu-ppce500/qemu-ppce500.c
> create mode 100644 board/freescale/qemu-ppce500/tlb.c
> create mode 100644 include/configs/qemu-ppce500.h
>
> diff --git a/arch/powerpc/cpu/mpc85xx/start.S
> b/arch/powerpc/cpu/mpc85xx/start.S
> index db84d10..ccbcc03 100644
> --- a/arch/powerpc/cpu/mpc85xx/start.S
> +++ b/arch/powerpc/cpu/mpc85xx/start.S
> @@ -80,6 +80,13 @@ _start_e500:
> li r1,MSR_DE
> mtmsr r1
>
> +#ifdef CONFIG_QEMU_E500
> +/* Save our ePAPR device tree off before we clobber it */
> +lis r2, CONFIG_QEMU_DT_ADDR@h
> +ori r2, r2, CONFIG_QEMU_DT_ADDR@l
> +stw r3, 0(r2)
> +#endif
> >>>
> >>> r2 has a special purpose -- please don't use it for other things, even
> >>> if it's too early to be a problem and other code is similarly bad.
> >>
> >> Heh, ok. I'll use r4 instead.
> >>
> >>> Instead of saving the DT pointer in some random hardcoded address, how
> >>> about sticking it in a callee-saved register until you're able to store
> >>> it in the global data struct?
> >>
> >> I did that at first but that didn't survive relocation. However, I just
> >> remembered that I had my global variable in bss, so maybe relocation
> >> doesn't work properly there. Instead I put it in .data now and that
> >> seems to work.
> >>
> >> It's certainly the nicer solution, I agree.
> >
> > I don't mean a global variable, but a field in the global data struct
> > (gd_t). BSS should not be accessed, and global variables should not be
> > written, before relocation (although you may get away with the latter
> > pre-relocation in ramboot cases, you still shouldn't).
>
> But the global data gets cleared in cpu_init_early_f(). So I'd have to
> shove it to some non-volatile, make sure no other code uses that
> particular register and then move it into gd.
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 ;
> +}
> >>>
> >>> s/doesn't matter/doesn't exist/
> >>>
> >>> Where is this used from? Can it be skipped for this target? Is the
Re: [U-Boot] [PATCH 3/3] PPC 85xx: Add qemu-ppce500 machine
On 24.01.2014, at 01:49, Scott Wood wrote:
> On Thu, 2014-01-23 at 14:08 +0100, Alexander Graf wrote:
>> On 21.01.2014, at 03:25, Scott Wood 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
---
arch/powerpc/cpu/mpc85xx/start.S|7 +
arch/powerpc/include/asm/config_mpc85xx.h |4 +
board/freescale/qemu-ppce500/Makefile | 10 ++
board/freescale/qemu-ppce500/qemu-ppce500.c | 260
+++
board/freescale/qemu-ppce500/tlb.c | 59 ++
boards.cfg |1 +
include/configs/qemu-ppce500.h | 235
7 files changed, 576 insertions(+)
create mode 100644 board/freescale/qemu-ppce500/Makefile
create mode 100644 board/freescale/qemu-ppce500/qemu-ppce500.c
create mode 100644 board/freescale/qemu-ppce500/tlb.c
create mode 100644 include/configs/qemu-ppce500.h
diff --git a/arch/powerpc/cpu/mpc85xx/start.S
b/arch/powerpc/cpu/mpc85xx/start.S
index db84d10..ccbcc03 100644
--- a/arch/powerpc/cpu/mpc85xx/start.S
+++ b/arch/powerpc/cpu/mpc85xx/start.S
@@ -80,6 +80,13 @@ _start_e500:
li r1,MSR_DE
mtmsr r1
+#ifdef CONFIG_QEMU_E500
+ /* Save our ePAPR device tree off before we clobber it */
+ lis r2, CONFIG_QEMU_DT_ADDR@h
+ ori r2, r2, CONFIG_QEMU_DT_ADDR@l
+ stw r3, 0(r2)
+#endif
>>>
>>> r2 has a special purpose -- please don't use it for other things, even
>>> if it's too early to be a problem and other code is similarly bad.
>>
>> Heh, ok. I'll use r4 instead.
>>
>>> Instead of saving the DT pointer in some random hardcoded address, how
>>> about sticking it in a callee-saved register until you're able to store
>>> it in the global data struct?
>>
>> I did that at first but that didn't survive relocation. However, I just
>> remembered that I had my global variable in bss, so maybe relocation
>> doesn't work properly there. Instead I put it in .data now and that
>> seems to work.
>>
>> It's certainly the nicer solution, I agree.
>
> I don't mean a global variable, but a field in the global data struct
> (gd_t). BSS should not be accessed, and global variables should not be
> written, before relocation (although you may get away with the latter
> pre-relocation in ramboot cases, you still shouldn't).
But the global data gets cleared in cpu_init_early_f(). So I'd have to shove it
to some non-volatile, make sure no other code uses that particular register and
then move it into gd. And how do I get offsets into the gd structure from asm
in the first place?
Or are you suggesting I move r3 into r2x, leave it there until after
cpu_init_early_f(), copy it back to r3 and then call a C helper that puts it
into gd? Sounds quite excessive for something that works quite well with a
global variable in .data :).
>
+ /*
+ * We already know where the initrd is inside the dtb, so no
+ * need to override it
+ */
+ setenv("ramdisk_addr", "-");
>>>
>>> Indentation.
>>>
>>> What if the user wants to specify the initrd from within U-Boot? This
>>> should be handled via the default environment, not by overriding the
>>> user's choice.
>>
>> I'm not sure I see the difference. We have the following default boot script:
>>
>> #define CONFIG_BOOTCOMMAND \
>> "test -n \"$kernel_addr\" && bootm $kernel_addr $ramdisk_addr
>> $fdt_addr\0"
>>
>> which is the only place where we actually use $ramdisk_addr. If a user
>> wants to specify the initrd within U-Boot he can do that easily because
>> he would specify his own boot command anyway, right?
>
> Oh. I was confusing it with the existing $ramdiskaddr. So I'm changing
> my objection to the fact that it's confusing. :-)
>
> Likewise $fdt_addr versus $fdtaddr.
Mind to explain what the difference is between $fdtaddr, $fdt_addr (pxe boot?)
and $fdt_addr_r? Which one should I set?
>
>> Maybe I should rename kernel_addr to qemu_kernel_addr and drop
>> ramdisk_addr completely in favor for a - in the bootm command? Yeah,
>> I'll do that :).
>
> OK.
>
+ ret = fdt_open_into((void*)dt_base, fdt, dt_size);
+ if (ret)
+ panic("Couldn't open fdt");
+
+ memory = fdt_path_offset(fdt, "/memory");
+ prop = fdt_getprop(fdt, memory, "reg", &len);
+
+ if (prop && len >= 16)
+ return *(ui
Re: [U-Boot] [PATCH 3/3] PPC 85xx: Add qemu-ppce500 machine
On Thu, 2014-01-23 at 14:08 +0100, Alexander Graf wrote:
> On 21.01.2014, at 03:25, Scott Wood 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
> >> ---
> >> arch/powerpc/cpu/mpc85xx/start.S|7 +
> >> arch/powerpc/include/asm/config_mpc85xx.h |4 +
> >> board/freescale/qemu-ppce500/Makefile | 10 ++
> >> board/freescale/qemu-ppce500/qemu-ppce500.c | 260
> >> +++
> >> board/freescale/qemu-ppce500/tlb.c | 59 ++
> >> boards.cfg |1 +
> >> include/configs/qemu-ppce500.h | 235
> >> 7 files changed, 576 insertions(+)
> >> create mode 100644 board/freescale/qemu-ppce500/Makefile
> >> create mode 100644 board/freescale/qemu-ppce500/qemu-ppce500.c
> >> create mode 100644 board/freescale/qemu-ppce500/tlb.c
> >> create mode 100644 include/configs/qemu-ppce500.h
> >>
> >> diff --git a/arch/powerpc/cpu/mpc85xx/start.S
> >> b/arch/powerpc/cpu/mpc85xx/start.S
> >> index db84d10..ccbcc03 100644
> >> --- a/arch/powerpc/cpu/mpc85xx/start.S
> >> +++ b/arch/powerpc/cpu/mpc85xx/start.S
> >> @@ -80,6 +80,13 @@ _start_e500:
> >>li r1,MSR_DE
> >>mtmsr r1
> >>
> >> +#ifdef CONFIG_QEMU_E500
> >> + /* Save our ePAPR device tree off before we clobber it */
> >> + lis r2, CONFIG_QEMU_DT_ADDR@h
> >> + ori r2, r2, CONFIG_QEMU_DT_ADDR@l
> >> + stw r3, 0(r2)
> >> +#endif
> >
> > r2 has a special purpose -- please don't use it for other things, even
> > if it's too early to be a problem and other code is similarly bad.
>
> Heh, ok. I'll use r4 instead.
>
> > Instead of saving the DT pointer in some random hardcoded address, how
> > about sticking it in a callee-saved register until you're able to store
> > it in the global data struct?
>
> I did that at first but that didn't survive relocation. However, I just
> remembered that I had my global variable in bss, so maybe relocation
> doesn't work properly there. Instead I put it in .data now and that
> seems to work.
>
> It's certainly the nicer solution, I agree.
I don't mean a global variable, but a field in the global data struct
(gd_t). BSS should not be accessed, and global variables should not be
written, before relocation (although you may get away with the latter
pre-relocation in ramboot cases, you still shouldn't).
> >> + /*
> >> + * 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 ;
> >> +}
> >
> > s/doesn't matter/doesn't exist/
> >
> > Where is this used from? Can it be skipped for th
Re: [U-Boot] [PATCH 3/3] PPC 85xx: Add qemu-ppce500 machine
On 21.01.2014, at 03:25, Scott Wood 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
>> ---
>> arch/powerpc/cpu/mpc85xx/start.S|7 +
>> arch/powerpc/include/asm/config_mpc85xx.h |4 +
>> board/freescale/qemu-ppce500/Makefile | 10 ++
>> board/freescale/qemu-ppce500/qemu-ppce500.c | 260
>> +++
>> board/freescale/qemu-ppce500/tlb.c | 59 ++
>> boards.cfg |1 +
>> include/configs/qemu-ppce500.h | 235
>> 7 files changed, 576 insertions(+)
>> create mode 100644 board/freescale/qemu-ppce500/Makefile
>> create mode 100644 board/freescale/qemu-ppce500/qemu-ppce500.c
>> create mode 100644 board/freescale/qemu-ppce500/tlb.c
>> create mode 100644 include/configs/qemu-ppce500.h
>>
>> diff --git a/arch/powerpc/cpu/mpc85xx/start.S
>> b/arch/powerpc/cpu/mpc85xx/start.S
>> index db84d10..ccbcc03 100644
>> --- a/arch/powerpc/cpu/mpc85xx/start.S
>> +++ b/arch/powerpc/cpu/mpc85xx/start.S
>> @@ -80,6 +80,13 @@ _start_e500:
>> li r1,MSR_DE
>> mtmsr r1
>>
>> +#ifdef CONFIG_QEMU_E500
>> +/* Save our ePAPR device tree off before we clobber it */
>> +lis r2, CONFIG_QEMU_DT_ADDR@h
>> +ori r2, r2, CONFIG_QEMU_DT_ADDR@l
>> +stw r3, 0(r2)
>> +#endif
>
> r2 has a special purpose -- please don't use it for other things, even
> if it's too early to be a problem and other code is similarly bad.
Heh, ok. I'll use r4 instead.
> Instead of saving the DT pointer in some random hardcoded address, how
> about sticking it in a callee-saved register until you're able to store
> it in the global data struct?
I did that at first but that didn't survive relocation. However, I just
remembered that I had my global variable in bss, so maybe relocation doesn't
work properly there. Instead I put it in .data now and that seems to work.
It's certainly the nicer solution, I agree.
>
>> 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 0xff70
>> #define CONFIG_SYS_FSL_ERRATUM_A005125
>>
>> +#elif defined(CONFIG_QEMU_E500)
>> +#define CONFIG_MAX_CPUS 1
>> +#define CONFIG_SYS_CCSRBAR_DEFAULT 0xe000
>
> 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 000..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
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>
> 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
Re: [U-Boot] [PATCH 3/3] PPC 85xx: Add qemu-ppce500 machine
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
> ---
> arch/powerpc/cpu/mpc85xx/start.S|7 +
> arch/powerpc/include/asm/config_mpc85xx.h |4 +
> board/freescale/qemu-ppce500/Makefile | 10 ++
> board/freescale/qemu-ppce500/qemu-ppce500.c | 260
> +++
> board/freescale/qemu-ppce500/tlb.c | 59 ++
> boards.cfg |1 +
> include/configs/qemu-ppce500.h | 235
> 7 files changed, 576 insertions(+)
> create mode 100644 board/freescale/qemu-ppce500/Makefile
> create mode 100644 board/freescale/qemu-ppce500/qemu-ppce500.c
> create mode 100644 board/freescale/qemu-ppce500/tlb.c
> create mode 100644 include/configs/qemu-ppce500.h
>
> diff --git a/arch/powerpc/cpu/mpc85xx/start.S
> b/arch/powerpc/cpu/mpc85xx/start.S
> index db84d10..ccbcc03 100644
> --- a/arch/powerpc/cpu/mpc85xx/start.S
> +++ b/arch/powerpc/cpu/mpc85xx/start.S
> @@ -80,6 +80,13 @@ _start_e500:
> li r1,MSR_DE
> mtmsr r1
>
> +#ifdef CONFIG_QEMU_E500
> + /* Save our ePAPR device tree off before we clobber it */
> + lis r2, CONFIG_QEMU_DT_ADDR@h
> + ori r2, r2, CONFIG_QEMU_DT_ADDR@l
> + stw r3, 0(r2)
> +#endif
r2 has a special purpose -- please don't use it for other things, even
if it's too early to be a problem and other code is similarly bad.
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 0xff70
> #define CONFIG_SYS_FSL_ERRATUM_A005125
>
> +#elif defined(CONFIG_QEMU_E500)
> +#define CONFIG_MAX_CPUS 1
> +#define CONFIG_SYS_CCSRBAR_DEFAULT 0xe000
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 000..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
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
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
[U-Boot] [PATCH 3/3] PPC 85xx: Add qemu-ppce500 machine
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
---
arch/powerpc/cpu/mpc85xx/start.S|7 +
arch/powerpc/include/asm/config_mpc85xx.h |4 +
board/freescale/qemu-ppce500/Makefile | 10 ++
board/freescale/qemu-ppce500/qemu-ppce500.c | 260 +++
board/freescale/qemu-ppce500/tlb.c | 59 ++
boards.cfg |1 +
include/configs/qemu-ppce500.h | 235
7 files changed, 576 insertions(+)
create mode 100644 board/freescale/qemu-ppce500/Makefile
create mode 100644 board/freescale/qemu-ppce500/qemu-ppce500.c
create mode 100644 board/freescale/qemu-ppce500/tlb.c
create mode 100644 include/configs/qemu-ppce500.h
diff --git a/arch/powerpc/cpu/mpc85xx/start.S b/arch/powerpc/cpu/mpc85xx/start.S
index db84d10..ccbcc03 100644
--- a/arch/powerpc/cpu/mpc85xx/start.S
+++ b/arch/powerpc/cpu/mpc85xx/start.S
@@ -80,6 +80,13 @@ _start_e500:
li r1,MSR_DE
mtmsr r1
+#ifdef CONFIG_QEMU_E500
+ /* Save our ePAPR device tree off before we clobber it */
+ lis r2, CONFIG_QEMU_DT_ADDR@h
+ ori r2, r2, CONFIG_QEMU_DT_ADDR@l
+ stw r3, 0(r2)
+#endif
+
#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 0xff70
#define CONFIG_SYS_FSL_ERRATUM_A005125
+#elif defined(CONFIG_QEMU_E500)
+#define CONFIG_MAX_CPUS1
+#define CONFIG_SYS_CCSRBAR_DEFAULT 0xe000
+
#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 000..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, w...@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 000..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
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+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 = 6000;
+ 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 == 3000) ? "33" :
+ (pci_speed == 6000) ? "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;
+ uint3
