Re: [Xen-devel] [PATCH v6 2/2] xen/arm: support gzip compressed kernels

2015-09-22 Thread Stefano Stabellini
On Tue, 22 Sep 2015, Julien Grall wrote:
> On 21/09/2015 23:51, Stefano Stabellini wrote:
> > +/*
> > + * Need to free pages after output_size here because they won't be
> > + * freed by discard_initial_modules
> > + */
> > +i = (output_size + PAGE_SIZE - 1) >> PAGE_SHIFT;
> > +for ( ; i < (1 << kernel_order_out); i++ )
> > +free_domheap_page(pages + i);
> > +
> > +vunmap(output);
> > +
> 
> I forgot to mention that vunmap should be called before free_domheap_pages to
> avoid mapping on unallocated pages.
 
You are right, thanks

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v6 2/2] xen/arm: support gzip compressed kernels

2015-09-22 Thread Julien Grall



On 21/09/2015 23:51, Stefano Stabellini wrote:

+/*
+ * Need to free pages after output_size here because they won't be
+ * freed by discard_initial_modules
+ */
+i = (output_size + PAGE_SIZE - 1) >> PAGE_SHIFT;
+for ( ; i < (1 << kernel_order_out); i++ )
+free_domheap_page(pages + i);
+
+vunmap(output);
+


I forgot to mention that vunmap should be called before 
free_domheap_pages to avoid mapping on unallocated pages.


Regards,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v6 2/2] xen/arm: support gzip compressed kernels

2015-09-22 Thread Julien Grall

Hi Stefano,

On 22/09/2015 02:35, Stefano Stabellini wrote:

You may want to use vmap here which is a wrapper to __vmap and will setup the
granularity, align, and flags automatically for you.


The arguments are different.


Yes, sorry I inverted nr with granularity.


I meant to say that it makes more sense to pass one region of higher 
granularity,
than to map pages one by one.


Well, the page will still be mapped one by one in map_pages_to_xen one 
by one because we don't support super-page mapping.


TBH if you care about performance, you would only map the strict 
necessary because the smallest granularity to fit all the kernel may be 
big compare to the real size.


For instance, if the decompress size is 65M, you would have to map 128M.

Anyway, it's only used during boot, so I don't much care if you use 
__vmap. My suggestion was only for simplifying the code.


Regards,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v6 2/2] xen/arm: support gzip compressed kernels

2015-09-21 Thread Stefano Stabellini
On Tue, 22 Sep 2015, Stefano Stabellini wrote:
> On Tue, 22 Sep 2015, Julien Grall wrote:
> > Hi Stefano,
> > 
> > On 21/09/2015 23:51, Stefano Stabellini wrote:
> > > Changes in v6:
> > > - use vmap to map pages
> > > - free old module and update mod->start and mod->size from
> > > kernel_decompress
> > 
> > I was expecting you to drop my Reviewed-by given those changes.
> 
> fair enough
> 
> 
> > > diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> > > index f641b12..ad7e6bc 100644
> > > --- a/xen/arch/arm/kernel.c
> > > +++ b/xen/arch/arm/kernel.c
> > 
> > [...]
> > 
> > 
> > > +static __init uint32_t output_length(char *image, unsigned long 
> > > image_len)
> > > +{
> > > +return *(uint32_t *)&image[image_len - 4];
> > > +}
> > > +
> > > +static __init int kernel_decompress(struct bootmodule *mod)
> > > +{
> > 
> > [...]
> > 
> > > +mfn = _mfn(page_to_mfn(pages));
> > > +output = __vmap(&mfn, 1 << kernel_order_out, 1, 1, PAGE_HYPERVISOR);
> > 
> > You may want to use vmap here which is a wrapper to __vmap and will setup 
> > the
> > granularity, align, and flags automatically for you.
>  
> The arguments are different.

I meant to say that it makes more sense to pass one region of higher 
granularity,
than to map pages one by one.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v6 2/2] xen/arm: support gzip compressed kernels

2015-09-21 Thread Stefano Stabellini
On Tue, 22 Sep 2015, Julien Grall wrote:
> Hi Stefano,
> 
> On 21/09/2015 23:51, Stefano Stabellini wrote:
> > Changes in v6:
> > - use vmap to map pages
> > - free old module and update mod->start and mod->size from
> > kernel_decompress
> 
> I was expecting you to drop my Reviewed-by given those changes.

fair enough


> > diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> > index f641b12..ad7e6bc 100644
> > --- a/xen/arch/arm/kernel.c
> > +++ b/xen/arch/arm/kernel.c
> 
> [...]
> 
> 
> > +static __init uint32_t output_length(char *image, unsigned long image_len)
> > +{
> > +return *(uint32_t *)&image[image_len - 4];
> > +}
> > +
> > +static __init int kernel_decompress(struct bootmodule *mod)
> > +{
> 
> [...]
> 
> > +mfn = _mfn(page_to_mfn(pages));
> > +output = __vmap(&mfn, 1 << kernel_order_out, 1, 1, PAGE_HYPERVISOR);
> 
> You may want to use vmap here which is a wrapper to __vmap and will setup the
> granularity, align, and flags automatically for you.
 
The arguments are different.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v6 2/2] xen/arm: support gzip compressed kernels

2015-09-21 Thread Julien Grall

Hi Stefano,

On 21/09/2015 23:51, Stefano Stabellini wrote:

Changes in v6:
- use vmap to map pages
- free old module and update mod->start and mod->size from
kernel_decompress


I was expecting you to drop my Reviewed-by given those changes.


diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index f641b12..ad7e6bc 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c


[...]



+static __init uint32_t output_length(char *image, unsigned long image_len)
+{
+return *(uint32_t *)&image[image_len - 4];
+}
+
+static __init int kernel_decompress(struct bootmodule *mod)
+{


[...]


+mfn = _mfn(page_to_mfn(pages));
+output = __vmap(&mfn, 1 << kernel_order_out, 1, 1, PAGE_HYPERVISOR);


You may want to use vmap here which is a wrapper to __vmap and will 
setup the granularity, align, and flags automatically for you.


Regards,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


[Xen-devel] [PATCH v6 2/2] xen/arm: support gzip compressed kernels

2015-09-21 Thread Stefano Stabellini
Free the memory used for the compressed kernel and update the relative
mod->start and mod->size parameters with the uncompressed ones.

To decompress the kernel, allocate memory from dommheap, because freeing
the modules is done by calling init_heap_pages, which frees to domheap.
Map these pages using vmap, because they might not be in the linear 1:1
map.

Signed-off-by: Stefano Stabellini 
Reviewed-by: Julien Grall 
CC: ian.campb...@citrix.com

---

Changes in v6:
- use vmap to map pages
- free old module and update mod->start and mod->size from
kernel_decompress

Changes in v5:
- code style

Changes in v4:
- return uint32_t from output_length
- __init kernel_decompress
- code style
- add comment
- if kernel_decompress fails with error, return

Changes in v3:
- better error checks in kernel_decompress
- free unneeded pages between output_size and kernel_order_out
- alloc pages from domheap

Changes in v2:
- use gzip_check
- avoid useless casts
- free original kernel image and update the mod with the new address and
size
- remove changes to common Makefile
- remove CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
---
 xen/arch/arm/kernel.c   |   89 ++-
 xen/arch/arm/setup.c|2 +-
 xen/include/asm-arm/setup.h |2 +
 3 files changed, 83 insertions(+), 10 deletions(-)

diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index f641b12..ad7e6bc 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -13,6 +13,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #include "kernel.h"
 
@@ -257,6 +259,74 @@ static int kernel_uimage_probe(struct kernel_info *info,
 return 0;
 }
 
+static __init uint32_t output_length(char *image, unsigned long image_len)
+{
+return *(uint32_t *)&image[image_len - 4];
+}
+
+static __init int kernel_decompress(struct bootmodule *mod)
+{
+char *output, *input;
+char magic[2];
+int rc;
+unsigned kernel_order_out;
+paddr_t output_size;
+struct page_info *pages;
+mfn_t mfn;
+int i;
+paddr_t addr = mod->start;
+paddr_t size = mod->size;
+
+if ( size < 2 )
+return -EINVAL;
+
+copy_from_paddr(magic, addr, sizeof(magic));
+
+/* only gzip is supported */
+if ( !gzip_check(magic, size) )
+return -EINVAL;
+
+input = ioremap_cache(addr, size);
+if ( input == NULL )
+return -EFAULT;
+
+output_size = output_length(input, size);
+kernel_order_out = get_order_from_bytes(output_size);
+pages = alloc_domheap_pages(NULL, kernel_order_out, 0);
+if ( pages == NULL )
+{
+iounmap(input);
+return -ENOMEM;
+}
+mfn = _mfn(page_to_mfn(pages));
+output = __vmap(&mfn, 1 << kernel_order_out, 1, 1, PAGE_HYPERVISOR);
+
+rc = perform_gunzip(output, input, size);
+clean_dcache_va_range(output, output_size);
+iounmap(input);
+
+/* 
+ * Need to free pages after output_size here because they won't be
+ * freed by discard_initial_modules
+ */
+i = (output_size + PAGE_SIZE - 1) >> PAGE_SHIFT;
+for ( ; i < (1 << kernel_order_out); i++ )
+free_domheap_page(pages + i);
+
+vunmap(output);
+
+/* 
+ * Free the original kernel, update the pointers to the
+ * decompressed kernel
+ */
+dt_unreserved_regions(mod->start, mod->start + mod->size,
+  init_domheap_pages, 0);
+mod->start = page_to_maddr(pages);
+mod->size = output_size;
+
+return 0;
+}
+
 #ifdef CONFIG_ARM_64
 /*
  * Check if the image is a 64-bit Image.
@@ -444,8 +514,6 @@ int kernel_probe(struct kernel_info *info)
 struct bootmodule *mod = boot_module_find_by_kind(BOOTMOD_KERNEL);
 int rc;
 
-paddr_t start, size;
-
 if ( !mod || !mod->size )
 {
 printk(XENLOG_ERR "Missing kernel boot module?\n");
@@ -453,25 +521,28 @@ int kernel_probe(struct kernel_info *info)
 }
 
 info->kernel_bootmodule = mod;
-start = mod->start;
-size = mod->size;
 
-printk("Loading kernel from boot module @ %"PRIpaddr"\n", start);
+printk("Loading kernel from boot module @ %"PRIpaddr"\n", mod->start);
 
 info->initrd_bootmodule = boot_module_find_by_kind(BOOTMOD_RAMDISK);
 if ( info->initrd_bootmodule )
 printk("Loading ramdisk from boot module @ %"PRIpaddr"\n",
info->initrd_bootmodule->start);
 
+/* if it is a gzip'ed image, 32bit or 64bit, uncompress it */
+rc = kernel_decompress(mod);
+if (rc < 0 && rc != -EINVAL)
+return rc;
+
 #ifdef CONFIG_ARM_64
-rc = kernel_zimage64_probe(info, start, size);
+rc = kernel_zimage64_probe(info, mod->start, mod->size);
 if (rc < 0)
 #endif
-rc = kernel_uimage_probe(info, start, size);
+rc = kernel_uimage_probe(info, mod->start, mod->size);
 if (rc < 0)
-rc = kernel_zimage32_probe(info, start, size);
+rc = kernel_zimage32_probe(info, mod->start, mod->size);
 if (rc < 0)
-rc = kernel_