[PATHC] ATAG export for the linux kernel

[PATHC] ATAG export for the linux kernel

[Uli Luckas]

This is a path against the 2.6.21 kernel to export ATAG related information to 
procfs. It was inspired by the work and discussions with Mike Westerhof on 
the arm linux kernel mailing list.
The patch will we needed by the kexec user space binary to propperly set up 
ATAGs for the 'to be kexeced' kernel. 
A port to 2.6.23/2.6.24 should be mostly mechanical after removal of git 
commit 033b8ffe3f1ea8174d51d125838ac6deea60f63f (for someone who has a 
running 2.6.23 device :-). 

The export interface is via two procfs files:
/proc/atags/addr
/proc/atags/tags
"addr" holds the hex representation of the physical address where the running 
kernel expects it's ATAG list.
"tags" holds the binary ATAGs that where passed to the running kernel.

regards,
Uli Luckas

-- 

------- ROAD ...the handyPC Company - - -  ) ) )

Uli Luckas
Software Development

ROAD GmbH
Bennigsenstr. 14 | 12159 Berlin | Germany
fon: +49 (30) 230069 - 64 | fax: +49 (30) 230069 - 69
url: www.road.de

Amtsgericht Charlottenburg: HRB 96688 B
Managing directors: Hans-Peter Constien, Hubertus von Streit

Re: [PATHC] ATAG export for the linux kernel

[Uli Luckas]

And here comes the patch ...

Signed-off-by: Uli Luckas <u.luckas@road.de>
---
 arch/arm/kernel/Makefile |    2 +-
 arch/arm/kernel/atags.c  |  113 
++++++++++++++++++++++++++++++++++++++++++++++
 arch/arm/kernel/atags.h  |    2 +
 arch/arm/kernel/setup.c  |    3 +
 4 files changed, 119 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/kernel/atags.c
 create mode 100644 arch/arm/kernel/atags.h

diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index bb28087..188e1d1 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -8,7 +8,7 @@ AFLAGS_head.o := -DTEXT_OFFSET=$(TEXT_OFFSET)
 
 obj-y		:= compat.o entry-armv.o entry-common.o irq.o \
 		   process.o ptrace.o semaphore.o setup.o signal.o sys_arm.o \
-		   time.o traps.o
+		   time.o traps.o atags.o
 
 obj-$(CONFIG_ISA_DMA_API)	+= dma.o
 obj-$(CONFIG_ARCH_ACORN)	+= ecard.o 
diff --git a/arch/arm/kernel/atags.c b/arch/arm/kernel/atags.c
new file mode 100644
index 0000000..f0922a7
--- /dev/null
+++ b/arch/arm/kernel/atags.c
@@ -0,0 +1,113 @@
+#include <asm/setup.h>
+#include <linux/slab.h>
+#include <linux/proc_fs.h>
+#include <asm/page.h>
+
+struct buffer {
+        size_t size;
+        char *data;
+};
+
+static char atags_addr[12];
+
+static struct buffer addr_buffer =
+{
+        .size = sizeof(atags_addr) - 1,
+        .data = atags_addr,
+};
+static struct buffer tags_buffer;
+
+
+static int
+read_buffer(char* page, char** start, off_t off, int count,
+        int* eof, void* data)
+{
+        struct buffer *buffer = (struct buffer *)data;
+
+        if (off >= buffer->size) {
+                *eof = 1;
+                return 0;
+        }
+
+        count = min((int) (buffer->size - off), count);
+
+        memcpy(page, &buffer->data[off], count);
+
+        return count;
+}
+
+
+static int
+create_proc_entries(void)
+{
+        struct proc_dir_entry* atags_dir;
+        struct proc_dir_entry* addr_entry;
+        struct proc_dir_entry* tags_entry;
+
+        atags_dir = proc_mkdir("atags", NULL);
+        if(!atags_dir)
+                return -ENOMEM;
+
+        addr_entry = create_proc_read_entry("addr", 0400, atags_dir, 
read_buffer, &addr_buffer);
+        if(!addr_entry) {
+                remove_proc_entry("atags", NULL);
+                return -ENOMEM;
+        }
+
+        tags_entry = create_proc_read_entry("tags", 0400, atags_dir, 
read_buffer, &tags_buffer);
+        if(!tags_entry) {
+                remove_proc_entry("addr", atags_dir);
+                remove_proc_entry("atags", NULL);
+                return -ENOMEM;
+        }
+
+        return 0;
+}
+
+
+#define BOOT_PARAMS_SIZE 1536
+static unsigned long __initdata atags_phys;
+static char __initdata atags_copy_buf[BOOT_PARAMS_SIZE];
+static char __initdata *atags_copy;
+
+
+void __init save_atags(const unsigned long phys, const struct tag *tags) {
+        atags_phys = phys;
+        atags_copy = atags_copy_buf;
+        memcpy(atags_copy, tags, BOOT_PARAMS_SIZE);
+}
+
+
+static int
+__init init_atags_procfs(void)
+{
+        struct tag *tag;
+        int error;
+
+        if (!atags_copy) {
+                printk(KERN_WARNING "Exporting ATAGs: No saved tags 
found\n");
+                return -EIO;
+        }
+
+        snprintf(atags_addr, sizeof(atags_addr), "0x%08lx\n", atags_phys);
+        for (tag = (struct tag *) atags_copy; tag->hdr.size; tag = 
tag_next(tag))
+                ;
+
+        tags_buffer.size = ((char *) tag - atags_copy) + sizeof(tag->hdr);
+        tags_buffer.data = kmalloc(tags_buffer.size, GFP_KERNEL);
+        if (tags_buffer.data == NULL)
+                return -ENOMEM;
+        memcpy(tags_buffer.data, atags_copy, tags_buffer.size);
+
+        error = create_proc_entries();
+        if (error) {
+                printk(KERN_ERR "Exporting ATAGs: not enough memory\n");
+                kfree(tags_buffer.data);
+                tags_buffer.size = 0;
+                tags_buffer.data = NULL;
+        }
+
+        return error;
+}
+
+arch_initcall(init_atags_procfs);
diff --git a/arch/arm/kernel/atags.h b/arch/arm/kernel/atags.h
new file mode 100644
index 0000000..792c4a8
--- /dev/null
+++ b/arch/arm/kernel/atags.h
@@ -0,0 +1,2 @@
+extern void
+save_atags( unsigned long phys, struct tag *tags);
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 0453dcc..168b975 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -37,6 +37,7 @@
 #include <asm/mach/time.h>
 
 #include "compat.h"
+#include "atags.h"
 
 #ifndef MEM_SIZE
 #define MEM_SIZE	(16*1024*1024)
@@ -798,6 +799,8 @@ void __init setup_arch(char **cmdline_p)
 	if (tags->hdr.tag == ATAG_CORE) {
 		if (meminfo.nr_banks != 0)
 			squash_mem_tags(tags);
+		if (mdesc->boot_params)
+			save_atags(mdesc->boot_params, tags);
 		parse_tags(tags);
 	}
 
-- 
1.5.3.4

Re: [PATHC] ATAG export for the linux kernel

[Hans Henry von Tresckow]

On Nov 12, 2007 2:02 AM, Uli Luckas <u.luckas@road.de> wrote:

> And here comes the patch ...
>
> Signed-off-by: Uli Luckas <u.luckas@road.de>
> ---
>  arch/arm/kernel/Makefile |    2 +-
>  arch/arm/kernel/atags.c  |  113
> ++++++++++++++++++++++++++++++++++++++++++++++
>  arch/arm/kernel/atags.h  |    2 +
>  arch/arm/kernel/setup.c  |    3 +
>  4 files changed, 119 insertions(+), 1 deletions(-)
>  create mode 100644 arch/arm/kernel/atags.c
>  create mode 100644 arch/arm/kernel/atags.h
>
> diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
> index bb28087..188e1d1 100644
> --- a/arch/arm/kernel/Makefile
> +++ b/arch/arm/kernel/Makefile
> @@ -8,7 +8,7 @@ AFLAGS_head.o := -DTEXT_OFFSET=$(TEXT_OFFSET)
>
>  obj-y          := compat.o entry-armv.o entry-common.o irq.o \
>                   process.o ptrace.o semaphore.o setup.o signal.osys_arm.o \
> -                  time.o traps.o
> +                  time.o traps.o atags.o
>
>  obj-$(CONFIG_ISA_DMA_API)      += dma.o
>  obj-$(CONFIG_ARCH_ACORN)       += ecard.o
> diff --git a/arch/arm/kernel/atags.c b/arch/arm/kernel/atags.c
> new file mode 100644
> index 0000000..f0922a7
> --- /dev/null
> +++ b/arch/arm/kernel/atags.c
> @@ -0,0 +1,113 @@
> +#include <asm/setup.h>
> +#include <linux/slab.h>
> +#include <linux/proc_fs.h>
> +#include <asm/page.h>
> +
> +struct buffer {
> +        size_t size;
> +        char *data;
> +};
> +
> +static char atags_addr[12];
> +
> +static struct buffer addr_buffer =
> +{
> +        .size = sizeof(atags_addr) - 1,
> +        .data = atags_addr,
> +};
> +static struct buffer tags_buffer;
> +
> +
> +static int
> +read_buffer(char* page, char** start, off_t off, int count,
> +        int* eof, void* data)
> +{
> +        struct buffer *buffer = (struct buffer *)data;
> +
> +        if (off >= buffer->size) {
> +                *eof = 1;
> +                return 0;
> +        }
> +
> +        count = min((int) (buffer->size - off), count);
> +
> +        memcpy(page, &buffer->data[off], count);
> +
> +        return count;
> +}
> +
> +
> +static int
> +create_proc_entries(void)
> +{
> +        struct proc_dir_entry* atags_dir;
> +        struct proc_dir_entry* addr_entry;
> +        struct proc_dir_entry* tags_entry;
> +
> +        atags_dir = proc_mkdir("atags", NULL);
> +        if(!atags_dir)
> +                return -ENOMEM;
> +
> +        addr_entry = create_proc_read_entry("addr", 0400, atags_dir,
> read_buffer, &addr_buffer);
> +        if(!addr_entry) {
> +                remove_proc_entry("atags", NULL);
> +                return -ENOMEM;
> +        }
> +
> +        tags_entry = create_proc_read_entry("tags", 0400, atags_dir,
> read_buffer, &tags_buffer);
> +        if(!tags_entry) {
> +                remove_proc_entry("addr", atags_dir);
> +                remove_proc_entry("atags", NULL);
> +                return -ENOMEM;
> +        }
> +
> +        return 0;
> +}
> +
> +
> +#define BOOT_PARAMS_SIZE 1536
> +static unsigned long __initdata atags_phys;
> +static char __initdata atags_copy_buf[BOOT_PARAMS_SIZE];
> +static char __initdata *atags_copy;
> +
> +
> +void __init save_atags(const unsigned long phys, const struct tag *tags)
> {
> +        atags_phys = phys;
> +        atags_copy = atags_copy_buf;
> +        memcpy(atags_copy, tags, BOOT_PARAMS_SIZE);
> +}
> +
> +
> +static int
> +__init init_atags_procfs(void)
> +{
> +        struct tag *tag;
> +        int error;
> +
> +        if (!atags_copy) {
> +                printk(KERN_WARNING "Exporting ATAGs: No saved tags
> found\n");
> +                return -EIO;
> +        }
> +
> +        snprintf(atags_addr, sizeof(atags_addr), "0x%08lx\n",
> atags_phys);
> +        for (tag = (struct tag *) atags_copy; tag->hdr.size; tag =
> tag_next(tag))
> +                ;
> +
> +        tags_buffer.size = ((char *) tag - atags_copy) +
> sizeof(tag->hdr);
> +        tags_buffer.data = kmalloc(tags_buffer.size, GFP_KERNEL);
> +        if (tags_buffer.data == NULL)
> +                return -ENOMEM;
> +        memcpy(tags_buffer.data, atags_copy, tags_buffer.size);
> +
> +        error = create_proc_entries();
> +        if (error) {
> +                printk(KERN_ERR "Exporting ATAGs: not enough memory\n");
> +                kfree(tags_buffer.data);
> +                tags_buffer.size = 0;
> +                tags_buffer.data = NULL;
> +        }
> +
> +        return error;
> +}
> +
> +arch_initcall(init_atags_procfs);
> diff --git a/arch/arm/kernel/atags.h b/arch/arm/kernel/atags.h
> new file mode 100644
> index 0000000..792c4a8
> --- /dev/null
> +++ b/arch/arm/kernel/atags.h
> @@ -0,0 +1,2 @@
> +extern void
> +save_atags( unsigned long phys, struct tag *tags);
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index 0453dcc..168b975 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -37,6 +37,7 @@
>  #include <asm/mach/time.h>
>
>  #include "compat.h"
> +#include "atags.h"
>
>  #ifndef MEM_SIZE
>  #define MEM_SIZE       (16*1024*1024)
> @@ -798,6 +799,8 @@ void __init setup_arch(char **cmdline_p)
>        if (tags->hdr.tag == ATAG_CORE) {
>                if (meminfo.nr_banks != 0)
>                        squash_mem_tags(tags);
> +               if (mdesc->boot_params)
> +                       save_atags(mdesc->boot_params, tags);
>                parse_tags(tags);
>        }
>
> --
> 1.5.3.4
>
>
Tested on 2.6.21 and 2.6.17 for poodle. Thank you very much. Any Chance you
could push this upstream?

-- 
Henry von Tresckow (hvontres)

Re: [PATHC] ATAG export for the linux kernel

[Uli Luckas]

Hans Henry von Tresckow wrote:
> On Nov 12, 2007 2:02 AM, Uli Luckas <u.luckas@road.de 
> <mailto:u.luckas@road.de>> wrote:
>
>     And here comes the patch ...
>
>
> Tested on 2.6.21 and 2.6.17 for poodle. Thank you very much. Any 
> Chance you could push this upstream?
Hi Hans Henry, Richard and Mike

Hans Henry, thanks for testing. Good to know it worked for you. Let's 
see how
we can get this thing pushed upstream.

1) I put this on the OE list as the original code is maintained in OE. And
that's why I'd like the code to be pushed to the OE repository first.
I am not really an OE developer though. Is someone willing to do the OE
integration to give my patch a home?

2) I need to push the kernel patch via Russell. Mike, I have not heard your
opinion. Will you support this patch?

3) I need to push my userspace work. My work is based on code by Richard
though. Richard, please jump in and let us know how you'd like to proceede?

I'd really like to see kexec working on arm without any patches soon.  
Let's
see...

regards,
Uli

-- 

------- ROAD ...the handyPC Company - - -  ) ) )

Uli Luckas
Software Development

ROAD GmbH
Bennigsenstr. 14 | 12159 Berlin | Germany
fon: +49 (30) 230069 - 64 | fax: +49 (30) 230069 - 69
url: www.road.de

Amtsgericht Charlottenburg: HRB 96688 B
Managing directors: Hans-Peter Constien, Hubertus von Streit

Re: [PATHC] ATAG export for the linux kernel

[Richard Purdie]

On Fri, 2007-12-07 at 11:06 +0000, Uli Luckas wrote:
> Hans Henry, thanks for testing. Good to know it worked for you. Let's 
> see how we can get this thing pushed upstream.
> 
> 1) I put this on the OE list as the original code is maintained in OE. And
> that's why I'd like the code to be pushed to the OE repository first.
> I am not really an OE developer though. Is someone willing to do the OE
> integration to give my patch a home?

I'm happy for the userspace side of things to be added in OE. I have
some reservations about the way the kernel patch is added to the zaurus
code, particularly about the way the kernel commandline is handled from
the broken bootloader and I need to find some time to look at that. I'm
fine with the patch in principle though, I just want it right when its
added (and preferably upstream too, the existing zaurus changes will not
be accepted upstream).

> 2) I need to push the kernel patch via Russell. Mike, I have not heard your
> opinion. Will you support this patch?
>
> 3) I need to push my userspace work. My work is based on code by Richard
> though. Richard, please jump in and let us know how you'd like to proceede?

I did try and find the kexec-tools maintainer but got distracted and
never got back to it. Does anyone know who we need to send this to? I
will try and get that done this weekend...

> I'd really like to see kexec working on arm without any patches soon.  

Agreed. I'm sorry I haven't found time to look at this yet, I will try
hard to do so this weekend.

Regards,

Richard

Re: [PATHC] ATAG export for the linux kernel

[Hans Henry von Tresckow]

On Dec 7, 2007 3:38 AM, Richard Purdie <rpurdie@rpsys.net> wrote:

> On Fri, 2007-12-07 at 11:06 +0000, Uli Luckas wrote:
> > Hans Henry, thanks for testing. Good to know it worked for you. Let's
> > see how we can get this thing pushed upstream.
> >
> > 1) I put this on the OE list as the original code is maintained in OE.
> And
> > that's why I'd like the code to be pushed to the OE repository first.
> > I am not really an OE developer though. Is someone willing to do the OE
> > integration to give my patch a home?
>
> I'm happy for the userspace side of things to be added in OE. I have
> some reservations about the way the kernel patch is added to the zaurus
> code, particularly about the way the kernel commandline is handled from
> the broken bootloader and I need to find some time to look at that. I'm
> fine with the patch in principle though, I just want it right when its
> added (and preferably upstream too, the existing zaurus changes will not
> be accepted upstream).
>
> > 2) I need to push the kernel patch via Russell. Mike, I have not heard
> your
> > opinion. Will you support this patch?
> >
> > 3) I need to push my userspace work. My work is based on code by Richard
> > though. Richard, please jump in and let us know how you'd like to
> proceede?
>
> I did try and find the kexec-tools maintainer but got distracted and
> never got back to it. Does anyone know who we need to send this to? I
> will try and get that done this weekend...
>
> > I'd really like to see kexec working on arm without any patches soon.
>
> Agreed. I'm sorry I haven't found time to look at this yet, I will try
> hard to do so this weekend.
>
> Regards,
>
> Richard
>
>
>
> Richard,

I updated http://bugs.openembedded.org/show_bug.cgi?id=3397 with a new patch
to filter out the Sharp command line (for poodle at least). Let me know if
that approach makes more sense.

http://bugs.openembedded.org/show_bug.cgi?id=3430 has the userspace kexec
patch.

Thanks for taking a look at this.


-- 
Henry von Tresckow (hvontres)

Re: [PATHC] ATAG export for the linux kernel

[Richard Purdie]

Hi,

On Fri, 2007-12-07 at 08:18 -0800, Hans Henry von Tresckow wrote:
> I updated http://bugs.openembedded.org/show_bug.cgi?id=3397 with a new
> patch to filter out the Sharp command line (for poodle at least). Let
> me know if that approach makes more sense.
>
> http://bugs.openembedded.org/show_bug.cgi?id=3430 has the userspace
> kexec patch.
>
> Thanks for taking a look at this.

I've had a look at the patches, have given them a lot of thought and I'm
not convinced the approach is quite right. I do have an alternative
proposal however.

I agree we need access to the existing atags from the kernel. To get
them I think we should memcpy them as per the existing code but do this
after any old style boot params have been converted into atags.

When it comes to kexec'ing the new kernel, we setup the new kernel at
the 32kiB offset as we currently to but we copy our new set of atags
into the first 16kiB region. We then point r2 of the new kernel at this
memory address and the new kernel should use this set of tags.

This means we don't have to mess around trying to work out what address
to put the atags, boot_params etc. at and also it means we know for
certain that the options passed from kexec are the ones used, not some
other random options. It also means no modifications are required to
existing machines such as the zaurus.

I'd also consider having kexec extract the commandline from the running
kernel is none is specified.

I will try and implement this if I get a chance and see how it works...

Cheers,

Richard

Re: [PATHC] ATAG export for the linux kernel

[Uli Luckas]

On Friday, 7. December 2007, Hans Henry von Tresckow wrote:
 > On Dec 7, 2007 3:38 AM, Richard Purdie <rpurdie@rpsys.net> wrote:
 > > On Fri, 2007-12-07 at 11:06 +0000, Uli Luckas wrote:
 > > > Hans Henry, thanks for testing. Good to know it worked for you. Let's
 > > > see how we can get this thing pushed upstream.
 > > >
 > > > 1) I put this on the OE list as the original code is maintained 
in OE.
 > >
 > > And
 > >
 > > > that's why I'd like the code to be pushed to the OE repository first.
 > > > I am not really an OE developer though. Is someone willing to do 
the OE
 > > > integration to give my patch a home?
 > >
 > > I'm happy for the userspace side of things to be added in OE. I have
 > > some reservations about the way the kernel patch is added to the zaurus
 > > code, particularly about the way the kernel commandline is handled 
from
 > > the broken bootloader and I need to find some time to look at that. I'm
 > > fine with the patch in principle though, I just want it right when its
 > > added (and preferably upstream too, the existing zaurus changes 
will not
 > > be accepted upstream).
 > >
 > > > 2) I need to push the kernel patch via Russell. Mike, I have not 
heard
 > > your opinion. Will you support this patch?
 > > >
 > > > 3) I need to push my userspace work. My work is based on code by
 > > > Richard though. Richard, please jump in and let us know how you'd 
like
 > > > to
 > >
 > > proceede?
 > >
 > > I did try and find the kexec-tools maintainer but got distracted and
 > > never got back to it. Does anyone know who we need to send this to? I
 > > will try and get that done this weekend...
 > >
 > > > I'd really like to see kexec working on arm without any patches soon.
 > >
 > > Agreed. I'm sorry I haven't found time to look at this yet, I will try
 > > hard to do so this weekend.
 > >
 > > Regards,
 > >
 > > Richard
 > >
 > >
 > >
 > > Richard,
 >
 > I updated http://bugs.openembedded.org/show_bug.cgi?id=3397 with a new
 > patch to filter out the Sharp command line (for poodle at least). Let me
 > know if that approach makes more sense.
 >
I think I am missing something here. Didn't you (Hans Henry) report my 
kernel
patch to work on poodle?
As I don't have a Sharp, could someone clue me in what problem you guys are
trying to sove here?

 > http://bugs.openembedded.org/show_bug.cgi?id=3430 has the userspace kexec
 > patch.
 >
Thanks for forwarding the patch to the right channel.

Regards,
Uli

-- 

------- ROAD ...the handyPC Company - - -  ) ) )

Uli Luckas
Software Development

ROAD GmbH
Bennigsenstr. 14 | 12159 Berlin | Germany
fon: +49 (30) 230069 - 64 | fax: +49 (30) 230069 - 69
url: www.road.de

Amtsgericht Charlottenburg: HRB 96688 B
Managing directors: Hans-Peter Constien, Hubertus von Streit

Re: [PATHC] ATAG export for the linux kernel

[Uli Luckas]

On Friday, 7. December 2007, Hans Henry von Tresckow wrote:
 > I updated http://bugs.openembedded.org/show_bug.cgi?id=3397 with a new
 > patch to filter out the Sharp command line (for poodle at least). Let me
 > know if that approach makes more sense.
 >
Hans Henry,
this patch still contains patch 80-kexec-atags.patch which should be 
obsoleted
by my patch.
The basic idea is that the kexec mechanism already in place moves the 
ATAGs to
the expected location. That's why it does not need any modifications to
arch/arm/kernel/relocate_kernel.S.
Could you please test again with 80-kexec-atags.patch _not_ applied?

Thanks
Uli


-- 

------- ROAD ...the handyPC Company - - -  ) ) )

Uli Luckas
Software Development

ROAD GmbH
Bennigsenstr. 14 | 12159 Berlin | Germany
fon: +49 (30) 230069 - 64 | fax: +49 (30) 230069 - 69
url: www.road.de

Amtsgericht Charlottenburg: HRB 96688 B
Managing directors: Hans-Peter Constien, Hubertus von Streit

Re: [PATHC] ATAG export for the linux kernel

[Uli Luckas]

Richard Purdie wrote:
> On Fri, 2007-12-07 at 08:18 -0800, Hans Henry von Tresckow wrote:
>   
>> I updated http://bugs.openembedded.org/show_bug.cgi?id=3397 with a new
>> patch to filter out the Sharp command line (for poodle at least). Let
>> me know if that approach makes more sense.
>>
>> http://bugs.openembedded.org/show_bug.cgi?id=3430 has the userspace
>> kexec patch.
>>
>> Thanks for taking a look at this.
>>     
>
> I've had a look at the patches, have given them a lot of thought and I'm
> not convinced the approach is quite right. I do have an alternative
> proposal however
Richard,
I think we are talking about different approaches here. The patch Hans 
Henry but on bugs.openembedded.org is not what I posted. It still 
contains code from Mike which is not needed any more. Could you please 
reconsider export_atags_2.patch + the userspace patches only?

Regards
Uli

-- 


------- ROAD ...the handyPC Company - - -  ) ) )

Uli Luckas
Software Development

ROAD GmbH
Bennigsenstr. 14 | 12159 Berlin | Germany
fon: +49 (30) 230069 - 64 | fax: +49 (30) 230069 - 69
url: www.road.de

Amtsgericht Charlottenburg: HRB 96688 B
Managing directors: Hans-Peter Constien, Hubertus von Streit

Re: [PATHC] ATAG export for the linux kernel

[Richard Purdie]

Hi Uli,

On Mon, 2007-12-10 at 10:26 +0000, Uli Luckas wrote:
> Richard Purdie wrote:
> I think we are talking about different approaches here. The patch Hans 
> Henry but on bugs.openembedded.org is not what I posted. It still 
> contains code from Mike which is not needed any more. Could you please 
> reconsider export_atags_2.patch + the userspace patches only?

Hans Henry is working against an old 2.6.17 kernel and I think the code
from Mike was merged into mainline but is still needed for 2.6.17. There
are a few problems in the Zaurus use case (and in general).

I appreciate his backport of the atags patch isn't 100% up to date but
its actually 2.6.23+patches I've been looking at (I'm assuming
backporting is straightforward and will therefore worry about that
later).

The zaurus does not set boot_params. This means the section of code
which does this:

--- linux-2.6.21_orig2/arch/arm/kernel/setup.c	2007-11-16 13:17:11.000000000 -0800
+++ linux-2.6.21/arch/arm/kernel/setup.c	2007-11-16 13:24:26.000000000 -0800
@@ -823,6 +824,9 @@
 	if (tags->hdr.tag == ATAG_CORE) {
 		if (meminfo.nr_banks != 0)
 			squash_mem_tags(tags);
+                if (mdesc->boot_params)
+                        save_atags(mdesc->boot_params, tags);
+
 		parse_tags(tags);
 	}

will not work.

In setup.c there is also the code:

	if (__atags_pointer) {
		kexec_boot_params_address = __atags_pointer;
		memcpy((void *)kexec_boot_params, tags, KEXEC_BOOT_PARAMS_SIZE);
	} else if (mdesc->boot_params) {
		kexec_boot_params_address = mdesc->boot_params;
		memcpy((void *)kexec_boot_params, tags, KEXEC_BOOT_PARAMS_SIZE);
	}

and the structure pointed at by boot_params could be in the old format
(see arch/arm/compat.c).

So, I think that:
  * save_atags() should be called unconditionally
  * We should memcpy the tags after the conversion or rely entirely on 
    save_atags() (probably the latter).
  * Userspace should always rebuild the tag list and provide in a new 
    location within the first 32kiB so machines which don't use 
    boot_params still work and only one structure is used to pass 
    parameters, simplifying the situation.

This isn't really a fundamental change to how it currently works, just a
simplification to make it clear, simple and work in all cases.

Cheers,

Richard

Re: [PATHC] ATAG export for the linux kernel

[Richard Purdie]

On Mon, 2007-12-10 at 09:56 +0000, Uli Luckas wrote:
> On Friday, 7. December 2007, Hans Henry von Tresckow wrote:
>  > On Dec 7, 2007 3:38 AM, Richard Purdie <rpurdie@rpsys.net> wrote:
>  > I updated http://bugs.openembedded.org/show_bug.cgi?id=3397 with a new
>  > patch to filter out the Sharp command line (for poodle at least). Let me
>  > know if that approach makes more sense.
>
> I think I am missing something here. Didn't you (Hans Henry) report my 
> kernel
> patch to work on poodle?
> As I don't have a Sharp, could someone clue me in what problem you guys are
> trying to sove here?

It can work but the changes needed to the kernel to make it work are not
acceptable in my opinion (and likely in mainline's opinion too). I'm
therefore trying to find a way we can add the feature in a way
acceptable to mainline

Regards,

Richard

Re: [PATHC] ATAG export for the linux kernel

[Mike (mwester)]

Uli Luckas writes:
> Richard Purdie wrote:
> > On Fri, 2007-12-07 at 08:18 -0800, Hans Henry von Tresckow wrote:
> >
> >> I updated http://bugs.openembedded.org/show_bug.cgi?id=3397 with a new
> >> patch to filter out the Sharp command line (for poodle at least). Let
> >> me know if that approach makes more sense.
> >>
> >> http://bugs.openembedded.org/show_bug.cgi?id=3430 has the userspace
> >> kexec patch.
> >>
> >> Thanks for taking a look at this.
> >>
> >
> > I've had a look at the patches, have given them a lot of thought and I'm
> > not convinced the approach is quite right. I do have an alternative
> > proposal however
> Richard,
> I think we are talking about different approaches here. The patch Hans
> Henry but on bugs.openembedded.org is not what I posted. It still
> contains code from Mike which is not needed any more. Could you please
> reconsider export_atags_2.patch + the userspace patches only?

I haven't had time to follow this thread closely (I apologize, time is at a
premium at this time of year for me).  I think I should mention that what
used to be the 80-kexec-atags.patch will be part of the 2.6.24 kernel, so
the presence of that code should be taken into account with any patching.
With that patch, the buffer location for the stored atags is an exported
symbol; the intent was to permit other code (kexec, for example) to read and
modify the atags prior to a kexec.

I'll have a bit more time as soon as the year ends (silly year-end company
stuff, I'm afraid).

Mike (mwester)

Re: [PATHC] ATAG export for the linux kernel

[Hans Henry von Tresckow]

On Dec 10, 2007 1:56 AM, Uli Luckas <u.luckas@road.de> wrote:

> On Friday, 7. December 2007, Hans Henry von Tresckow wrote:
>  > On Dec 7, 2007 3:38 AM, Richard Purdie <rpurdie@rpsys.net> wrote:
>  > > On Fri, 2007-12-07 at 11:06 +0000, Uli Luckas wrote:
>  > > > Hans Henry, thanks for testing. Good to know it worked for you.
> Let's
>  > > > see how we can get this thing pushed upstream.
>  > > >
>  > > > 1) I put this on the OE list as the original code is maintained
> in OE.
>  > >
>  > > And
>  > >
>  > > > that's why I'd like the code to be pushed to the OE repository
> first.
>  > > > I am not really an OE developer though. Is someone willing to do
> the OE
>  > > > integration to give my patch a home?
>  > >
>  > > I'm happy for the userspace side of things to be added in OE. I have
>  > > some reservations about the way the kernel patch is added to the
> zaurus
>  > > code, particularly about the way the kernel commandline is handled
> from
>  > > the broken bootloader and I need to find some time to look at that.
> I'm
>  > > fine with the patch in principle though, I just want it right when
> its
>  > > added (and preferably upstream too, the existing zaurus changes
> will not
>  > > be accepted upstream).
>  > >
>  > > > 2) I need to push the kernel patch via Russell. Mike, I have not
> heard
>  > > your opinion. Will you support this patch?
>  > > >
>  > > > 3) I need to push my userspace work. My work is based on code by
>  > > > Richard though. Richard, please jump in and let us know how you'd
> like
>  > > > to
>  > >
>  > > proceede?
>  > >
>  > > I did try and find the kexec-tools maintainer but got distracted and
>  > > never got back to it. Does anyone know who we need to send this to? I
>  > > will try and get that done this weekend...
>  > >
>  > > > I'd really like to see kexec working on arm without any patches
> soon.
>  > >
>  > > Agreed. I'm sorry I haven't found time to look at this yet, I will
> try
>  > > hard to do so this weekend.
>  > >
>  > > Regards,
>  > >
>  > > Richard
>  > >
>  > >
>  > >
>  > > Richard,
>  >
>  > I updated http://bugs.openembedded.org/show_bug.cgi?id=3397 with a new
>  > patch to filter out the Sharp command line (for poodle at least). Let
> me
>  > know if that approach makes more sense.
>  >
> I think I am missing something here. Didn't you (Hans Henry) report my
> kernel
> patch to work on poodle?
> As I don't have a Sharp, could someone clue me in what problem you guys
> are
> trying to sove here?
>
>  > http://bugs.openembedded.org/show_bug.cgi?id=3430 has the userspace
> kexec
>  > patch.
>  >
>
Uli, the basic problem with Sharp machines is that the bootloader has the
default commandline for the old 2.4.x embeddix/lineo kernels. This
commandline is completely unusable by the 2.6 kernel series. To get mikes
and your patches working I have had to resort to one of two methods, either
of which Richard considers a no-no:
1) set up .boot_params at  0xa0000200 instead of the more standard
0xa0000100
2) patch the kernel to check the command line and reject it if it looks like
the standard Sharp one.

>
> Thanks for forwarding the patch to the right channel.
>
> Regards,
> Uli
>
> --
>
> ------- ROAD ...the handyPC Company - - -  ) ) )
>
> Uli Luckas
> Software Development
>
> ROAD GmbH
> Bennigsenstr. 14 | 12159 Berlin | Germany
> fon: +49 (30) 230069 - 64 | fax: +49 (30) 230069 - 69
> url: www.road.de
>
> Amtsgericht Charlottenburg: HRB 96688 B
> Managing directors: Hans-Peter Constien, Hubertus von Streit
>
>


-- 
Henry von Tresckow (hvontres)

Re: [PATHC] ATAG export for the linux kernel

[Uli Luckas]

On Monday 10 December 2007, Richard Purdie wrote:
> Hi Uli,
>
> On Mon, 2007-12-10 at 10:26 +0000, Uli Luckas wrote:
> > Richard Purdie wrote:
> > I think we are talking about different approaches here. The patch Hans
> > Henry but on bugs.openembedded.org is not what I posted. It still
> > contains code from Mike which is not needed any more. Could you please
> > reconsider export_atags_2.patch + the userspace patches only?
>
> Hans Henry is working against an old 2.6.17 kernel and I think the code
> from Mike was merged into mainline but is still needed for 2.6.17. There
> are a few problems in the Zaurus use case (and in general).
>
Richard,
thanks for explaining. I still see a need to clarify some things.
I am working on an older kernel (2.6.21) which does not have Mike's patch 
either and my original posting mentions: "A port to 2.6.23/2.6.24 should be 
mostly mechanical after removal of git commit 
033b8ffe3f1ea8174d51d125838ac6deea60f63f" This commit is basically 
80-kexec-atags.patch

Mikes approach is orthogonal to mine. Let's see how they compare:

Mike's patch saves the atags to a bufer within the kernel and brings extra 
kernel code in relocate_kernel.S to relocate this buffer into place after 
kexec. The buffer is an exported symbol so a (not yet written) module could 
provide ways to manipulate this bufer before kexec is executed.

My approach exports the atags plus their physical address to userspace via 
procfs. Then there is my usersace patch which enables kexec to read the atags 
and manipulate the command line and initrd tags. 
The modified atags are then provided as a memory segement to the kexec system 
call for later relocation to the expected physical address. This relocation 
is done by the regular kexec mechanisms (your original kexec patch). 
Obviously Mikes modifications are not needed for this to work.
 
Of course, I am convinced that my approach is more elegant ;-)

> I appreciate his backport of the atags patch isn't 100% up to date but
> its actually 2.6.23+patches I've been looking at.
>
Right, but 2.6.23+patches contains two seperate approaches to the atags 
relocation. As mentioned above we need 2.6.23 
_minus_033b8ffe3f1ea8174d51d125838ac6deea60f63f_ plus my patch.

> (I'm assuming  backporting is straightforward and will therefore worry about
> that later).
> The zaurus does not set boot_params. This means the section of code
> which does this:
>
> --- linux-2.6.21_orig2/arch/arm/kernel/setup.c	2007-11-16
> 13:17:11.000000000 -0800 +++
> linux-2.6.21/arch/arm/kernel/setup.c	2007-11-16 13:24:26.000000000 -0800 @@
> -823,6 +824,9 @@
>  	if (tags->hdr.tag == ATAG_CORE) {
>  		if (meminfo.nr_banks != 0)
>  			squash_mem_tags(tags);
> +                if (mdesc->boot_params)
> +                        save_atags(mdesc->boot_params, tags);
> +
>  		parse_tags(tags);
>  	}
>
> will not work.
>
OK. I didn't know that. Lurking to the code below, shouldn't something like 
this work:
if (__atags_pointer)
    save_atags(__atags_pointer, tags);
else if (mdesc->boot_params)
    save_atags(mdesc->boot_params, tags);

> In setup.c there is also the code:
>
> 	if (__atags_pointer) {
> 		kexec_boot_params_address = __atags_pointer;
> 		memcpy((void *)kexec_boot_params, tags, KEXEC_BOOT_PARAMS_SIZE);
> 	} else if (mdesc->boot_params) {
> 		kexec_boot_params_address = mdesc->boot_params;
> 		memcpy((void *)kexec_boot_params, tags, KEXEC_BOOT_PARAMS_SIZE);
> 	}
>
> and the structure pointed at by boot_params could be in the old format
> (see arch/arm/compat.c).
>
This is Mike's code. This is one of the examples where things are done twice 
if you have mikes and my patch applied ...

> So, I think that:
>   * save_atags() should be called unconditionally
Or maybe as suggested above?

>   * We should memcpy the tags after the conversion or rely entirely on
>     save_atags() (probably the latter).
Yep, the latter. This is what you get if you drop Mike's patch.

>   * Userspace should always rebuild the tag list and provide in a new
>     location within the first 32kiB so machines which don't use
>     boot_params still work and only one structure is used to pass
>     parameters, simplifying the situation.
>
This is exactly what my approach does if you extend it to use __atags_pointer.

> This isn't really a fundamental change to how it currently works, just a
> simplification to make it clear, simple and work in all cases.
>
Absolutely agreed.

regards,
Uli