Re: [kvm-ppc-devel] [PATCH 5 of 7] Add dynamic device tree manipulation & change uboot loader for PPC bamboo board model

[Hollis Blanchard <hollisb@us.ibm.com>]

On Tue, 2008-03-18 at 15:06 -0500, Jerone Young wrote:
> # HG changeset patch
> # User Jerone Young <jyoung5@us.ibm.com>
> # 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 <jyoung5@us.ibm.com>
> 
> 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 <jyoung5@us.ibm.com>
> + *
> + * 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 <jyoung5@us.ibm.com>
>   *
> - * 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 0x400000 /* 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 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;

Why is there an extra blank line here?

-- 
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/vse0120000070mrt/direct/01/