Re: PCI resource problems caused by improper address rounding

[Andrew Morton]

On Mon, 17 Dec 2007 19:25:27 -0500 Chuck Ebbert <cebbert@xxxxxxxxxx> wrote:

> Looks like a commit that I can't find in git due to the arch merge
> has broken PCI address assignment. This patch by Richard Henderson
> against 2.6.23 fixes it for x86_64:
> 
> --- linux-2.6.23.x86_64/arch/x86_64/kernel/e820.c	2007-10-09 13:31:38.000000000 -0700
> +++ linux-2.6.23.x86_64-rth/arch/x86_64/kernel/e820.c	2007-12-15 12:37:44.000000000 -0800
> @@ -718,8 +718,8 @@ __init void e820_setup_gap(void)
>  	while ((gapsize >> 4) > round)
>  		round += round;
>  	/* Fun with two's complement */
> -	pci_mem_start = (gapstart + round) & -round;
> +	pci_mem_start = (gapstart + round - 1) & -round;
>  
>  	printk(KERN_INFO "Allocating PCI resources starting at %lx (gap: %lx:%lx)\n",
>  		pci_mem_start, gapstart, gapsize);
> 
> 
> Here is the original changeset, taken from the Mercurial repo. It was
> merged in 2.6.14:
> 
> # HG changeset patch
> # User Daniel Ritz <daniel.ritz@xxxxxx>
> # Date 1126304746 -700
> # Node ID 51367d6e0b839be0b425a8f67c29f625b670f126
> # Parent f4852c862b04efc9f8e2c7913191f5f7d140d895
> [PATCH] Update PCI IOMEM allocation start
> 
> This fixes the problem with "Averatec 6240 pcmcia_socket0: unable to
> apply power", which was due to the CardBus IOMEM register region being
> allocated at an address that was actually inside the RAM window that had
> been reserved for video frame-buffers in an UMA setup.
> 
> The BIOS _should_ have marked that region reserved in the e820 memory
> descriptor tables, but did not.
> 
> It is fixed by rounding up the default starting address of PCI memory
> allocations, so that we leave a bigger gap after the final known memory
> location.  The amount of rounding depends on how big the unused memory
> gap is that we can allocate IOMEM from.
> 
> Based on example code by Linus.
> 
> Acked-by: Greg KH <greg@xxxxxxxxx>
> Acked-by: Ivan Kokshaysky <ink@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Linus Torvalds <torvalds@xxxxxxxx>
> 
> committer: Linus Torvalds <torvalds@xxxxxxxxxxx> 1126304746 -0700
> 
> 
> --- a/arch/i386/kernel/setup.c	Fri Sep 09 22:28:40 2005 +0011
> +++ b/arch/i386/kernel/setup.c	Fri Sep 09 22:37:26 2005 +0011
> @@ -1300,7 +1300,7 @@ legacy_init_iomem_resources(struct resou
>   */
>  static void __init register_memory(void)
>  {
> -	unsigned long gapstart, gapsize;
> +	unsigned long gapstart, gapsize, round;
>  	unsigned long long last;
>  	int	      i;
>  
> @@ -1345,14 +1345,14 @@ static void __init register_memory(void)
>  	}
>  
>  	/*
> -	 * Start allocating dynamic PCI memory a bit into the gap,
> -	 * aligned up to the nearest megabyte.
> -	 *
> -	 * Question: should we try to pad it up a bit (do something
> -	 * like " + (gapsize >> 3)" in there too?). We now have the
> -	 * technology.
> +	 * See how much we want to round up: start off with
> +	 * rounding to the next 1MB area.
>  	 */
> -	pci_mem_start = (gapstart + 0xfffff) & ~0xfffff;
> +	round = 0x100000;
> +	while ((gapsize >> 4) > round)
> +		round += round;
> +	/* Fun with two's complement */
> +	pci_mem_start = (gapstart + round) & -round;
>  
>  	printk("Allocating PCI resources starting at %08lx (gap: %08lx:%08lx)\n",
>  		pci_mem_start, gapstart, gapsize);
> --- a/arch/x86_64/kernel/e820.c	Fri Sep 09 22:28:40 2005 +0011
> +++ b/arch/x86_64/kernel/e820.c	Fri Sep 09 22:37:26 2005 +0011
> @@ -567,7 +567,7 @@ unsigned long pci_mem_start = 0xaeedbabe
>   */
>  __init void e820_setup_gap(void)
>  {
> -	unsigned long gapstart, gapsize;
> +	unsigned long gapstart, gapsize, round;
>  	unsigned long last;
>  	int i;
>  	int found = 0;
> @@ -604,14 +604,14 @@ __init void e820_setup_gap(void)
>  	}
>  
>  	/*
> -	 * Start allocating dynamic PCI memory a bit into the gap,
> -	 * aligned up to the nearest megabyte.
> -	 *
> -	 * Question: should we try to pad it up a bit (do something
> -	 * like " + (gapsize >> 3)" in there too?). We now have the
> -	 * technology.
> +	 * See how much we want to round up: start off with
> +	 * rounding to the next 1MB area.
>  	 */
> -	pci_mem_start = (gapstart + 0xfffff) & ~0xfffff;
> +	round = 0x100000;
> +	while ((gapsize >> 4) > round)
> +		round += round;
> +	/* Fun with two's complement */
> +	pci_mem_start = (gapstart + round) & -round;
>  
>  	printk(KERN_INFO "Allocating PCI resources starting at %lx (gap: %lx:%lx)\n",
>  		pci_mem_start, gapstart, gapsize);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/