[Linux-ia64] [PATCH] arch/ia64/kernel/efi.c

[Linux-ia64] [PATCH] arch/ia64/kernel/efi.c

[John Marvin]

In doing some testing, I discovered the the mem= kernel command line option
doesn't behave as expected for zx1 based machines. The current implementation
assumes that memory starts at physical address zero and is physically
contiguous, i.e. the mem= value is used as a limit on the physical address,
rather than specifying the maximum physical memory to use.

I've enclosed a patch to fix the problem.  Since this is primarily a
debugging option, this isn't a critical fix.

John Marvin
jsm@fc.hp.com

--- arch/ia64/kernel/efi.c.old	Mon Oct 17 14:01:13 2044
+++ arch/ia64/kernel/efi.c	Mon Oct 17 14:01:31 2044
@@ -210,6 +210,7 @@ efi_memmap_walk (efi_freemem_callback_t 
 	void *efi_map_start, *efi_map_end, *p, *q;
 	efi_memory_desc_t *md, *check_md;
 	u64 efi_desc_size, start, end, granule_addr, first_non_wb_addr = 0;
+       unsigned long mem_found = 0;
 
 	efi_map_start = __va(ia64_boot_param->efi_memmap);
 	efi_map_end   = efi_map_start + ia64_boot_param->efi_memmap_size;
@@ -259,14 +260,16 @@ efi_memmap_walk (efi_freemem_callback_t 
 		trim_top(md, first_non_wb_addr);
 
 		if (is_available_memory(md)) {
-			if (md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT) > mem_limit) {
-				if (md->phys_addr > mem_limit)
+			if (mem_found + (md->num_pages << EFI_PAGE_SHIFT) > mem_limit) {
+				if (mem_found >= mem_limit)
 					continue;
-				md->num_pages = (mem_limit - md->phys_addr) >> EFI_PAGE_SHIFT;
+				md->num_pages = (mem_limit - mem_found) >> EFI_PAGE_SHIFT;
 			}
 
 			if (md->num_pages = 0)
 				continue;
+
+			mem_found += (md->num_pages << EFI_PAGE_SHIFT);
 
 			curr.start = PAGE_OFFSET + md->phys_addr;
 			curr.end   = curr.start + (md->num_pages << EFI_PAGE_SHIFT);

Re: [Linux-ia64] [PATCH] arch/ia64/kernel/efi.c

[Andreas Schwab]

John Marvin <jsm@udlkern.fc.hp.com> writes:

|> In doing some testing, I discovered the the mem= kernel command line option
|> doesn't behave as expected for zx1 based machines. The current implementation
|> assumes that memory starts at physical address zero and is physically
|> contiguous, i.e. the mem= value is used as a limit on the physical address,
|> rather than specifying the maximum physical memory to use.

IMHO the current behaviour is still useful.  For example to make sure you
have only memory below 4G you can say "mem=4G".

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Deutschherrnstr. 15-19, D-90429 Nürnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [Linux-ia64] [PATCH] arch/ia64/kernel/efi.c

[Matthew Wilcox]

On Fri, Jul 26, 2002 at 03:50:03PM +0200, Andreas Schwab wrote:
> IMHO the current behaviour is still useful.  For example to make sure you
> have only memory below 4G you can say "mem=4G".

maybe it's useful, but it's not what the option is specified to do:

        mem=nn[KMG]     [KNL,BOOT] force use of a specific amount of
                        memory; to be used when the kernel is not able
                        to see the whole system memory or for test.

-- 
Revolutions do not require corporate support.

Re: [Linux-ia64] [PATCH] arch/ia64/kernel/efi.c

[David Mosberger]

>>>>> On Fri, 26 Jul 2002 15:13:22 +0100, Matthew Wilcox <willy@debian.org> said:

  Matthew> On Fri, Jul 26, 2002 at 03:50:03PM +0200, Andreas Schwab
  Matthew> wrote:
  >> IMHO the current behaviour is still useful.  For example to make
  >> sure you have only memory below 4G you can say "mem=4G".

  Matthew> maybe it's useful, but it's not what the option is
  Matthew> specified to do:

  Matthew>         mem=nn[KMG] [KNL,BOOT] force use of a specific
  Matthew> amount of memory; to be used when the kernel is not able to
  Matthew> see the whole system memory or for test.

I definitely want the old behavior.  But we can rename the old
behavior to "memlimit" or something like that.  I'd be happy to apply
such a patch.

	--david

Re: [Linux-ia64] [PATCH] arch/ia64/kernel/efi.c

[Grant Grundler]

John Marvin wrote:
> I've enclosed a patch to fix the problem.  Since this is primarily a
> debugging option, this isn't a critical fix.

Performance folks use this to characterize system performance too.
They don't like to muck with HW any more than they have to.

We've used it in the past to work around platform issues - not just
to "debug" them. It becomes critical if we discover we need such
a workaround.

grant