[PATCH] export htab value for kexec on non-lpar

[Arnd Bergmann]

On Dinsdag 23 August 2005 10:04, R Sharada wrote:

> 
> diff -puN ./arch/ppc64/kernel/setup.c~kexec-export-htab-value ./arch/ppc64/kernel/setup.c
> --- linux-2.6.13-rc6/./arch/ppc64/kernel/setup.c~kexec-export-htab-value	2005-08-23 18:27:14.000000000 +0530
> +++ linux-2.6.13-rc6-sharada/./arch/ppc64/kernel/setup.c	2005-08-23 18:34:11.000000000 +0530
> @@ -59,12 +59,19 @@
>  #include <asm/lmb.h>
>  #include <asm/iSeries/ItLpNaca.h>
>  
> +#include <linux/bootmem.h>
> +
>  #ifdef DEBUG
>  #define DBG(fmt...) udbg_printf(fmt)
>  #else
>  #define DBG(fmt...)
>  #endif
>  
> +#ifdef CONFIG_KEXEC
> +#define HASH_GROUP_SIZE 0x80    /* size of each hash group, asm/mmu.h */

This should not be inside of #ifdef. From the comment, I'd also expect
that this would better fit into asm/mmu.h ;-)

> +void export_htab_value(void);

The function should be static when it is only used in a single file.
Also, forward declarations of functions within .c files are often
a bad idea. Just put your own function more to the top to avoid this.

> +#endif
> +
>  /*
>   * Here are some early debugging facilities. You can enable one
>   * but your kernel will not boot on anything else if you do so
> @@ -1086,6 +1093,9 @@ void __init setup_arch(char **cmdline_p)
>  	}
>  
>  	paging_init();
> +#ifdef CONFIG_KEXEC
> +	export_htab_value();
> +#endif

Instead of doing the #ifdef here, you can simply have an #else
path at the definition that provides an empty inline function.
That is, if it makes sense to have this conditional in the
first place.

>  	ppc64_boot_msg(0x15, "Setup Done");
>  }
>  
> @@ -1358,3 +1368,39 @@ void cpu_die(void)
>  	if (ppc_md.cpu_die)
>  		ppc_md.cpu_die();
>  }
> +
> +#ifdef CONFIG_KEXEC
> +void export_htab_value()
> +{
> +	struct device_node *node;
> +	struct property *newprop1, *newprop2;
> +	unsigned long low, high;
> +
> +	if (systemcfg->platform != PLATFORM_PSERIES_LPAR) {
> +		node = of_find_node_by_path("/chosen");
> +		if (!node)
> +			return;
> +		newprop1 = alloc_bootmem(sizeof(struct property) + sizeof(unsigned long));
> +		if (newprop1 == NULL)
> +			return;
> +		newprop2 = alloc_bootmem(sizeof(struct property) + sizeof(unsigned long));
> +		if (newprop1 == NULL)
> +			return;

Your memory layout is a bit obscure here. I'd either do a single allocation
of a structure for this purpose or simply have the properties statically
allocated. A static allocation would actually decrease the memory
requirements because you would not need the code to initialize it.

> +		memset(newprop1, 0, sizeof(struct property));
> +		memset(newprop2, 0, sizeof(struct property));
> +		newprop1->name = "htab_address";
> +		newprop1->length = sizeof(unsigned long);
> +		newprop1->value = (unsigned char *)&newprop1[1];
> +		low = __pa(htab_address);
> +		memcpy(newprop1->value, &low, sizeof(low));

Just to an assignment of the variably instead of memcpy. It's
only a single integer.

> +		prom_add_property(node, newprop1);
> +		newprop2->name = "htab_size";
> +		newprop2->length = sizeof(unsigned long);
> +		newprop2->value = (unsigned char *)&newprop2[1];
> +		high = low + (htab_hash_mask + 1) * HASH_GROUP_SIZE;
> +		memcpy(newprop2->value, &high, sizeof(high));
> +		prom_add_property(node, newprop2);
> +		of_node_put(node);
> +	}

	Arnd <><