diff mbox series

[2/2] imc/catalog: Decompress catalog asynchronously

Message ID 20190118035213.18902-2-santosh@fossix.org
State Accepted
Headers show
Series [1/2] flash: Add support for async decompression | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success master/apply_patch Successfully applied
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot success Test snowpatch/job/snowpatch-skiboot on branch master

Commit Message

Santosh Sivaraj Jan. 18, 2019, 3:52 a.m. UTC
In-Memory Collection(IMC) counters catalog is compressed blob which is
loaded from the flash; decompression starts once the data is loaded from
nvram by the main thread. This can be optimized by using the libxz API
function which creates a job to do the decompression by not blocking the
main thread.

Refactor decompress() to use the libxz asynchronous wrapper
functions. This also cleans up the error handling path in imc_init().

CC: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
---
 core/init.c   |   3 ++
 hw/imc.c      | 141 ++++++++++++++++++++------------------------------
 include/imc.h |   1 +
 3 files changed, 61 insertions(+), 84 deletions(-)

Comments

Stewart Smith Feb. 26, 2019, 6:07 a.m. UTC | #1
Santosh Sivaraj <santosh@fossix.org> writes:

> In-Memory Collection(IMC) counters catalog is compressed blob which is
> loaded from the flash; decompression starts once the data is loaded from
> nvram by the main thread. This can be optimized by using the libxz API
> function which creates a job to do the decompression by not blocking the
> main thread.
>
> Refactor decompress() to use the libxz asynchronous wrapper
> functions. This also cleans up the error handling path in imc_init().
>
> CC: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
> ---
>  core/init.c   |   3 ++
>  hw/imc.c      | 141 ++++++++++++++++++++------------------------------
>  include/imc.h |   1 +
>  3 files changed, 61 insertions(+), 84 deletions(-)
>
> diff --git a/core/init.c b/core/init.c
> index 262ac5fb..adf515f8 100644
> --- a/core/init.c
> +++ b/core/init.c
> @@ -1153,6 +1153,9 @@ void __noreturn __nomcount main_cpu_entry(const void *fdt)
>  	preload_capp_ucode();
>  	start_preload_kernel();
>  
> +	/* Catalog decompression routine */
> +	imc_decompress_catalog();

My nitpick with how this solution works is that we end up having a
synchronisation point here where we have to wait for the IMA_CATALOG
partition to be loaded before we continue with our init process.

I had the idea perhaps we could link into some of the lower routines and
do things there, but everything looks to be a bit more annoying and
invasive.

So, I've taken this series as is for now and we can clean things up a
bit in the future.

Merged to master as of c86fb12c07a6dc7d878503c1d8ff2de705bf61d7
diff mbox series

Patch

diff --git a/core/init.c b/core/init.c
index 262ac5fb..adf515f8 100644
--- a/core/init.c
+++ b/core/init.c
@@ -1153,6 +1153,9 @@  void __noreturn __nomcount main_cpu_entry(const void *fdt)
 	preload_capp_ucode();
 	start_preload_kernel();
 
+	/* Catalog decompression routine */
+	imc_decompress_catalog();
+
 	/* Virtual Accelerator Switchboard */
 	vas_init();
 
diff --git a/hw/imc.c b/hw/imc.c
index 3392eaf1..586fa8f9 100644
--- a/hw/imc.c
+++ b/hw/imc.c
@@ -180,58 +180,6 @@  static int pause_microcode_at_boot(void)
 	return 0;
 }
 
-/*
- * Decompresses the blob obtained from the IMC pnor sub-partition
- * in "src" of size "src_size", assigns the uncompressed device tree
- * binary to "dst" and returns.
- *
- * Returns 0 on success and -1 on error.
- *
- * TODO: Ideally this should be part of generic subpartition load
- * infrastructure. And decompression can be queued as another CPU job
- */
-static int decompress(void *dst, size_t dst_size, void *src, size_t src_size)
-{
-	struct xz_dec *s;
-	struct xz_buf b;
-	int ret = 0;
-
-	/* Initialize the xz library first */
-	xz_crc32_init();
-	s = xz_dec_init(XZ_SINGLE, 0);
-	if (s == NULL) {
-		prerror("initialization error for xz\n");
-		return -1;
-	}
-
-	/*
-	 * Source address : src
-	 * Source size : src_size
-	 * Destination address : dst
-	 * Destination size : dst_src
-	 */
-	b.in = src;
-	b.in_pos = 0;
-	b.in_size = src_size;
-	b.out = dst;
-	b.out_pos = 0;
-	b.out_size = dst_size;
-
-	/* Start decompressing */
-	ret = xz_dec_run(s, &b);
-	if (ret != XZ_STREAM_END) {
-		prerror("failed to decompress subpartition\n");
-		ret = -1;
-		goto err;
-	}
-
-	return 0;
-err:
-	/* Clean up memory */
-	xz_dec_end(s);
-	return ret;
-}
-
 /*
  * Function return list of properties names for the fixup
  */
@@ -504,6 +452,50 @@  static void imc_dt_update_nest_node(struct dt_node *dev)
 	}
 }
 
+static struct xz_decompress *imc_xz;
+
+void imc_decompress_catalog(void)
+{
+	void *decompress_buf = NULL;
+	uint32_t pvr = (mfspr(SPR_PVR) & ~(0xf0ff));
+	int ret;
+
+	/* Check we succeeded in starting the preload */
+	if (compress_buf == NULL)
+		return;
+
+	ret = wait_for_resource_loaded(RESOURCE_ID_IMA_CATALOG, pvr);
+	if (ret != OPAL_SUCCESS) {
+		prerror("IMC Catalog load failed\n");
+		return;
+	}
+
+	/*
+	 * Memory for decompression.
+	 */
+	decompress_buf = malloc(MAX_DECOMPRESSED_IMC_DTB_SIZE);
+	if (!decompress_buf) {
+		prerror("No memory for decompress_buf \n");
+		return;
+	}
+
+	/*
+	 * Decompress the compressed buffer
+	 */
+	imc_xz = malloc(sizeof(struct xz_decompress));
+	if (!imc_xz) {
+		prerror("No memory to decompress IMC catalog\n");
+		free(decompress_buf);
+		return;
+	}
+
+	imc_xz->dst = decompress_buf;
+	imc_xz->src = compress_buf;
+	imc_xz->dst_size = MAX_DECOMPRESSED_IMC_DTB_SIZE;
+	imc_xz->src_size = compress_buf_size;
+	xz_start_decompress(imc_xz);
+}
+
 /*
  * Load the IMC pnor partition and find the appropriate sub-partition
  * based on the platform's PVR.
@@ -512,10 +504,8 @@  static void imc_dt_update_nest_node(struct dt_node *dev)
  */
 void imc_init(void)
 {
-	void *decompress_buf = NULL;
-	uint32_t pvr = (mfspr(SPR_PVR) & ~(0xf0ff));
 	struct dt_node *dev;
-	int ret;
+	int err_flag = -1;
 
 	if (proc_chip_quirks & QUIRK_MAMBO_CALLOUTS) {
 		dev = dt_find_compatible_node(dt_root, NULL,
@@ -530,15 +520,12 @@  void imc_init(void)
 	if (proc_gen != proc_gen_p9)
 		return;
 
-	/* Check we succeeded in starting the preload */
-	if (compress_buf == NULL)
+	if (!imc_xz)
 		return;
 
-	ret = wait_for_resource_loaded(RESOURCE_ID_IMA_CATALOG, pvr);
-	if (ret != OPAL_SUCCESS) {
-		prerror("IMC Catalog load failed\n");
-		return;
-	}
+	wait_xz_decompress(imc_xz);
+	if (imc_xz->status != OPAL_SUCCESS)
+		goto err;
 
 	/*
 	 * Flow of the data from PNOR to main device tree:
@@ -549,22 +536,6 @@  void imc_init(void)
 	 * free compressed local buffer
 	 */
 
-	/*
-	 * Memory for decompression.
-	 */
-	decompress_buf = malloc(MAX_DECOMPRESSED_IMC_DTB_SIZE);
-	if (!decompress_buf) {
-		prerror("No memory for decompress_buf \n");
-		goto err;
-	}
-
-	/*
-	 * Decompress the compressed buffer
-	 */
-	ret = decompress(decompress_buf, MAX_DECOMPRESSED_IMC_DTB_SIZE,
-				compress_buf, compress_buf_size);
-	if (ret < 0)
-		goto err;
 
 	/* Create a device tree entry for imc counters */
 	dev = dt_new_root("imc-counters");
@@ -575,8 +546,7 @@  void imc_init(void)
 	 * Attach the new decompress_buf to the imc-counters node.
 	 * dt_expand_node() does sanity checks for fdt_header, piggyback
 	 */
-	ret = dt_expand_node(dev, decompress_buf, 0);
-	if (ret < 0) {
+	if (dt_expand_node(dev, imc_xz->dst, 0) < 0) {
 		dt_free(dev);
 		goto err;
 	}
@@ -623,12 +593,15 @@  imc_mambo:
 		goto err;
 	}
 
-	free(compress_buf);
-	return;
+	err_flag = OPAL_SUCCESS;
+
 err:
-	prerror("IMC Devices not added\n");
-	free(decompress_buf);
+	if (err_flag != OPAL_SUCCESS)
+		prerror("IMC Devices not added\n");
+
 	free(compress_buf);
+	free(imc_xz->dst);
+	free(imc_xz);
 }
 
 /*
diff --git a/include/imc.h b/include/imc.h
index f3d906ee..c2616296 100644
--- a/include/imc.h
+++ b/include/imc.h
@@ -130,6 +130,7 @@  struct imc_chip_cb
 
 void imc_init(void);
 void imc_catalog_preload(void);
+void imc_decompress_catalog(void);
 
 #define MAX_NEST_COMBINED_UNITS		4
 struct combined_units_node {