diff mbox

[7/9] sparc: use piggyback_32 for sparc64

Message ID 1293915632-4596-7-git-send-email-sam@ravnborg.org
State Superseded
Delegated to: David Miller
Headers show

Commit Message

Sam Ravnborg Jan. 1, 2011, 9 p.m. UTC
piggyback_32 adapted to support sparc64:
- locating "HdrS" differs for sparc and sparc64
- sparc64 updates a_text, a_data + a_bss in the final a.out header

Updated Makefile to use piggyback_32 for sparc64.
Deleted the now unused piggyback_64.c

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
---
 arch/sparc/boot/Makefile       |    6 +-
 arch/sparc/boot/piggyback_32.c |   86 +++++++++++++++++++++++++------
 arch/sparc/boot/piggyback_64.c |  110 ----------------------------------------
 3 files changed, 72 insertions(+), 130 deletions(-)
 delete mode 100644 arch/sparc/boot/piggyback_64.c

Comments

Josip Rodin Jan. 1, 2011, 9:45 p.m. UTC | #1
On Sat, Jan 01, 2011 at 10:00:30PM +0100, Sam Ravnborg wrote:
> diff --git a/arch/sparc/boot/piggyback_64.c b/arch/sparc/boot/piggyback_64.c
> deleted file mode 100644
> index a26a686..0000000
> --- a/arch/sparc/boot/piggyback_64.c
> +++ /dev/null
> @@ -1,110 +0,0 @@
> -/*
> -   Simple utility to make a single-image install kernel with initial ramdisk
> -   for Sparc64 tftpbooting without need to set up nfs.
> -   
> -   Copyright (C) 1997 Jakub Jelinek (jj@sunsite.mff.cuni.cz)

Presumably you copied all that code from this file, so you should also
move over the copyright statement (the year in the other one differs).
Sam Ravnborg Jan. 1, 2011, 11:01 p.m. UTC | #2
Hi Josip.

On Sat, Jan 01, 2011 at 10:45:52PM +0100, Josip Rodin wrote:
> On Sat, Jan 01, 2011 at 10:00:30PM +0100, Sam Ravnborg wrote:
> > diff --git a/arch/sparc/boot/piggyback_64.c b/arch/sparc/boot/piggyback_64.c
> > deleted file mode 100644
> > index a26a686..0000000
> > --- a/arch/sparc/boot/piggyback_64.c
> > +++ /dev/null
> > @@ -1,110 +0,0 @@
> > -/*
> > -   Simple utility to make a single-image install kernel with initial ramdisk
> > -   for Sparc64 tftpbooting without need to set up nfs.
> > -   
> > -   Copyright (C) 1997 Jakub Jelinek (jj@sunsite.mff.cuni.cz)
> 
> Presumably you copied all that code from this file, so you should also
> move over the copyright statement (the year in the other one differs).

Thanks for looking through the patches.

I have based all the work on the _32.c variant.

From the _64.c variant I used only small bits to redo
the same functionality.

If this warrant to update the copyright or not I am not sure.
But as you brougth it to my attention I will do so and
add my own copyright in the process.

Jakub - you can see the original thread here: 
http://marc.info/?l=linux-sparc&m=129391542615466&w=2

	Sam
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Josip Rodin Jan. 1, 2011, 11:29 p.m. UTC | #3
On Sat, Jan 01, 2011 at 10:00:30PM +0100, Sam Ravnborg wrote:
> +		for (i = 0; i < LOOKBACK; ++i) {
> +			if (buffer[i + 0] == 'H' && buffer[i + 1] == 'd' &&
> +			    buffer[i + 2] == 'r' && buffer[i + 3] == 'S') {
> +				return offset + i;
> +			}
> +		}

> -	for (q = buffer, r = q + 512; q < r; q += 4) {
> -		if (*q == 'H' && q[1] == 'd' && q[2] == 'r' && q[3] == 'S')
> -			break;
> -	}

BTW, while the new code is eminently more readable in general, is there
a reason to slow down that search by using increments of 1 instead of 4?
Did the old one ever miss?
Sam Ravnborg Jan. 1, 2011, 11:48 p.m. UTC | #4
On Sun, Jan 02, 2011 at 12:29:17AM +0100, Josip Rodin wrote:
> On Sat, Jan 01, 2011 at 10:00:30PM +0100, Sam Ravnborg wrote:
> > +		for (i = 0; i < LOOKBACK; ++i) {
> > +			if (buffer[i + 0] == 'H' && buffer[i + 1] == 'd' &&
> > +			    buffer[i + 2] == 'r' && buffer[i + 3] == 'S') {
> > +				return offset + i;
> > +			}
> > +		}
> 
> > -	for (q = buffer, r = q + 512; q < r; q += 4) {
> > -		if (*q == 'H' && q[1] == 'd' && q[2] == 'r' && q[3] == 'S')
> > -			break;
> > -	}
> 
> BTW, while the new code is eminently more readable in general, is there
> a reason to slow down that search by using increments of 1 instead of 4?
> Did the old one ever miss?

I used increment by 1 because there was nothing in head_32/_64 comments
that guaranteed that "HdrS" would be on a 4-byte aligned address.
[The actual implmentation guarantees this today].

I had two options:
- use increment by one
- use increment by 4 and explain why it was safe to do so

And I just picked the simpler variant.

So I do not fix any know issue - just playing safe.
And speed does not matter here....

	Sam
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Josip Rodin Jan. 2, 2011, 12:54 a.m. UTC | #5
On Sun, Jan 02, 2011 at 12:48:35AM +0100, Sam Ravnborg wrote:
> On Sun, Jan 02, 2011 at 12:29:17AM +0100, Josip Rodin wrote:
> > On Sat, Jan 01, 2011 at 10:00:30PM +0100, Sam Ravnborg wrote:
> > > +		for (i = 0; i < LOOKBACK; ++i) {
> > > +			if (buffer[i + 0] == 'H' && buffer[i + 1] == 'd' &&
> > > +			    buffer[i + 2] == 'r' && buffer[i + 3] == 'S') {
> > > +				return offset + i;
> > > +			}
> > > +		}
> > 
> > > -	for (q = buffer, r = q + 512; q < r; q += 4) {
> > > -		if (*q == 'H' && q[1] == 'd' && q[2] == 'r' && q[3] == 'S')
> > > -			break;
> > > -	}
> > 
> > BTW, while the new code is eminently more readable in general, is there
> > a reason to slow down that search by using increments of 1 instead of 4?
> > Did the old one ever miss?
> 
> I used increment by 1 because there was nothing in head_32/_64 comments
> that guaranteed that "HdrS" would be on a 4-byte aligned address.
> [The actual implmentation guarantees this today].
> 
> I had two options:
> - use increment by one
> - use increment by 4 and explain why it was safe to do so
> 
> And I just picked the simpler variant.
> 
> So I do not fix any know issue - just playing safe.
> And speed does not matter here....

OK, just thought you might want to document that, given all the discussion
about arcane code. (Thanks.) I would actually hesitate from calling this one
the simpler variant, because making as few changes as possible compared to
the old code usually makes a refactoring simpler to debug if the need arises :)
Sam Ravnborg Jan. 2, 2011, 3:50 p.m. UTC | #6
On Sun, Jan 02, 2011 at 01:54:28AM +0100, Josip Rodin wrote:
> On Sun, Jan 02, 2011 at 12:48:35AM +0100, Sam Ravnborg wrote:
> > On Sun, Jan 02, 2011 at 12:29:17AM +0100, Josip Rodin wrote:
> > > On Sat, Jan 01, 2011 at 10:00:30PM +0100, Sam Ravnborg wrote:
> > > > +		for (i = 0; i < LOOKBACK; ++i) {
> > > > +			if (buffer[i + 0] == 'H' && buffer[i + 1] == 'd' &&
> > > > +			    buffer[i + 2] == 'r' && buffer[i + 3] == 'S') {
> > > > +				return offset + i;
> > > > +			}
> > > > +		}
> > > 
> > > > -	for (q = buffer, r = q + 512; q < r; q += 4) {
> > > > -		if (*q == 'H' && q[1] == 'd' && q[2] == 'r' && q[3] == 'S')
> > > > -			break;
> > > > -	}
> > > 
> > > BTW, while the new code is eminently more readable in general, is there
> > > a reason to slow down that search by using increments of 1 instead of 4?
> > > Did the old one ever miss?
> > 
> > I used increment by 1 because there was nothing in head_32/_64 comments
> > that guaranteed that "HdrS" would be on a 4-byte aligned address.
> > [The actual implmentation guarantees this today].
> > 
> > I had two options:
> > - use increment by one
> > - use increment by 4 and explain why it was safe to do so
> > 
> > And I just picked the simpler variant.
> > 
> > So I do not fix any know issue - just playing safe.
> > And speed does not matter here....
> 
> OK, just thought you might want to document that, given all the discussion
> about arcane code. (Thanks.) I would actually hesitate from calling this one
> the simpler variant, because making as few changes as possible compared to
> the old code usually makes a refactoring simpler to debug if the need arises :)

Good points!
I will aredo this to use increment-by-four and document this in v3 of the patchset.

	Sam
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/sparc/boot/Makefile b/arch/sparc/boot/Makefile
index 4c81fe5..06a79a2 100644
--- a/arch/sparc/boot/Makefile
+++ b/arch/sparc/boot/Makefile
@@ -6,7 +6,7 @@ 
 ROOT_IMG	:= /usr/src/root.img
 ELFTOAOUT	:= elftoaout
 
-hostprogs-y	:= piggyback_32 piggyback_64 btfixupprep
+hostprogs-y	:= piggyback_32 btfixupprep
 targets		:= tftpboot.img btfix.o btfix.S image zImage vmlinux.aout
 clean-files	:= System.map
 
@@ -69,7 +69,7 @@  endif
 
 ifeq ($(CONFIG_SPARC64),y)
 quiet_cmd_piggy     = PIGGY   $@
-      cmd_piggy     = $(obj)/piggyback_64 $@ System.map $(ROOT_IMG)
+      cmd_piggy     = $(obj)/piggyback_32 $(BITS) $@ System.map $(ROOT_IMG)
 quiet_cmd_strip     = STRIP   $@
       cmd_strip     = $(STRIP) -R .comment -R .note -K sun4u_init -K _end -K _start vmlinux -o $@
 
@@ -82,7 +82,7 @@  $(obj)/image: vmlinux FORCE
 $(obj)/zImage: $(obj)/image
 	$(call if_changed,gzip)
 
-$(obj)/tftpboot.img: $(obj)/image $(obj)/piggyback_64 System.map $(ROOT_IMG) FORCE
+$(obj)/tftpboot.img: $(obj)/image $(obj)/piggyback_32 System.map $(ROOT_IMG) FORCE
 	$(call if_changed,elftoaout)
 	$(call if_changed,piggy)
 	@echo '  kernel: $@ is ready'
diff --git a/arch/sparc/boot/piggyback_32.c b/arch/sparc/boot/piggyback_32.c
index c4b0843..82926f4 100644
--- a/arch/sparc/boot/piggyback_32.c
+++ b/arch/sparc/boot/piggyback_32.c
@@ -131,11 +131,59 @@  static int get_start_end(const char *filename, unsigned int *start, unsigned int
 	return 1;
 }
 
+#define LOOKBACK 512
+#define BUFSIZE 1024
+/*
+ * Find the HdrS entry from head_32/head_64.
+ * We check if it is at the beginning of the file (sparc64 case)
+ * and if not we search for it.
+ *
+ * Return the offset to the HdrS entry (as off_t)
+ */
+static off_t get_hdrs_offset(int kernelfd, const char *filename)
+{
+	char buffer[BUFSIZE];
+	off_t offset;
+	int i;
+
+	if (lseek(kernelfd, 0, SEEK_SET) < 0)
+		die("lseek");
+	if (read(kernelfd, buffer, BUFSIZE) != BUFSIZE)
+		die(filename);
+
+	if (buffer[40] == 'H' && buffer[41] == 'd' &&
+	    buffer[42] == 'r' && buffer[43] == 'S') {
+		return 40;
+	} else {
+		/*  Find the gokernel label */
+		/* Decode offset from branch instruction */
+		offset = ld2(buffer + AOUT_TEXT_OFFSET + 2) << 2;
+		/* Go back 512 bytes so we do not miss HdrS */
+		offset -= LOOKBACK;
+		/* skip a.out header */
+		offset += AOUT_TEXT_OFFSET;
+		if (lseek(kernelfd, offset, SEEK_SET) < 0)
+			die("lseek");
+		if (read(kernelfd, buffer, BUFSIZE) != BUFSIZE)
+			die(filename);
+
+		for (i = 0; i < LOOKBACK; ++i) {
+			if (buffer[i + 0] == 'H' && buffer[i + 1] == 'd' &&
+			    buffer[i + 2] == 'r' && buffer[i + 3] == 'S') {
+				return offset + i;
+			}
+		}
+	}
+	fprintf (stderr, "Couldn't find headers signature in %s\n", filename);
+	exit(1);
+}
+
 int main(int argc,char **argv)
 {
 	static char aout_magic[] = { 0x01, 0x03, 0x01, 0x07 };
-	char buffer[1024], *q, *r;
-	unsigned int i, start, end, offset;
+	char buffer[1024];
+	unsigned int i, start, end;
+	off_t offset;
 	struct stat s;
 	int image, tail;
 
@@ -165,21 +213,10 @@  int main(int argc,char **argv)
 	 * See definition of these in init_32.S
 	 */
 
-	/*  Find the gokernel label */
-	i = AOUT_TEXT_OFFSET + (ld2(buffer + AOUT_TEXT_OFFSET + 2) << 2) - 512;
-	if (lseek(image, i, 0) < 0)
-		die("lseek");
-	if (read(image, buffer, 1024) != 1024)
-		die(argv[2]);
-	for (q = buffer, r = q + 512; q < r; q += 4) {
-		if (*q == 'H' && q[1] == 'd' && q[2] == 'r' && q[3] == 'S')
-			break;
-	}
-	if (q == r) {
-		fprintf (stderr, "Couldn't find headers signature in the kernel.\n");
-		exit(1);
-	}
-	offset = i + (q - buffer) + 10;
+	offset = get_hdrs_offset(image, argv[2]);
+	/* skip HdrS + LINUX_VERSION_CODE + HdrS version */
+	offset += 10;
+
 	if (lseek(image, offset, 0) < 0)
 		die("lseek");
 
@@ -198,6 +235,21 @@  int main(int argc,char **argv)
 	if (write(image, buffer + 2, 14) != 14)
 		die(argv[2]);
 
+	/* For sparc64 update a_text and clear a_data + a_bss */
+	if (is64bit)
+	{
+		if (lseek(image, 4, 0) < 0)
+			die("lseek");
+		/* a_text */
+		st4(buffer, align(end + 32 + 8191) - (start & ~0x3fffffUL) + s.st_size);
+		/* a_data */
+		st4(buffer + 4, 0);
+		/* a_bss */
+		st4(buffer + 8, 0);
+		if (write(image, buffer, 12) != 12)
+			die(argv[2]);
+	}
+
 	/* seek page aligned boundary in the image file and add boot image */
 	if (lseek(image, AOUT_TEXT_OFFSET - start + align(end + 32), 0) < 0)
 		die("lseek");
diff --git a/arch/sparc/boot/piggyback_64.c b/arch/sparc/boot/piggyback_64.c
deleted file mode 100644
index a26a686..0000000
--- a/arch/sparc/boot/piggyback_64.c
+++ /dev/null
@@ -1,110 +0,0 @@ 
-/*
-   Simple utility to make a single-image install kernel with initial ramdisk
-   for Sparc64 tftpbooting without need to set up nfs.
-   
-   Copyright (C) 1997 Jakub Jelinek (jj@sunsite.mff.cuni.cz)
-   
-   This program is free software; you can redistribute it and/or modify
-   it under the terms of the GNU General Public License as published by
-   the Free Software Foundation; either version 2 of the License, or
-   (at your option) any later version.
-   
-   This program is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-   GNU General Public License for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with this program; if not, write to the Free Software
-   Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.  */
-   
-#include <stdio.h>
-#include <string.h>
-#include <ctype.h>
-#include <errno.h>
-#include <fcntl.h>
-#include <dirent.h>
-#include <unistd.h>
-#include <stdlib.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-
-/* Note: run this on an a.out kernel (use elftoaout for it), as PROM looks for a.out image onlly
-   usage: piggyback vmlinux System.map tail, where tail is gzipped fs of the initial ramdisk */
-
-static void die(char *str)
-{
-	perror (str);
-	exit(1);
-}
-
-int main(int argc,char **argv)
-{
-	char buffer [1024], *q, *r;
-	unsigned int i, j, k, start, end, offset;
-	FILE *map;
-	struct stat s;
-	int image, tail;
-	
-	start = end = 0;
-	if (stat (argv[3], &s) < 0) die (argv[3]);
-	map = fopen (argv[2], "r");
-	if (!map) die(argv[2]);
-	while (fgets (buffer, 1024, map)) {
-		if (!strcmp (buffer + 19, "_start\n"))
-		start = strtoul (buffer + 8, NULL, 16);
-		else if (!strcmp (buffer + 19, "_end\n"))
-		end = strtoul (buffer + 8, NULL, 16);
-	}
-	fclose (map);
-	if ((image = open(argv[1],O_RDWR)) < 0) die(argv[1]);
-	if (read(image,buffer,512) != 512) die(argv[1]);
-	if (!memcmp (buffer, "\177ELF", 4)) {
-		unsigned int *p = (unsigned int *)(buffer + *(unsigned int *)(buffer + 28));
-		
-		i = p[1] + *(unsigned int *)(buffer + 24) - p[2];
-		if (lseek(image,i,0) < 0) die("lseek");
-		if (read(image,buffer,512) != 512) die(argv[1]);
-		j = 0;
-	} else if (*(unsigned int *)buffer == 0x01030107) {
-		i = j = 32;
-	} else {
-		fprintf (stderr, "Not ELF nor a.out. Don't blame me.\n");
-		exit(1);
-	}
-	k = i;
-	if (j == 32 && buffer[40] == 'H' && buffer[41] == 'd' && buffer[42] == 'r' && buffer[43] == 'S') {
-		offset = 40 + 10;
-	} else {
-		i += ((*(unsigned short *)(buffer + j + 2))<<2) - 512;
-		if (lseek(image,i,0) < 0) die("lseek");
-		if (read(image,buffer,1024) != 1024) die(argv[1]);
-		for (q = buffer, r = q + 512; q < r; q += 4) {
-			if (*q == 'H' && q[1] == 'd' && q[2] == 'r' && q[3] == 'S')
-				break;
-		}
-		if (q == r) {
-			fprintf (stderr, "Couldn't find headers signature in the kernel.\n");
-			exit(1);
-		}
-		offset = i + (q - buffer) + 10;
-	}
-	if (lseek(image, offset, 0) < 0) die ("lseek");
-	*(unsigned *)buffer = 0;
-	*(unsigned *)(buffer + 4) = 0x01000000;
-	*(unsigned *)(buffer + 8) = ((end + 32 + 8191) & ~8191);
-	*(unsigned *)(buffer + 12) = s.st_size;
-	if (write(image,buffer+2,14) != 14) die (argv[1]);
-	if (lseek(image, 4, 0) < 0) die ("lseek");
-	*(unsigned *)buffer = ((end + 32 + 8191) & ~8191) - (start & ~0x3fffffUL) + s.st_size;
-	*(unsigned *)(buffer + 4) = 0;
-	*(unsigned *)(buffer + 8) = 0;
-	if (write(image,buffer,12) != 12) die (argv[1]);
-	if (lseek(image, k - start + ((end + 32 + 8191) & ~8191), 0) < 0) die ("lseek");
-	if ((tail = open(argv[3],O_RDONLY)) < 0) die(argv[3]);
-	while ((i = read (tail,buffer,1024)) > 0)
-		if (write(image,buffer,i) != i) die (argv[1]);
-	if (close(image) < 0) die("close");
-	if (close(tail) < 0) die("close");
-    	return 0;
-}