On Wed, 2008-03-12 at 21:53 -0500, Hollis Blanchard wrote:
> You've misspelled "licence" several times in this patch.
Got them. More of one error propagating through cut & paste.
>
> On Tue, 2008-03-11 at 23:50 -0500, Jerone Young wrote:
> > # HG changeset patch
> > # User Jerone Young <[EMAIL PROTECTED]>
> > # Date 1205296680 18000
> > # Branch merge
> > # Node ID 50fddb23a4c19ec6f359a4dd39e98712eb6bcaeb
> > # Parent 9c15709640cd55bf6f782d6856423363312493bb
> > Add dynamic device tree manipulation & change uboot loader for PPC
> > bamboo board model
> >
> > This patch adds code to dynamically manipulate the device tree when
> > loaded into memory. This allows us to finally have the ability to
> > manipulate the kernel command line & initrd from the qemu command
> > line. This will also let us setup different settings for the board.
> >
> > This patch also now uses new uboot loader uboot_loader_l() to load
> > kernel image.
>
> So the load_uboot() part should be a separate patch, right?
>
> > Signed-off-by: Jerone Young <[EMAIL PROTECTED]>
> >
> > diff --git a/qemu/Makefile.target b/qemu/Makefile.target
> > --- a/qemu/Makefile.target
> > +++ b/qemu/Makefile.target
> > @@ -615,7 +615,7 @@ OBJS+= unin_pci.o ppc_chrp.o
> > OBJS+= unin_pci.o ppc_chrp.o
> > # PowerPC 4xx boards
> > OBJS+= pflash_cfi02.o ppc4xx_devs.o ppc405_uc.o ppc405_boards.o
> > -OBJS+= ppc440.o ppc440_bamboo.o
> > +OBJS+= ppc440.o ppc440_bamboo.o ppc_device_tree_support.o
> > endif
> > ifeq ($(TARGET_BASE_ARCH), mips)
> > OBJS+= mips_r4k.o mips_malta.o mips_pica61.o mips_mipssim.o
> > diff --git a/qemu/hw/ppc440_bamboo.c b/qemu/hw/ppc440_bamboo.c
> > --- a/qemu/hw/ppc440_bamboo.c
> > +++ b/qemu/hw/ppc440_bamboo.c
> > @@ -8,11 +8,12 @@
> > *
> > */
> >
> > +#include "config.h"
> > #include "ppc440.h"
> > +#include "qemu-kvm.h"
> > +#include "ppc_device_tree_support.h"
> >
> > -#define KERNEL_LOAD_ADDR 0x400000 /* uboot loader puts kernel at 4MB */
> > -
> > -#include "qemu-kvm.h"
> > +#define BINARY_DEVICE_TREE_FILE "bamboo.dtb"
> >
> > /* PPC 440 refrence demo board
> > *
> > @@ -26,14 +27,22 @@ void bamboo_init(ram_addr_t ram_size, in
> > const char *initrd_filename,
> > const char *cpu_model)
> > {
> > + char buf[1024];
> > target_phys_addr_t ram_bases[2], ram_sizes[2];
> > qemu_irq *pic;
> > CPUState *env;
> > - target_ulong ep;
> > + target_ulong ep=0;
> > + target_ulong la=0;
> > int is_linux=1; /* Will assume allways is Linux for now */
> > - long kernel_size=0;
> > + target_long kernel_size=0;
> > target_ulong initrd_base=0;
> > - target_ulong initrd_size=0;
> > + target_long initrd_size=0;
> > + target_ulong dt_base=0;
> > + void *fdt;
> > + int ret;
> > +
> > + uint32_t cpu_freq;
> > + uint32_t timebase_freq;
> >
> > printf("%s: START\n", __func__);
> >
> > @@ -78,18 +87,23 @@ void bamboo_init(ram_addr_t ram_size, in
> >
> > /* load kernel with uboot loader */
> > printf("%s: load kernel\n", __func__);
> > - kernel_size = load_uboot(kernel_filename, &ep, &is_linux);
> > + load_uboot_l(kernel_filename, &ep, &la, &kernel_size, &is_linux);
> > if (kernel_size < 0) {
> > fprintf(stderr, "qemu: could not load kernel '%s'\n",
> > kernel_filename);
> > exit(1);
> > }
> > + printf("kernel is at guest address: 0x%lx\n", (unsigned long)la);
> >
> > /* load initrd */
> > if (initrd_filename) {
> > - initrd_base = kernel_size + KERNEL_LOAD_ADDR;
> > + initrd_base = kernel_size + la;
> > + printf("%s: load initrd\n", __func__);
> > initrd_size = load_image(initrd_filename,
> > phys_ram_base + initrd_base);
> > +
> > + printf("initrd is at guest address: 0x%lx\n",
> > + (unsigned long) initrd_base);
> >
> > if (initrd_size < 0) {
> > fprintf(stderr,
> > @@ -99,17 +113,58 @@ void bamboo_init(ram_addr_t ram_size, in
> > }
> > }
> >
> > +#ifdef CONFIG_LIBFDT
> > + /* get variable for device tree */
> > + cpu_freq = get_proc_dt_prop_cpu_clock_freq();
> > + timebase_freq = get_proc_dt_prop_cpu_timebase_freq();
> > +
> > + /* load binary device tree into qemu (not guest memory) */
> > + printf("%s: load device tree file\n", __func__);
> > +
> > + snprintf(buf, sizeof(buf), "%s/%s", bios_dir,
> > + BINARY_DEVICE_TREE_FILE);
> > +
> > + /* set base for device tree that will be in guest memory */
> > + if (initrd_base)
> > + dt_base = initrd_base + initrd_size;
> > + else
> > + dt_base = kernel_size + la;
> > +
> > + fdt = load_device_tree(buf, (phys_ram_base + dt_base));
> > + if (fdt == NULL) {
> > + printf("Loading device tree failed!\n");
> > + exit(1);
> > + }
> > +
> > + printf("device tree address is at guest address: 0x%lx\n",
> > + (unsigned long) dt_base);
> > +
> > + /* manipulate device tree in memory */
> > + set_dt_cpu_0_clock_freq_prop(fdt, cpu_freq);
> > + set_dt_cpu_0_timebase_prop(fdt, timebase_freq);
> > +
> > + set_dt_initrd_start_prop(fdt, initrd_base);
> > + set_dt_initrd_end_prop(fdt, (initrd_base + initrd_size));
> > +
> > + set_dt_bootargs_prop(fdt, kernel_cmdline);
> > +#endif
> > +
> > if (kvm_enabled()) {
> > - /* XXX insert TLB entries */
> > - env->gpr[1] = (16<<20) - 8;
> > - env->gpr[4] = initrd_base;
> > - env->gpr[5] = initrd_size;
> >
> > - env->nip = ep;
> > + /* XXX insert TLB entries */
> > + env->gpr[1] = (16<<20) - 8;
>
> Please fix your whitespace.
This actually is the white space fix. Moves from
>
> > - env->cpu_index = 0;
> > - printf("%s: loading kvm registers\n", __func__);
> > - kvm_load_registers(env);
> > +#ifdef CONFIG_LIBFDT
> > + /* location of device tree in register */
> > + env->gpr[3] = dt_base;
> > +#else
> > + env->gpr[4] = initrd_base;
> > + env->gpr[5] = initrd_size;
> > +#endif
>
> Linux ignores R4 and R5 regardless of whether qemu has libfdt support or
> not. Those should be removed.
Oh I forgot.
>
> > + env->nip = ep;
> > +
> > + printf("%s: loading kvm registers\n", __func__);
> > + kvm_load_registers(env);
> > }
> >
> > printf("%s: DONE\n", __func__);
> > diff --git a/qemu/hw/ppc_device_tree_support.c
> > b/qemu/hw/ppc_device_tree_support.c
> > new file mode 100644
> > --- /dev/null
> > +++ b/qemu/hw/ppc_device_tree_support.c
>
> This is a really long file name.
Just renamed them something much shorter.
>
> > @@ -0,0 +1,223 @@
> > +/*
> > + * Functions to help device tree manipulation using libfdt.
> > + * It also provides functions to read entries from device tree proc
> > + * interface.
> > + *
> > + * Copyright 2008 IBM Corporation.
> > + * Authors: Jerone Young <[EMAIL PROTECTED]>
> > + *
> > + * This work is licensed under the GNU GPL licence version 2 or later.
> > + *
> > + */
> > +
> > +#include <stdio.h>
> > +#include <sys/types.h>
> > +#include <sys/stat.h>
> > +#include <fcntl.h>
> > +#include <unistd.h>
> > +#include <stdlib.h>
> > +
> > +#include "config.h"
> > +#include "ppc440.h"
> > +
> > +#ifdef CONFIG_LIBFDT
> > + #include "libfdt.h"
> > +#endif
>
> Don't indent this.
>
> > +#define DT_PROC_INTERFACE_PATH "/proc/device-tree"
> > +
> > +/* FUNCTIONS FOR READING FROM DEVICE TREE OF HOST IN /PROC */
> > +
> > +/* This function reads device-tree property files that are of
> > + * a single cell size
> > + */
> > +static uint32_t read_proc_device_tree_prop_cell(char *path_in_device_tree)
> > +{
> > + char buf[1024];
> > + uint32_t num;
> > + FILE *stream;
> > +
> > + snprintf(buf, sizeof(buf), "%s/%s", DT_PROC_INTERFACE_PATH,
> > + path_in_device_tree);
> > +
> > + stream = fopen(buf, "rb");
> > +
> > + if (stream == NULL)
> > + {
> > + printf("%s: Unable to open '%s'\n", __func__, buf);
> > + exit(1);
> > + }
> > +
> > + fread(&num, sizeof(num), 1, stream);
> > + fclose(stream);
> > +
> > + return num;
> > +}
>
> I hate that "buf" variable. You can use snprintf() with length 0 to find
> out how much memory to malloc.
done
>
> ...
> > +void set_dt_cpu_0_timebase_prop(void *fdt, uint32_t timebase)
> > +{
> > + int offset;
> > + offset = get_offset_of_node(fdt, "/cpus/[EMAIL PROTECTED]");
> > + set_dt_prop_cell(fdt, offset, "timebase-frequency",
> > + timebase);
> > +
> > +}
> > +
> > +void set_dt_initrd_start_prop(void *fdt, uint32_t start_addr)
> > +{
> > + int offset;
> > + offset = get_offset_of_node(fdt, "/chosen");
> > + set_dt_prop_cell(fdt, offset, "linux,initrd-start",
> > + start_addr);
> > +}
> > +
> > +void set_dt_initrd_end_prop(void *fdt, uint32_t end_addr)
> > +{
> > + int offset;
> > + offset = get_offset_of_node(fdt, "/chosen");
> > + set_dt_prop_cell(fdt, offset, "linux,initrd-end",
> > + end_addr);
> > +}
> > +
> > +void set_dt_bootargs_prop(void *fdt, char *cmdline)
> > +{
> > + int offset;
> > + offset = get_offset_of_node(fdt, "/chosen");
> > + set_dt_prop_string(fdt,offset, "bootargs", cmdline);
> > +}
> > +
> > +#endif
>
> These are also really long function names. If you're basically going to
> encode the full device tree path into the function name, I don't think
> they're very convenient and they're not worth having. You could just
> pass the path in from the caller.
I've reworked the codd to remove all this.
>
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
