Skip to contentSkip to conversation

- romsignature-checksum-cleanup-2.patch removed from -mm tree

PATCH New [email protected] 3 participants 6 messages Jan 10, 2007 – Jan 29, 2007

lore
Share & tools
lore

[email protected]

author tree notice
#0 lore raw

The patch titled
romsignature/checksum cleanup
has been removed from the -mm tree. Its filename was
romsignature-checksum-cleanup-2.patch

This patch was dropped because x86_64 tree changes trashed it

------------------------------------------------------
Subject: romsignature/checksum cleanup
From: Rene Herman <[email protected]>

Remove the assumption that if the first page of an ISA ROM is mapped, it'll all be mapped. This'll also stop people reading this code from wondering if they're looking at a bug.

Not-strongly-objected-to-by Jeremy Fitzhardinge <[email protected]>
Cc Zachary Amsden <[email protected]>
Cc Rusty Russell <[email protected]>
Cc Andi Kleen <[email protected]>

---

 arch/i386/kernel/e820.c |   36 ++++++++++++++++++++++--------------
 1 files changed, 22 insertions(+), 14 deletions(-)
arch/i386/kernel/e820.c renamed from arch/i386/kernel/e820.c~romsignature-checksum-cleanup-2 +22 -14
diff -puN arch/i386/kernel/e820.c~romsignature-checksum-cleanup-2 arch/i386/kernel/e820.c
@@ -157,29 +157,31 @@ static struct resource standard_io_resou
 	.flags	= IORESOURCE_BUSY | IORESOURCE_IO
 } };
 
-static int romsignature(const unsigned char *x)
+#define ROMSIGNATURE 0xaa55
+
+static int __init romsignature(const unsigned char *rom)
 {
+	const unsigned short * const ptr = (const unsigned short *)rom;
 	unsigned short sig;
-	int ret = 0;
-	if (probe_kernel_address((const unsigned short *)x, sig) == 0)
-		ret = (sig == 0xaa55);
-	return ret;
+
+	return probe_kernel_address(ptr, sig) == 0 && sig == ROMSIGNATURE;
 }
 
-static int __init romchecksum(unsigned char *rom, unsigned long length)
+static int __init romchecksum(const unsigned char *rom, unsigned long length)
 {
-	unsigned char *p, sum = 0;
+	unsigned char sum, c;
 
-	for (p = rom; p < rom + length; p++)
-		sum += *p;
-	return sum == 0;
+	for (sum = 0; length && probe_kernel_address(rom++, c) == 0; length--)
+		sum += c;
+	return !length && !sum;
 }
 
 static void __init probe_roms(void)
 {
+	const unsigned char *rom;
 	unsigned long start, length, upper;
-	unsigned char *rom;
-	int	      i;
+	unsigned char c;
+	int i;
 
 	/* video rom */
 	upper = adapter_rom_resources[0].start;
@@ -190,8 +192,11 @@ static void __init probe_roms(void)
 
 		video_rom_resource.start = start;
 
+		if (probe_kernel_address(rom + 2, c) != 0)
+			continue;
+
 		/* 0 < length <= 0x7f * 512, historically */
-		length = rom[2] * 512;
+		length = c * 512;
 
 		/* if checksum okay, trust length byte */
 		if (length && romchecksum(rom, length))
@@ -225,8 +230,11 @@ static void __init probe_roms(void)
 		if (!romsignature(rom))
 			continue;
 
+		if (probe_kernel_address(rom + 2, c) != 0)
+			continue;
+
 		/* 0 < length <= 0x7f * 512, historically */
-		length = rom[2] * 512;
+		length = c * 512;
 
 		/* but accept any length that fits if checksum okay */
 		if (!length || start + length > upper || !romchecksum(rom, length))

_

Patches currently in -mm which might be from [email protected] are

romsignature-checksum-cleanup-2.patch

#1 lore raw
Quoted 6 lines · On 01/10/2007 11:44 PM, [email protected] wrote:

The patch titled
romsignature/checksum cleanup
has been removed from the -mm tree. Its filename was
romsignature-checksum-cleanup-2.patch

This patch was dropped because x86_64 tree changes trashed it

I was (am) quite unsure why this was, given that the patch did not touch x86_64 at all and moreover continued to apply cleanly to mm...

Did you just mean you also wanted the x86_64 version of it? In the next two messages I'll resubmit the i386 version, and submit the x86_64 version.

i386 has been compiled and tested, x86_64 has been crosscompiled only, due to lack of hardware. It's completely the same as the i386 version though...

Rene.

#2 lore raw
Quoted 11 lines · Rene Herman
Quoted 6 lines · On 01/10/2007 11:44 PM, [email protected] wrote:

The patch titled
romsignature/checksum cleanup
has been removed from the -mm tree. Its filename was
romsignature-checksum-cleanup-2.patch

This patch was dropped because x86_64 tree changes trashed it

I was (am) quite unsure why this was, given that the patch did not touch x86_64 at all and moreover continued to apply cleanly to mm...

The x86_64 tree is really a x86 tree these days and contains most i386 changes.

But in general if you change i386 then changing x86_64 makes sense too.

-Andi

#3 lore raw

PATCH 1/1 i386: probe_roms() cleanup

In reply to [email protected]

Hi Andrew.

Resubmit. I once heard you say you wanted patches not against -mm but against mainline so this replaces "romsignature-checksum-cleanup.patch" in current -mm.

===
Remove the assumption that if the first page of a legacy ROM is mapped, it'll all be mapped. This'll also stop people reading this code from wondering if they're looking at a bug...

===

Rene.

arch/i386/kernel/e820.c +22 -14
index f391abc..8b8741f 100644
@@ -156,29 +156,31 @@ static struct resource standard_io_resou
 	.flags	= IORESOURCE_BUSY | IORESOURCE_IO
 } };
 
-static int romsignature(const unsigned char *x)
+#define ROMSIGNATURE 0xaa55
+
+static int __init romsignature(const unsigned char *rom)
 {
+	const unsigned short * const ptr = (const unsigned short *)rom;
 	unsigned short sig;
-	int ret = 0;
-	if (probe_kernel_address((const unsigned short *)x, sig) == 0)
-		ret = (sig == 0xaa55);
-	return ret;
+	
+	return probe_kernel_address(ptr, sig) == 0 && sig == ROMSIGNATURE;
 }
 
-static int __init romchecksum(unsigned char *rom, unsigned long length)
+static int __init romchecksum(const unsigned char *rom, unsigned long length)
 {
-	unsigned char *p, sum = 0;
+	unsigned char sum, c;
 
-	for (p = rom; p < rom + length; p++)
-		sum += *p;
-	return sum == 0;
+	for (sum = 0; length && probe_kernel_address(rom++, c) == 0; length--)
+		sum += c;
+	return !length && !sum;
 }
 
 static void __init probe_roms(void)
 {
+	const unsigned char *rom;
 	unsigned long start, length, upper;
-	unsigned char *rom;
-	int	      i;
+	unsigned char c;
+	int i;
 
 	/* video rom */
 	upper = adapter_rom_resources[0].start;
@@ -189,8 +191,11 @@ static void __init probe_roms(void)
 
 		video_rom_resource.start = start;
 
+		if (probe_kernel_address(rom + 2, c) != 0)
+			continue;
+
 		/* 0 < length <= 0x7f * 512, historically */
-		length = rom[2] * 512;
+		length = c * 512;
 
 		/* if checksum okay, trust length byte */
 		if (length && romchecksum(rom, length))
@@ -224,8 +229,11 @@ static void __init probe_roms(void)
 		if (!romsignature(rom))
 			continue;
 
+		if (probe_kernel_address(rom + 2, c) != 0)
+			continue;
+
 		/* 0 < length <= 0x7f * 512, historically */
-		length = rom[2] * 512;
+		length = c * 512;
 
 		/* but accept any length that fits if checksum okay */
 		if (!length || start + length > upper || !romchecksum(rom, length))
#4 lore raw

PATCH 1/1 x86_64: sync up probe_roms() with i386

In reply to [email protected]

Hi Andrew.

This syncs up the x86_64 probe_roms() with the i386 version as just submitted.

===
Sync up with i386. Specifically, be careful about touching the legacy ROMs; in virtualized environments they may not be mapped.

Crosscompiled, but not booted due to lack of hardware.

===

Rene

arch/x86_64/kernel/setup.c +25 -10
index af425a8..0b3d237 100644
@@ -183,22 +183,31 @@ static struct resource video_ram_resourc
 	.flags = IORESOURCE_RAM,
 };
 
-#define romsignature(x) (*(unsigned short *)(x) == 0xaa55)
+#define ROMSIGNATURE 0xaa55
 
-static int __init romchecksum(unsigned char *rom, unsigned long length)
+static int __init romsignature(const unsigned char *rom)
 {
-	unsigned char *p, sum = 0;
+	const unsigned short * const ptr = (const unsigned short *)rom;
+	unsigned short sig;
 
-	for (p = rom; p < rom + length; p++)
-		sum += *p;
-	return sum == 0;
+	return probe_kernel_address(ptr, sig) == 0 && sig == ROMSIGNATURE;
+}
+
+static int __init romchecksum(const unsigned char *rom, unsigned long length)
+{
+	unsigned char sum, c;
+
+	for (sum = 0; length && probe_kernel_address(rom++, c) == 0; length--)
+		sum += c;
+	return !length && !sum;
 }
 
 static void __init probe_roms(void)
 {
+	const unsigned char *rom;
 	unsigned long start, length, upper;
-	unsigned char *rom;
-	int	      i;
+	unsigned char c;
+	int i;
 
 	/* video rom */
 	upper = adapter_rom_resources[0].start;
@@ -209,8 +218,11 @@ static void __init probe_roms(void)
 
 		video_rom_resource.start = start;
 
+		if (probe_kernel_address(rom, c) != 0)
+			continue;
+
 		/* 0 < length <= 0x7f * 512, historically */
-		length = rom[2] * 512;
+		length = c * 512;
 
 		/* if checksum okay, trust length byte */
 		if (length && romchecksum(rom, length))
@@ -245,8 +257,11 @@ static void __init probe_roms(void)
 		if (!romsignature(rom))
 			continue;
 
+		if (probe_kernel_address(rom, c) != 0)
+			continue;
+
 		/* 0 < length <= 0x7f * 512, historically */
-		length = rom[2] * 512;
+		length = c * 512;
 
 		/* but accept any length that fits if checksum okay */
 		if (!length || start + length > upper || !romchecksum(rom, length))
#5 lore raw

Re: [PATCH] x86_64: sync up probe_roms() with i386

Quoted 10 lines · Rene Herman

This syncs up the x86_64 probe_roms() with the i386 version as just submitted.

===
Sync up with i386. Specifically, be careful about touching the legacy ROMs; in virtualized environments they may not be mapped.

Crosscompiled, but not booted due to lack of hardware.

===

In the meantime tested by Martin Murray on x86_64 native and inside vmware (thanks much!) who told me I could add his sob:

Rene.