[Skiboot] [PATCH 04/10] core: Add generic XZ decoding function

[Samuel Mendoza-Jonas]

On Fri, 2017-06-09 at 16:55 +1000, Matt Brown wrote:
> This adds a simple function to decode any given XZ data area.
> The uncomp_len value is optional. Unfortunately we cannot calculate the
> uncompressed size of the XZ region before decompressing it.
> We can pass in the value if this is known, otherwise we allocate a region
> 10x the original size. Which may be significantly more than necessary.
> 
> Signed-off-by: Matt Brown <matthew.brown.dev at gmail.com>
> ---
>  core/Makefile.inc  |   2 +-
>  core/decode-xz.c   | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/platform.h |   2 +
>  3 files changed, 115 insertions(+), 1 deletion(-)
>  create mode 100644 core/decode-xz.c
> 
> diff --git a/core/Makefile.inc b/core/Makefile.inc
> index b09c30c..7f376bd 100644
> --- a/core/Makefile.inc
> +++ b/core/Makefile.inc
> @@ -8,7 +8,7 @@ CORE_OBJS += pci-opal.o fast-reboot.o device.o exceptions.o trace.o affinity.o
>  CORE_OBJS += vpd.o hostservices.o platform.o nvram.o nvram-format.o hmi.o
>  CORE_OBJS += console-log.o ipmi.o time-utils.o pel.o pool.o errorlog.o
>  CORE_OBJS += timer.o i2c.o rtc.o flash.o sensor.o ipmi-opal.o
> -CORE_OBJS += flash-subpartition.o bitmap.o buddy.o pci-quirk.o
> +CORE_OBJS += flash-subpartition.o bitmap.o buddy.o pci-quirk.o decode-xz.o
>  
>  ifeq ($(SKIBOOT_GCOV),1)
>  CORE_OBJS += gcov-profiling.o
> diff --git a/core/decode-xz.c b/core/decode-xz.c
> new file mode 100644
> index 0000000..4855e9b
> --- /dev/null
> +++ b/core/decode-xz.c
> @@ -0,0 +1,112 @@
> +/* Copyright 2017 IBM Corp.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at
> + *
> + * 	http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> + * implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <skiboot.h>
> +#include <platform.h>
> +#include <libxz/xz.h>
> +
> +
> +#define HEADER_MAGIC "\3757zXZ"
> +#define HEADER_MAGIC_SIZE 6
> +/*
> + * decode_resource_xz
> + * Decodes any xz compressed memory region.
> + *
> + * @buf : pointer to memory region pointer
> + * @len : pointer to length of compressed region
> + * @uncomp_len: pointer to uncompressed length (optional)
> + *
> + * Returns 1 - successful or 0 - failed 
> + */
> +int decode_resource_xz(void **buf, size_t *len, size_t *uncomp_len)

If decode_resource_xz is used in patch 1 ("Decode payload initrd on FSP
machines"), this patch should come before it.

> +{
> +	struct xz_dec *file;
> +	struct xz_buf bufs;
> +	char *input_header;
> +	uint64_t *out_len;
> +	enum xz_ret rc;
> +	
> +	uint8_t *r_buf = (uint8_t *)(*buf);
> +	size_t   r_len = *len;
> +
> +	/* Check if input header matched XZ encoding signature */
> +	input_header = malloc(HEADER_MAGIC_SIZE);
> +	if (!input_header)
> +		return 0;
> +
> +	memcpy(input_header, r_buf, HEADER_MAGIC_SIZE);
> +	if (strcmp(input_header, HEADER_MAGIC)){
> +		prlog(PR_PRINTF, "DECODE: resource header magic does not match\
> +				  xz format\n");
> +		free(input_header);
> +		return 0;
> +	}
> +
> +
> +	/* Set up decoder */
> +	file = xz_dec_init(XZ_SINGLE, 0x100000);
> +
> +	if (!file) {
> +		/* Allocation failed */
> +		prlog(PR_PRINTF, "DECODE: xz_dec_init allocation error\n ");
> +		free(input_header);
> +		return 0;
> +	}
> +
> +	out_len = (uint64_t *)malloc(sizeof(uint64_t));

I think you only use out_len within the function and don't pass the
pointer anywhere - would it be worth just having this instead? One less
thing to free :)

	uint64_t out_len;
	..
	out_len = uncomp_len ? *uncomp_len : (r_len * 10);

> +	if (!uncomp_len)
> +       		*out_len = (r_len) * 10;
> +	else
> +		*out_len = *uncomp_len;
> +
> +	bufs.in       = (const uint8_t *)r_buf;
> +	bufs.in_pos   = 0;
> +	bufs.in_size  = r_len;
> +	bufs.out      = (uint8_t *)local_alloc(0, *out_len, 0x10000);
> +	bufs.out_pos  = 0;
> +	bufs.out_size = *(size_t *)out_len;

I don't think you need these casts (or for eg. r_buf, could declare it as
const to begin with).

> +
> +	if (bufs.out == NULL) {
> +		/* Buffer allocation failed */
> +		prlog(PR_PRINTF, "DECODE: bufs.out allocation error\n ");
> +		free(out_len);
> +		free(input_header);
> +		return 0;
> +	}
> +
> +	rc = xz_dec_run(file, &bufs);
> +
> +	if (rc != XZ_STREAM_END) {
> +		prlog(PR_ALERT, "DECODE: XZ decompression failed rc:%d\n", rc);
> +		free(bufs.out);
> +		free(out_len);
> +		free(input_header);
> +		return 0;
> +	}
> +
> +	/* Redefine resource base and size */
> +	*buf = (void *)bufs.out;
> +	*len = (size_t)*out_len;
> +
> +	prlog(PR_PRINTF, "DECODE: decode_resource_xz base: %p, len: %llu \
> +			remote: %p\n", bufs.out, *out_len, *buf);
> +
> +	xz_dec_end(file);
> +	free(out_len);
> +	free(input_header);
> +
> +	return 1;
> +}
> diff --git a/include/platform.h b/include/platform.h
> index e8848ca..0c41c35 100644
> --- a/include/platform.h
> +++ b/include/platform.h
> @@ -234,4 +234,6 @@ extern void set_bmc_platform(const struct bmc_platform *bmc);
>  
>  extern struct cpu_job* start_async_load(struct boot_resources *r);
>  
> +/* XZ decoding*/
> +extern int decode_resource_xz(void **buf, size_t *len, size_t *uncomp_len);
>  #endif /* __PLATFORM_H */