Re: [git pull] "big box" x86 changes, boot protocol

[Andrew Morton <akpm@linux-foundation.org>]

On Sat, 26 Apr 2008 21:54:07 +0200 Ingo Molnar <mingo@elte.hu> wrote:

> 
> Linus, please pull the following x86 changes from:
> 
>    git://git.kernel.org/pub/scm/linux/kernel/git/x86/linux-2.6-x86-bigbox-bootparam.git for-linus
> 
> these are boot parameter extensions for really large SGI UV boxes. The 
> change was seen and acked by the boot protocol guys. (well, Peter that 
> is ;-)
> 
> ...
>
> +void __init free_early(unsigned long start, unsigned long end)
> +{
> +	struct early_res *r;
> +	int i, j;
> +
> +	for (i = 0; i < MAX_EARLY_RES && early_res[i].end; i++) {
> +		r = &early_res[i];
> +		if (start == r->start && end == r->end)
> +			break;
> +	}
> +	if (i >= MAX_EARLY_RES || !early_res[i].end)
> +		panic("free_early on not reserved area: %lx-%lx!", start, end);
> +
> +	for (j = i + 1; j < MAX_EARLY_RES && early_res[j].end; j++)
> +		;
> +
> +	memcpy(&early_res[i], &early_res[i + 1],
> +	       (j - 1 - i) * sizeof(struct early_res));

nit: memcpy() shouldn't be used for overlapping copies.  It happens to be
OK (for dst<src) in the kernel implementations.  We hope.

> +	early_res[j - 1].end = 0;
> +}
> +

> +static ssize_t
> +setup_data_read(struct file *file, char __user *user_buf, size_t count,
> +		loff_t *ppos)
> +{
> +	struct setup_data_node *node = file->private_data;
> +	unsigned long remain;
> +	loff_t pos = *ppos;
> +	struct page *pg;
> +	void *p;
> +	u64 pa;
> +
> +	if (pos < 0)
> +		return -EINVAL;
> +	if (pos >= node->len)
> +		return 0;
> +
> +	if (count > node->len - pos)
> +		count = node->len - pos;
> +	pa = node->paddr + sizeof(struct setup_data) + pos;
> +	pg = pfn_to_page((pa + count - 1) >> PAGE_SHIFT);
> +	if (PageHighMem(pg)) {
> +		p = ioremap_cache(pa, count);
> +		if (!p)
> +			return -ENXIO;
> +	} else {
> +		p = __va(pa);
> +	}
> +
> +	remain = copy_to_user(user_buf, p, count);
> +
> +	if (PageHighMem(pg))
> +		iounmap(p);
> +
> +	if (remain)
> +		return -EFAULT;
> +
> +	*ppos = pos + count;
> +
> +	return count;
> +}

nit2: a read() function should return the number of bytes copied, and
should advance the file pointer by that much.  This code fails to do this
when a partial copy_to_user() occurs.

But we've made that mistake in many places and it doesn't appear to matter.