Re: [kvm-devel] [kvm-ppc-devel] [PATCH 5 of 7] Add dynamic device tree manipulation change uboot loader for PPC bamboo board model
On Tue, 2008-03-18 at 15:06 -0500, Jerone Young wrote: # HG changeset patch # User Jerone Young [EMAIL PROTECTED] # Date 1205870472 18000 # Branch merge # Node ID 3e87db599895937824b9bf3369eb67ea7f5a7595 # Parent ba2876c3e8916ba9c19b75c4464cbb8dc6858fbd 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 load_image() to load kernel image. Again, the load_uimage part (which you've misspelled here) should be a separate patch? 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 @@ -617,7 +617,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 device_tree.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/device_tree.c b/qemu/hw/device_tree.c new file mode 100644 --- /dev/null +++ b/qemu/hw/device_tree.c @@ -0,0 +1,181 @@ +/* + * 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 license 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 Again, 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 + */ +uint32_t read_proc_dt_prop_cell(char *path_in_device_tree) +{ + char *buf = NULL; + int i; + uint32_t num; + FILE *stream; + + i = snprintf(buf, 0, %s/%s, DT_PROC_INTERFACE_PATH, + path_in_device_tree); + + buf = (char *)malloc(i); + if (buf == NULL) + { + printf(%s: Unable to malloc string buffer buf\n, + __func__); + exit(1); + } Braces. + i = snprintf(buf, i+1, %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); + } Braces. + fread(num, sizeof(num), 1, stream); + fclose(stream); + + return num; +} + +/* FUNCTIONS FOR LOADING MANIPULATION OF DEVICE TREE IN GUEST */ + +#ifdef CONFIG_LIBFDT +/* support functions */ +static int get_offset_of_node(void *fdt, char *node_path) +{ + int node_offset; + node_offset = fdt_path_offset(fdt, node_path); + if (node_offset 0) { + printf(Unable to find node in device tree '%s'\n, + node_path); + exit(1); + } + return node_offset; +} + +/* public functions */ +void *load_device_tree(char *filename_path, unsigned long load_addr) +{ + int dt_file_size; + int dt_file_load_size; + int new_dt_size;int ret; Does this look right to you? ... 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 @@ -4,15 +4,16 @@ * Copyright 2007 IBM Corporation. * Authors: Jerone Young [EMAIL PROTECTED] * - * This work is licensed under the GNU GPL licence version 2 or later. + * This work is licensed under the GNU GPL license version 2 or later. * */ +#include config.h #include ppc440.h +#include qemu-kvm.h +#include device_tree.h -#define KERNEL_LOAD_ADDR 0x40 /* uboot loader puts kernel at 4MB */ - -#include qemu-kvm.h +#define BINARY_DEVICE_TREE_FILE bamboo.dtb /* PPC 440 refrence demo board Could you fix this typo while you're at it? @@ -26,14 +27,22 @@ void bamboo_init(ram_addr_t ram_size, in const char *initrd_filename, const char *cpu_model) { + char buf[1024]; You previously said you had removed 'buf' and replaced it with dynamic allocation, but I don't see that here. 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
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 5 of 7] Add dynamic device tree manipulation change uboot loader for PPC bamboo board model
On Tue, 2008-03-18 at 16:25 -0500, Hollis Blanchard wrote: On Tue, 2008-03-18 at 15:06 -0500, Jerone Young wrote: # HG changeset patch # User Jerone Young [EMAIL PROTECTED] # Date 1205870472 18000 # Branch merge # Node ID 3e87db599895937824b9bf3369eb67ea7f5a7595 # Parent ba2876c3e8916ba9c19b75c4464cbb8dc6858fbd 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 load_image() to load kernel image. Again, the load_uimage part (which you've misspelled here) should be a separate patch? 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 @@ -617,7 +617,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 device_tree.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/device_tree.c b/qemu/hw/device_tree.c new file mode 100644 --- /dev/null +++ b/qemu/hw/device_tree.c @@ -0,0 +1,181 @@ +/* + * 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 license 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 Again, 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 + */ +uint32_t read_proc_dt_prop_cell(char *path_in_device_tree) +{ + char *buf = NULL; + int i; + uint32_t num; + FILE *stream; + + i = snprintf(buf, 0, %s/%s, DT_PROC_INTERFACE_PATH, + path_in_device_tree); + + buf = (char *)malloc(i); + if (buf == NULL) + { + printf(%s: Unable to malloc string buffer buf\n, + __func__); + exit(1); + } Braces. What is the deal. They are braces. They are done diffrenent through outt the qemu code. This + i = snprintf(buf, i+1, %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); + } Braces. + fread(num, sizeof(num), 1, stream); + fclose(stream); + + return num; +} + +/* FUNCTIONS FOR LOADING MANIPULATION OF DEVICE TREE IN GUEST */ + +#ifdef CONFIG_LIBFDT +/* support functions */ +static int get_offset_of_node(void *fdt, char *node_path) +{ + int node_offset; + node_offset = fdt_path_offset(fdt, node_path); + if (node_offset 0) { + printf(Unable to find node in device tree '%s'\n, + node_path); + exit(1); + } + return node_offset; +} + +/* public functions */ +void *load_device_tree(char *filename_path, unsigned long load_addr) +{ + int dt_file_size; + int dt_file_load_size; + int new_dt_size;int ret; Does this look right to you? can clean that. ... 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 @@ -4,15 +4,16 @@ * Copyright 2007 IBM Corporation. * Authors: Jerone Young [EMAIL PROTECTED] * - * This work is licensed under the GNU GPL licence version 2 or later. + * This work is licensed under the GNU GPL license version 2 or later. * */ +#include config.h #include ppc440.h +#include qemu-kvm.h +#include device_tree.h -#define KERNEL_LOAD_ADDR 0x40 /* uboot loader puts kernel at 4MB */ - -#include qemu-kvm.h +#define BINARY_DEVICE_TREE_FILE bamboo.dtb /* PPC 440 refrence demo board Could you fix this typo while you're at it? @@ -26,14 +27,22 @@ void bamboo_init(ram_addr_t ram_size, in const char *initrd_filename, const char *cpu_model) { + char buf[1024]; You previously said you had removed 'buf' and replaced it
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 5 of 7] Add dynamic device tree manipulation change uboot loader for PPC bamboo board model
I can only assume that you will actually make the corrections that you didn't respond to in this mail. On Tue, 2008-03-18 at 16:35 -0500, Jerone Young wrote: On Tue, 2008-03-18 at 16:25 -0500, Hollis Blanchard wrote: +#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 + */ +uint32_t read_proc_dt_prop_cell(char *path_in_device_tree) +{ + char *buf = NULL; + int i; + uint32_t num; + FILE *stream; + + i = snprintf(buf, 0, %s/%s, DT_PROC_INTERFACE_PATH, + path_in_device_tree); + + buf = (char *)malloc(i); + if (buf == NULL) + { + printf(%s: Unable to malloc string buffer buf\n, + __func__); + exit(1); + } Braces. What is the deal. They are braces. They are done diffrenent through outt the qemu code. This I don't enjoy correcting whitespace, and in fact I hate it when that's all comments are about. If it weren't so egregious in this patch series, I probably would have let it slide. In general I don't really care as long as it's *consistent*. The tabs you use in this code clashes with the rest of qemu, and that makes it difficult to use the right editor settings. Despite that, I still didn't say anything, because at least it's consistent. In general, don't just make your code work; make it pretty too. @@ -26,14 +27,22 @@ void bamboo_init(ram_addr_t ram_size, in const char *initrd_filename, const char *cpu_model) { + char buf[1024]; You previously said you had removed 'buf' and replaced it with dynamic allocation, but I don't see that here. Removing of buf discussed was from hw/device_tree.c not this file. So you can fix it here too, right? -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 5 of 7] Add dynamic device tree manipulation change uboot loader for PPC bamboo board model
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 0x40 /* 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,
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 5 of 7] Add dynamic device tree manipulation change uboot loader for PPC bamboo board model
You've misspelled licence several times in this patch. 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 0x40 /* 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()) { - /*