| Message ID | 1310475369-3000-1-git-send-email-kaloz@openwrt.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jul 12, 2011 at 02:56:09PM +0200, Imre Kaloz wrote:
> Wire up support for the XZ decompressor
Ok.
On Fri, 15 Jul 2011 18:25:19 +0200, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Tue, Jul 12, 2011 at 02:56:09PM +0200, Imre Kaloz wrote: >> Wire up support for the XZ decompressor > > Ok. Thanks, added to the patch system. Imre
On Sat, Jul 16, 2011 at 04:07:34PM +0200, Imre Kaloz wrote: > On Fri, 15 Jul 2011 18:25:19 +0200, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > >> On Tue, Jul 12, 2011 at 02:56:09PM +0200, Imre Kaloz wrote: >>> Wire up support for the XZ decompressor >> >> Ok. > > Thanks, added to the patch system. FYI: Patching 7001/1... <stdin>:56: space before tab in indent. $(addprefix $(obj)/, $(OBJS)) $(lib1funcs) $(ashldi3) FORCE warning: 1 line applied after fixing whitespace errors.
On Tue, Jul 19, 2011 at 12:02:35PM +0100, Russell King - ARM Linux wrote: > On Sat, Jul 16, 2011 at 04:07:34PM +0200, Imre Kaloz wrote: > > On Fri, 15 Jul 2011 18:25:19 +0200, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > > > >> On Tue, Jul 12, 2011 at 02:56:09PM +0200, Imre Kaloz wrote: > >>> Wire up support for the XZ decompressor > >> > >> Ok. > > > > Thanks, added to the patch system. > > FYI: > > Patching 7001/1... > <stdin>:56: space before tab in indent. > $(addprefix $(obj)/, $(OBJS)) $(lib1funcs) $(ashldi3) FORCE > warning: 1 line applied after fixing whitespace errors. BTW, has anyone outside of the ARM community seen the patch to lib/xz/xz_dec_stream.c adding the linux/kernel.h include? How well tested is that change on other architectures?
On Tue, 19 Jul 2011 13:02:35 +0200, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Sat, Jul 16, 2011 at 04:07:34PM +0200, Imre Kaloz wrote: >> On Fri, 15 Jul 2011 18:25:19 +0200, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: >> >>> On Tue, Jul 12, 2011 at 02:56:09PM +0200, Imre Kaloz wrote: >>>> Wire up support for the XZ decompressor >>> >>> Ok. >> >> Thanks, added to the patch system. > > FYI: > > Patching 7001/1... > <stdin>:56: space before tab in indent. > $(addprefix $(obj)/, $(OBJS)) $(lib1funcs) $(ashldi3) FORCE > warning: 1 line applied after fixing whitespace errors. > That space is there since there's git history for the file ;) Imre
On Tue, Jul 19, 2011 at 12:07:17PM +0100, Russell King - ARM Linux wrote: > On Tue, Jul 19, 2011 at 12:02:35PM +0100, Russell King - ARM Linux wrote: > > On Sat, Jul 16, 2011 at 04:07:34PM +0200, Imre Kaloz wrote: > > > On Fri, 15 Jul 2011 18:25:19 +0200, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > > > > > >> On Tue, Jul 12, 2011 at 02:56:09PM +0200, Imre Kaloz wrote: > > >>> Wire up support for the XZ decompressor > > >> > > >> Ok. > > > > > > Thanks, added to the patch system. > > > > FYI: > > > > Patching 7001/1... > > <stdin>:56: space before tab in indent. > > $(addprefix $(obj)/, $(OBJS)) $(lib1funcs) $(ashldi3) FORCE > > warning: 1 line applied after fixing whitespace errors. > > BTW, has anyone outside of the ARM community seen the patch to > lib/xz/xz_dec_stream.c adding the linux/kernel.h include? How > well tested is that change on other architectures? Ok, no reply to this, so I'm dropping your patch for the merge window.
On Fri, 22 Jul 2011 11:16:04 +0200, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Tue, Jul 19, 2011 at 12:07:17PM +0100, Russell King - ARM Linux wrote: >> On Tue, Jul 19, 2011 at 12:02:35PM +0100, Russell King - ARM Linux wrote: >> > On Sat, Jul 16, 2011 at 04:07:34PM +0200, Imre Kaloz wrote: >> > > On Fri, 15 Jul 2011 18:25:19 +0200, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: >> > > >> > >> On Tue, Jul 12, 2011 at 02:56:09PM +0200, Imre Kaloz wrote: >> > >>> Wire up support for the XZ decompressor >> > >> >> > >> Ok. >> > > >> > > Thanks, added to the patch system. >> > >> > FYI: >> > >> > Patching 7001/1... >> > <stdin>:56: space before tab in indent. >> > $(addprefix $(obj)/, $(OBJS)) $(lib1funcs) $(ashldi3) FORCE >> > warning: 1 line applied after fixing whitespace errors. >> >> BTW, has anyone outside of the ARM community seen the patch to >> lib/xz/xz_dec_stream.c adding the linux/kernel.h include? How >> well tested is that change on other architectures? > > Ok, no reply to this, so I'm dropping your patch for the merge window. > Guessed I'm not the one outside the ARM community, so.. I can say it didn't break neither ppc or mips for me, but have no idea who's feedback are you waiting for. Imre
On Fri, Jul 22, 2011 at 11:52:19AM +0200, Imre Kaloz wrote: > On Fri, 22 Jul 2011 11:16:04 +0200, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > >> On Tue, Jul 19, 2011 at 12:07:17PM +0100, Russell King - ARM Linux wrote: >>> On Tue, Jul 19, 2011 at 12:02:35PM +0100, Russell King - ARM Linux wrote: >>> > On Sat, Jul 16, 2011 at 04:07:34PM +0200, Imre Kaloz wrote: >>> > > On Fri, 15 Jul 2011 18:25:19 +0200, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: >>> > > >>> > >> On Tue, Jul 12, 2011 at 02:56:09PM +0200, Imre Kaloz wrote: >>> > >>> Wire up support for the XZ decompressor >>> > >> >>> > >> Ok. >>> > > >>> > > Thanks, added to the patch system. >>> > >>> > FYI: >>> > >>> > Patching 7001/1... >>> > <stdin>:56: space before tab in indent. >>> > $(addprefix $(obj)/, $(OBJS)) $(lib1funcs) $(ashldi3) FORCE >>> > warning: 1 line applied after fixing whitespace errors. >>> >>> BTW, has anyone outside of the ARM community seen the patch to >>> lib/xz/xz_dec_stream.c adding the linux/kernel.h include? How >>> well tested is that change on other architectures? >> >> Ok, no reply to this, so I'm dropping your patch for the merge window. >> > > Guessed I'm not the one outside the ARM community, so.. I can say it > didn't break neither ppc or mips for me, but have no idea who's feedback > are you waiting for. What I'm concerned about is the apparant lack of exposure of the patch to people outside of ARM, especially as it touches lib/xz/xz_dec_stream.c. I have no idea whether that breaks some architecture or not. Is Lasse Collin (the original contributer of that file) aware of this change? What is the additional include fixing on ARM? Why isn't this include needed on other architectures? And why isn't any of this mentioned in the patch commit log?
On Fri, Jul 22, 2011 at 10:57:38AM +0100, Russell King - ARM Linux wrote: > What I'm concerned about is the apparant lack of exposure of the patch > to people outside of ARM, especially as it touches lib/xz/xz_dec_stream.c. > > I have no idea whether that breaks some architecture or not. > > Is Lasse Collin (the original contributer of that file) aware of this > change? I forwarded it to Lasse, and got some comments back... essentially putting includes in xz_dec_stream.c could be problematical. Lasse would - as I did - like to have the full information on why the include was necessary. Lasse is - again like me - concerned about dragging stuff into the pre-boot environment which can cause it to fail unexpectedly, especially as linux/kernel.h includes a whole raft of other includes. What I don't understand is if this code requires min_t, where does it get this for everyone else - that's something to be discussed in the wider discussion over this. Does it build outside of the decompressor on ARM already? So... given the increasing number of questions over this, I think you need to put something together which describes the problem, how its caused, why it seems to only exhibit on ARM, and discuss solutions with all those involved. I know Lasse is very interested to find out more information about your patch... But in the mean time, this isn't a patch for 3.1.
On Fri, 22 Jul 2011 14:50:36 +0200, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Fri, Jul 22, 2011 at 10:57:38AM +0100, Russell King - ARM Linux wrote: >> What I'm concerned about is the apparant lack of exposure of the patch >> to people outside of ARM, especially as it touches lib/xz/xz_dec_stream.c. >> >> I have no idea whether that breaks some architecture or not. >> >> Is Lasse Collin (the original contributer of that file) aware of this >> change? > > I forwarded it to Lasse, and got some comments back... essentially > putting includes in xz_dec_stream.c could be problematical. Lasse > would - as I did - like to have the full information on why the > include was necessary. > > Lasse is - again like me - concerned about dragging stuff into the > pre-boot environment which can cause it to fail unexpectedly, > especially as linux/kernel.h includes a whole raft of other includes. > > What I don't understand is if this code requires min_t, where does it > get this for everyone else - that's something to be discussed in the > wider discussion over this. Does it build outside of the decompressor > on ARM already? > > So... given the increasing number of questions over this, I think you > need to put something together which describes the problem, how its > caused, why it seems to only exhibit on ARM, and discuss solutions > with all those involved. I know Lasse is very interested to find out > more information about your patch... > > But in the mean time, this isn't a patch for 3.1. > I've already proposed the right way to fix it in http://lists.arm.linux.org.uk/lurker/message/20110722.102718.6e9fe66c.en.html Imre
On Fri, Jul 22, 2011 at 09:57:09PM +0200, Imre Kaloz wrote:
> I've already proposed the right way to fix it in http://lists.arm.linux.org.uk/lurker/message/20110722.102718.6e9fe66c.en.html
I think you're missing the point.
So, as I've already said I've dropped your original patch, and I'm now
talking directly to Lasse about how best to resolve this (which IMHO is
what should've already happened.) If Lasse doesn't want to fix it in his
code then we'll see about modifying ARM specific stuff.
As you've already pointed out, the code uses min_t, but doesn't contain
an include of linux/kernel.h - so maybe its also broken on other arches.
Until it's discussed, we won't know.
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 9adc278..286873e 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -20,6 +20,7 @@ config ARM select HAVE_KERNEL_GZIP select HAVE_KERNEL_LZO select HAVE_KERNEL_LZMA + select HAVE_KERNEL_XZ select HAVE_IRQ_WORK select HAVE_PERF_EVENTS select PERF_USE_VMALLOC diff --git a/arch/arm/boot/compressed/.gitignore b/arch/arm/boot/compressed/.gitignore index c602896..593a712 100644 --- a/arch/arm/boot/compressed/.gitignore +++ b/arch/arm/boot/compressed/.gitignore @@ -3,5 +3,6 @@ lib1funcs.S piggy.gzip piggy.lzo piggy.lzma +piggy.xzkern vmlinux vmlinux.lds diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile index 23aad07..e5db34e 100644 --- a/arch/arm/boot/compressed/Makefile +++ b/arch/arm/boot/compressed/Makefile @@ -82,13 +82,14 @@ SEDFLAGS = s/TEXT_START/$(ZTEXTADDR)/;s/BSS_START/$(ZBSSADDR)/ suffix_$(CONFIG_KERNEL_GZIP) = gzip suffix_$(CONFIG_KERNEL_LZO) = lzo suffix_$(CONFIG_KERNEL_LZMA) = lzma +suffix_$(CONFIG_KERNEL_XZ) = xzkern targets := vmlinux vmlinux.lds \ piggy.$(suffix_y) piggy.$(suffix_y).o \ font.o font.c head.o misc.o $(OBJS) # Make sure files are removed during clean -extra-y += piggy.gzip piggy.lzo piggy.lzma lib1funcs.S +extra-y += piggy.gzip piggy.lzo piggy.lzma piggy.xzkern lib1funcs.S ashldi3.S ifeq ($(CONFIG_FUNCTION_TRACER),y) ORIG_CFLAGS := $(KBUILD_CFLAGS) @@ -133,8 +134,14 @@ bad_syms=$$($(CROSS_COMPILE)nm $@ | sed -n 's/^.\{8\} [bc] \(.*\)/\1/p') && \ ( echo "following symbols must have non local/private scope:" >&2; \ echo "$$bad_syms" >&2; rm -f $@; false ) +# For __aeabi_llsl +ashldi3 = $(obj)/ashldi3.o + +$(obj)/ashldi3.S: $(srctree)/arch/$(SRCARCH)/lib/ashldi3.S FORCE + $(call cmd,shipped) + $(obj)/vmlinux: $(obj)/vmlinux.lds $(obj)/$(HEAD) $(obj)/piggy.$(suffix_y).o \ - $(addprefix $(obj)/, $(OBJS)) $(lib1funcs) FORCE + $(addprefix $(obj)/, $(OBJS)) $(lib1funcs) $(ashldi3) FORCE $(call if_changed,ld) @$(check_for_bad_syms) diff --git a/arch/arm/boot/compressed/decompress.c b/arch/arm/boot/compressed/decompress.c index 07be5a2..0ecd8b4 100644 --- a/arch/arm/boot/compressed/decompress.c +++ b/arch/arm/boot/compressed/decompress.c @@ -44,6 +44,10 @@ extern void error(char *); #include "../../../../lib/decompress_unlzma.c" #endif +#ifdef CONFIG_KERNEL_XZ +#include "../../../../lib/decompress_unxz.c" +#endif + int do_decompress(u8 *input, int len, u8 *output, void (*error)(char *x)) { return decompress(input, len, NULL, NULL, output, NULL, error); diff --git a/arch/arm/boot/compressed/piggy.xzkern.S b/arch/arm/boot/compressed/piggy.xzkern.S new file mode 100644 index 0000000..5703f30 --- /dev/null +++ b/arch/arm/boot/compressed/piggy.xzkern.S @@ -0,0 +1,6 @@ + .section .piggydata,#alloc + .globl input_data +input_data: + .incbin "arch/arm/boot/compressed/piggy.xzkern" + .globl input_data_end +input_data_end: diff --git a/lib/xz/xz_dec_stream.c b/lib/xz/xz_dec_stream.c index ac809b1..9a60cc2 100644 --- a/lib/xz/xz_dec_stream.c +++ b/lib/xz/xz_dec_stream.c @@ -9,6 +9,7 @@ #include "xz_private.h" #include "xz_stream.h" +#include <linux/kernel.h> /* Hash used to validate the Index field */ struct xz_dec_hash {
Wire up support for the XZ decompressor Signed-off-by: Imre Kaloz <kaloz@openwrt.org> --- Change since v1: added missing .gitignore modification arch/arm/Kconfig | 1 + arch/arm/boot/compressed/.gitignore | 1 + arch/arm/boot/compressed/Makefile | 11 +++++++++-- arch/arm/boot/compressed/decompress.c | 4 ++++ arch/arm/boot/compressed/piggy.xzkern.S | 6 ++++++ lib/xz/xz_dec_stream.c | 1 + 6 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 arch/arm/boot/compressed/piggy.xzkern.S