Patchwork clean up physmap, fix unaligned mapping problems

login
register
about
Submitter Carl-Daniel Hailfinger
Date 2014-03-03 23:55:49
Message ID <53151685.9090700@gmx.net>
Download mbox | patch
Permalink /patch/4113/
State Accepted
Commit r1765
Headers show

Comments

Carl-Daniel Hailfinger - 2014-03-03 23:55:49
Convert all physmaps in the internal DMI code to use aligned readonly maps.
Convert all physmaps in the cbtable code to use unaligned readonly maps.
Make physunmap() a generic architecture-independent wrapper.
Add physunmap_unaligned() to complement physmap*_unaligned().

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
Stefan Tauner - 2014-03-04 23:21:00
On Tue, 04 Mar 2014 00:55:49 +0100
Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote:

> Convert all physmaps in the internal DMI code to use aligned readonly maps.
> Convert all physmaps in the cbtable code to use unaligned readonly maps.
> Make physunmap() a generic architecture-independent wrapper.
> Add physunmap_unaligned() to complement physmap*_unaligned().
> 
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
Acked-by: Stefan Tauner <stefan.tauner@student.tuwien.ac.at>
Carl-Daniel Hailfinger - 2014-03-05 00:18:51
Am 05.03.2014 00:21 schrieb Stefan Tauner:
> On Tue, 04 Mar 2014 00:55:49 +0100
> Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote:
>
>> Convert all physmaps in the internal DMI code to use aligned readonly maps.
>> Convert all physmaps in the cbtable code to use unaligned readonly maps.
>> Make physunmap() a generic architecture-independent wrapper.
>> Add physunmap_unaligned() to complement physmap*_unaligned().
>>
>> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
> Acked-by: Stefan Tauner <stefan.tauner@student.tuwien.ac.at>

Thanks, committed in r1765.

Regards,
Carl-Daniel

Patch

Index: flashrom-physmap_ro_rw_aligned_unaligned_cleanup/dmi.c
===================================================================
--- flashrom-physmap_ro_rw_aligned_unaligned_cleanup/dmi.c	(Revision 1764)
+++ flashrom-physmap_ro_rw_aligned_unaligned_cleanup/dmi.c	(Arbeitskopie)
@@ -153,7 +153,7 @@ 
 {
 	int i = 0, j = 0;
 
-	uint8_t *dmi_table_mem = physmap_round("DMI Table", base, len);
+	uint8_t *dmi_table_mem = physmap_ro("DMI Table", base, len);
 	if (dmi_table_mem == NULL) {
 		msg_perr("Unable to access DMI Table\n");
 		return;
Index: flashrom-physmap_ro_rw_aligned_unaligned_cleanup/cbtable.c
===================================================================
--- flashrom-physmap_ro_rw_aligned_unaligned_cleanup/cbtable.c	(Revision 1764)
+++ flashrom-physmap_ro_rw_aligned_unaligned_cleanup/cbtable.c	(Arbeitskopie)
@@ -261,7 +261,7 @@ 
 #else
 	start = 0x0;
 #endif
-	table_area = physmap_ro("low megabyte", start, BYTES_TO_MAP - start);
+	table_area = physmap_ro_unaligned("low megabyte", start, BYTES_TO_MAP - start);
 	if (ERROR_PTR == table_area) {
 		msg_perr("Failed getting access to coreboot low tables.\n");
 		return -1;
@@ -276,8 +276,9 @@ 
 		if (forward->tag == LB_TAG_FORWARD) {
 			start = forward->forward;
 			start &= ~(getpagesize() - 1);
-			physunmap(table_area, BYTES_TO_MAP);
-			table_area = physmap_ro("high tables", start, BYTES_TO_MAP);
+			physunmap_unaligned(table_area, BYTES_TO_MAP);
+			// FIXME: table_area is never unmapped below, nor is it unmapped above in the no-forward case
+			table_area = physmap_ro_unaligned("high tables", start, BYTES_TO_MAP);
 			if (ERROR_PTR == table_area) {
 				msg_perr("Failed getting access to coreboot high tables.\n");
 				return -1;
Index: flashrom-physmap_ro_rw_aligned_unaligned_cleanup/physmap.c
===================================================================
--- flashrom-physmap_ro_rw_aligned_unaligned_cleanup/physmap.c	(Revision 1764)
+++ flashrom-physmap_ro_rw_aligned_unaligned_cleanup/physmap.c	(Arbeitskopie)
@@ -27,6 +27,7 @@ 
 #include <string.h>
 #include <errno.h>
 #include "flash.h"
+#include "programmer.h"
 #include "hwaccess.h"
 
 #if !defined(__DJGPP__) && !defined(__LIBPAYLOAD__)
@@ -89,7 +90,7 @@ 
 #define sys_physmap_rw_uncached	sys_physmap
 #define sys_physmap_ro_cached	sys_physmap
 
-void physunmap(void *virt_addr, size_t len)
+void sys_physunmap_unaligned(void *virt_addr, size_t len)
 {
 	__dpmi_meminfo mi;
 
@@ -118,7 +119,7 @@ 
 #define sys_physmap_rw_uncached	sys_physmap
 #define sys_physmap_ro_cached	sys_physmap
 
-void physunmap(void *virt_addr, size_t len)
+void sys_physunmap_unaligned(void *virt_addr, size_t len)
 {
 }
 #elif defined(__MACH__) && defined(__APPLE__)
@@ -139,7 +140,7 @@ 
 #define sys_physmap_rw_uncached	sys_physmap
 #define sys_physmap_ro_cached	sys_physmap
 
-void physunmap(void *virt_addr, size_t len)
+void sys_physunmap_unaligned(void *virt_addr, size_t len)
 {
 	unmap_physical(virt_addr, len);
 }
@@ -192,13 +193,8 @@ 
 	return MAP_FAILED == virt_addr ? ERROR_PTR : virt_addr;
 }
 
-void physunmap(void *virt_addr, size_t len)
+void sys_physunmap_unaligned(void *virt_addr, size_t len)
 {
-	if (len == 0) {
-		msg_pspew("Not unmapping zero size at %p\n", virt_addr);
-		return;
-	}
-
 	munmap(virt_addr, len);
 }
 #endif
@@ -242,7 +238,7 @@ 
 		return 1;
 	}
 	struct undo_physmap_data *d = data;
-	physunmap(d->virt_addr, d->len);
+	physunmap_unaligned(d->virt_addr, d->len);
 	free(data);
 	return 0;
 }
@@ -292,7 +288,7 @@ 
 		struct undo_physmap_data *d = malloc(sizeof(struct undo_physmap_data));
 		if (d == NULL) {
 			msg_perr("%s: Out of memory!\n", __func__);
-			physunmap(virt_addr, len);
+			physunmap_unaligned(virt_addr, len);
 			return ERROR_PTR;
 		}
 
@@ -300,7 +296,7 @@ 
 		d->len = len;
 		if (register_shutdown(undo_physmap, d) != 0) {
 			msg_perr("%s: Could not register shutdown function!\n", __func__);
-			physunmap(virt_addr, len);
+			physunmap_unaligned(virt_addr, len);
 			return ERROR_PTR;
 		}
 	}
@@ -308,9 +304,41 @@ 
 	return virt_addr + offset;
 }
 
+void physunmap_unaligned(void *virt_addr, size_t len)
+{
+	/* No need to check for zero size, such mappings would have yielded ERROR_PTR. */
+	if (virt_addr == ERROR_PTR) {
+		msg_perr("Trying to unmap a nonexisting mapping!\n"
+			 "Please report a bug at flashrom@flashrom.org\n");
+		return;
+	}
+
+	sys_physunmap_unaligned(virt_addr, len);
+}
+
+void physunmap(void *virt_addr, size_t len)
+{
+	uintptr_t tmp;
+
+	/* No need to check for zero size, such mappings would have yielded ERROR_PTR. */
+	if (virt_addr == ERROR_PTR) {
+		msg_perr("Trying to unmap a nonexisting mapping!\n"
+			 "Please report a bug at flashrom@flashrom.org\n");
+		return;
+	}
+	tmp = (uintptr_t)virt_addr;
+	/* We assume that the virtual address of a page-aligned physical address is page-aligned as well. By
+	 * extension, rounding a virtual unaligned address as returned by physmap should yield the same offset
+	 * between rounded and original virtual address as between rounded and original physical address.
+	 */
+	round_to_page_boundaries(&tmp, &len);
+	virt_addr = (void *)tmp;
+	physunmap_unaligned(virt_addr, len);
+}
+
 void *physmap(const char *descr, uintptr_t phys_addr, size_t len)
 {
-	return physmap_common(descr, phys_addr, len, PHYSM_RW, PHYSM_NOCLEANUP, PHYSM_EXACT);
+	return physmap_common(descr, phys_addr, len, PHYSM_RW, PHYSM_NOCLEANUP, PHYSM_ROUND);
 }
 
 void *rphysmap(const char *descr, uintptr_t phys_addr, size_t len)
@@ -318,12 +346,12 @@ 
 	return physmap_common(descr, phys_addr, len, PHYSM_RW, PHYSM_CLEANUP, PHYSM_ROUND);
 }
 
-void *physmap_round(const char *descr, uintptr_t phys_addr, size_t len)
+void *physmap_ro(const char *descr, uintptr_t phys_addr, size_t len)
 {
-	return physmap_common(descr, phys_addr, len, PHYSM_RW, PHYSM_NOCLEANUP, PHYSM_ROUND);
+	return physmap_common(descr, phys_addr, len, PHYSM_RO, PHYSM_NOCLEANUP, PHYSM_ROUND);
 }
 
-void *physmap_ro(const char *descr, uintptr_t phys_addr, size_t len)
+void *physmap_ro_unaligned(const char *descr, uintptr_t phys_addr, size_t len)
 {
 	return physmap_common(descr, phys_addr, len, PHYSM_RO, PHYSM_NOCLEANUP, PHYSM_EXACT);
 }
Index: flashrom-physmap_ro_rw_aligned_unaligned_cleanup/programmer.h
===================================================================
--- flashrom-physmap_ro_rw_aligned_unaligned_cleanup/programmer.h	(Revision 1764)
+++ flashrom-physmap_ro_rw_aligned_unaligned_cleanup/programmer.h	(Arbeitskopie)
@@ -277,9 +277,10 @@ 
 /* physmap.c */
 void *physmap(const char *descr, uintptr_t phys_addr, size_t len);
 void *rphysmap(const char *descr, uintptr_t phys_addr, size_t len);
-void *physmap_round(const char *descr, uintptr_t phys_addr, size_t len);
 void *physmap_ro(const char *descr, uintptr_t phys_addr, size_t len);
+void *physmap_ro_unaligned(const char *descr, uintptr_t phys_addr, size_t len);
 void physunmap(void *virt_addr, size_t len);
+void physunmap_unaligned(void *virt_addr, size_t len);
 #if CONFIG_INTERNAL == 1
 int setup_cpu_msr(int cpu);
 void cleanup_cpu_msr(void);