Skip to contentSkip to conversation

always print note when trimming EFI memory map

PATCH In review Bjorn Helgaas 2 participants 3 messages Sep 24, 2003 – Sep 26, 2003

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 always print note when trimming EFI memory map

I think it would be a good idea to always print a note when trimming the EFI memory map, even if it's not "available" memory.

I just debugged a problem where we trimmed out some memory that contained ACPI tables. When we went to look at a table later, we used the wrong attribute because we couldn't find its address in the EFI memory map. This caused an MCA (uncached access to memory). Straightforward to debug, but the note would have made it quicker.

This patch is against 2.5.

Bjorn

=== arch/ia64/kernel/efi.c 1.23 vs edited ==--- 1.23/arch/ia64/kernel/efi.c Wed Aug 27 10:47:27 2003
+++ edited/arch/ia64/kernel/efi.c Wed Sep 24 20:14:01 2003
@@ -249,11 +249,10 @@
if (num_skipped_pages > md->num_pages)
num_skipped_pages = md->num_pages;

- if (is_available_memory(md))
- printk(KERN_NOTICE "efi.%s: ignoring %luKB of memory at 0x%lx due to granule hole "

-		       "at 0x%lx\n", __FUNCTION__,
-		       (num_skipped_pages << EFI_PAGE_SHIFT) >> 10,
-		       md->phys_addr, start_addr - IA64_GRANULE_SIZE);
+	printk(KERN_NOTICE "efi.%s: ignoring %luKB of memory at 0x%lx due to granule hole "
+	       "at 0x%lx\n", __FUNCTION__,
+	       (num_skipped_pages << EFI_PAGE_SHIFT) >> 10,
+	       md->phys_addr, start_addr - IA64_GRANULE_SIZE);

/*
* NOTE: Don't set md->phys_addr to START_ADDR because that could cause the memory
* descriptor list to become unsorted. In such a case, md->num_pages will be
@@ -277,11 +276,10 @@
if (num_dropped_pages > md->num_pages)
num_dropped_pages = md->num_pages;

- if (is_available_memory(md))
- printk(KERN_NOTICE "efi.%s: ignoring %luKB of memory at 0x%lx due to granule hole "

-		       "at 0x%lx\n", __FUNCTION__,
-		       (num_dropped_pages << EFI_PAGE_SHIFT) >> 10,
-		       md->phys_addr, end_addr);
+	printk(KERN_NOTICE "efi.%s: ignoring %luKB of memory at 0x%lx due to granule hole "
+	       "at 0x%lx\n", __FUNCTION__,
+	       (num_dropped_pages << EFI_PAGE_SHIFT) >> 10,
+	       md->phys_addr, end_addr);

md->num_pages -= num_dropped_pages;
}

#1 lore raw
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line · Bjorn Helgaas

Bjorn> I think it would be a good idea to always print a note when Bjorn> trimming the EFI memory map, even if it's not "available" Bjorn> memory.

Bjorn> I just debugged a problem where we trimmed out some memory Bjorn> that contained ACPI tables. When we went to look at a table Bjorn> later, we used the wrong attribute because we couldn't find Bjorn> its address in the EFI memory map. This caused an MCA Bjorn> (uncached access to memory). Straightforward to debug, but Bjorn> the note would have made it quicker.

The intention was to give the user an idea how much real & available memory is lost due to the trimming. When reporting unavailable memory, you'll lose that info. However, maybe we should be more verbose if EFI_DEBUG is enabled?

	--david
#2 lore raw
In reply to Bjorn Helgaas
Quoted 4 lines · David Mosberger

The intention was to give the user an idea how much real & available memory is lost due to the trimming. When reporting unavailable memory, you'll lose that info. However, maybe we should be more verbose if EFI_DEBUG is enabled?

How about something like the following? It still always prints a note when trimming, but identifies "reserved" memory to distinguish it from the normal case.

This is the second or third time I've had to debug the problem, and the failure mode doesn't obviously point to EFI_DEBUG. It just MCAs because you try to access main memory with a UC access. In fact, I think on most boxes you don't even get an MCA; it just appears to work, even though it introduces attribute aliasing.

=== arch/ia64/kernel/efi.c 1.23 vs edited ==--- 1.23/arch/ia64/kernel/efi.c Wed Aug 27 10:47:27 2003
+++ edited/arch/ia64/kernel/efi.c Fri Sep 26 17:33:36 2003
@@ -249,11 +249,11 @@
if (num_skipped_pages > md->num_pages)
num_skipped_pages = md->num_pages;

- if (is_available_memory(md))
- printk(KERN_NOTICE "efi.%s: ignoring %luKB of memory at 0x%lx due to granule hole "

-		       "at 0x%lx\n", __FUNCTION__,
-		       (num_skipped_pages << EFI_PAGE_SHIFT) >> 10,
-		       md->phys_addr, start_addr - IA64_GRANULE_SIZE);
+	printk(KERN_NOTICE "efi.%s: ignoring %luKB of %smemory at 0x%lx due to granule hole "
+	       "at 0x%lx\n", __FUNCTION__,
+	       (num_skipped_pages << EFI_PAGE_SHIFT) >> 10,
+	       is_available_memory(md) ? "" : "reserved ",
+	       md->phys_addr, start_addr - IA64_GRANULE_SIZE);

/*
* NOTE: Don't set md->phys_addr to START_ADDR because that could cause the memory
* descriptor list to become unsorted. In such a case, md->num_pages will be
@@ -277,11 +277,11 @@
if (num_dropped_pages > md->num_pages)
num_dropped_pages = md->num_pages;

- if (is_available_memory(md))
- printk(KERN_NOTICE "efi.%s: ignoring %luKB of memory at 0x%lx due to granule hole "

-		       "at 0x%lx\n", __FUNCTION__,
-		       (num_dropped_pages << EFI_PAGE_SHIFT) >> 10,
-		       md->phys_addr, end_addr);
+	printk(KERN_NOTICE "efi.%s: ignoring %luKB of %smemory at 0x%lx due to granule hole "
+	       "at 0x%lx\n", __FUNCTION__,
+	       (num_dropped_pages << EFI_PAGE_SHIFT) >> 10,
+	       is_available_memory(md) ? "" : "reserved ",
+	       md->phys_addr, end_addr);

md->num_pages -= num_dropped_pages;
}