On Wed, 12 Aug 2015, Ian Campbell wrote:
> On Wed, 2015-08-12 at 15:47 +0100, Stefano Stabellini wrote:
> > Signed-off-by: Stefano Stabellini <stefano.stabell...@eu.citrix.com>
> > CC: julien.gr...@citrix.com
> > CC: ian.campb...@citrix.com
> > ---
> > xen/arch/arm/kernel.c | 36
> > ++++++++++++++++++++++++++++++++++++
> > xen/common/Makefile | 2 +-
> > xen/include/asm-arm/byteorder.h | 2 ++
> > 3 files changed, 39 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> > index f641b12..ca50cdd 100644
> > --- a/xen/arch/arm/kernel.c
> > +++ b/xen/arch/arm/kernel.c
> > @@ -13,6 +13,8 @@
> > #include <asm/byteorder.h>
> > #include <asm/setup.h>
> > #include <xen/libfdt/libfdt.h>
> > +#include <xen/decompress.h>
> > +#include <xen/vmap.h>
> >
> > #include "kernel.h"
> >
> > @@ -310,6 +312,38 @@ static int kernel_zimage64_probe(struct kernel_info
> > *info,
> >
> > return 0;
> > }
> > +
> > +static int kernel_zimage64_compressed_probe(struct kernel_info *info,
> > + paddr_t addr, paddr_t size)
> > +{
> > + char *output, *input;
> > + unsigned char magic[2];
> > + int rc;
> > + unsigned kernel_order_in;
> > + unsigned kernel_order_out;
> > + paddr_t output_size;
> > +
> > + copy_from_paddr(magic, addr, sizeof(magic));
> > +
> > + if (!((magic[0] == 0x1f) && ((magic[1] == 0x8b) || (magic[1] ==
> > 0x9e))))
> > + return -EINVAL;
>
> This is an open coded check_gzip. I think you could call that function on
> magic?
>
> > +
> > + kernel_order_in = get_order_from_bytes(size);
> > + input = (char *)ioremap_cache(addr, size);
>
> I don't think you need to cast this, do you? It's a void * already.
>
> > +
> > + output_size = output_length(input, size);
> > + kernel_order_out = get_order_from_bytes(output_size);
> > + output = (char *)alloc_xenheap_pages(kernel_order_out, 0);
>
> Likewise.
>
> Where is this buffer freed?
>
> When I said IRL we recover the kernel memory I meant the thing in the boot
> modules list. You might be able to get away with flipping the boot module
> over to this, but that would have ordering constraints which I didn't
> check, it'll probably get subtle fast.
>
> > +
> > + rc = decompress(input, size, output);
> > + clean_dcache_va_range(output, output_size);
> > + iounmap(input);
> > +
> > + if (rc != 0)
> > + return rc;
> > +
> > + return kernel_zimage64_probe(info, virt_to_maddr(output), output_size);
>
>
>
> > +}
> > #endif
> >
> > /*
> > @@ -466,6 +500,8 @@ int kernel_probe(struct kernel_info *info)
> > #ifdef CONFIG_ARM_64
> > rc = kernel_zimage64_probe(info, start, size);
> > if (rc < 0)
> > + rc = kernel_zimage64_compressed_probe(info, start, size);
>
> I don't see a reason not to support compressed 32 bit kernels too. All it
> would take would be to try and uncompress the buffer first before falling
> through to the various probe routines, instead of chaining a probe into the
> decompressor.
>
> Probably the easiest way to solve this and the buffer allocation issue
> above would be to always either copy or decompress the original kernel into
> a buffer and then change all the probe function to use a virtual address
> instead of an maddr (which might have tricky cache interactions since the
> mapping still exists).
I'll give it a look
> > + if (rc < 0)
> > #endif
> > rc = kernel_uimage_probe(info, start, size);
> > if (rc < 0)
> > diff --git a/xen/common/Makefile b/xen/common/Makefile
> > index 0a4d4fa..a8aefc6 100644
> > --- a/xen/common/Makefile
> > +++ b/xen/common/Makefile
> > @@ -56,7 +56,7 @@ obj-y += vsprintf.o
> > obj-y += wait.o
> > obj-y += xmalloc_tlsf.o
> >
> > -obj-bin-$(CONFIG_X86) += $(foreach n,decompress gunzip bunzip2 unxz unlzma
> > unlzo unlz4 earlycpio,$(n).init.o)
> > +obj-bin-y += $(foreach n,decompress gunzip bunzip2 unxz unlzma unlzo unlz4
> > earlycpio,$(n).init.o)
>
> I don't think we need/want earlycpio support on ARM (not yet anyway).
Unfortunately it is not possible to only compile some and not all
because decompress.c makes use of them all.
> > obj-$(perfc) += perfc.o
> > obj-$(crash_debug) += gdbstub.o
> > diff --git a/xen/include/asm-arm/byteorder.h b/xen/include/asm
> > -arm/byteorder.h
> > index 9c712c4..3b7feda 100644
> > --- a/xen/include/asm-arm/byteorder.h
> > +++ b/xen/include/asm-arm/byteorder.h
> > @@ -5,6 +5,8 @@
> >
> > #include <xen/byteorder/little_endian.h>
> >
> > +#define CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
>
> While CONFIG_HAVE_UNALIGNED_ACCESS might be true on arm64 it may not be the
> case that it is efficient. Also I'm not sure about arm32 at all.
It is true on arm archs >= v8, so we should be fine. See:
http://lwn.net/Articles/540022/
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
