[Cbe-oss-dev] [PATCH] libspe2: Initialise object-id on isolated program load

[Kazunori Asayama]

Jeremy Kerr <jk at ozlabs.org> wrote:
> When performing an isolated program load, we don't update the object-id
> file, used for debugging and profiling. This changes does the write for
> both isolated and non-isolated cases.
> 
> In the case of isolated contexts, we need to defer this loading until we
> have received the appropriate stop code from the loader.

If we save the pointer to an spe_program_handle_t structure specified
via spe_program_load and use it later, I think that we must define the
required lifetime of the spe_program_handle_t structure in the libspe2
spec to guarantee the pointer is valid when it is derefered later (or
we should save only the object ID).

> 
> Signed-off-by: Jeremy Kerr <jk at ozlabs.org>
> 
> ---
> 
>  spebase/create.c  |    1 +
>  spebase/load.c    |   49 +++++++++++++++++++++++++++++++++++--------------
>  spebase/run.c     |    2 +-
>  spebase/spebase.h |   17 +++++++++++++++++
>  4 files changed, 54 insertions(+), 15 deletions(-)
> 
> Index: libspe2/spebase/load.c
> ===================================================================
> --- libspe2.orig/spebase/load.c
> +++ libspe2/spebase/load.c
> @@ -31,6 +31,34 @@
>  #endif
>  
>  /**
> + * Register the SPE program's start address with the oprofile and gdb, by
> + * writing to the object-id file.
> + */
> +void _base_spe_program_load_complete(spe_context_ptr_t spectx)
> +{
> +	int objfd, len;
> +	char buf[20];
> +	spe_program_handle_t *program;
> +
> +	program = spectx->base_private->loaded_program;
> +
> +	if (!program || !program->elf_image) {
> +		DEBUG_PRINTF("%s called, but no program loaded\n", __func__);
> +		return;
> +	}
> +
> +	objfd = openat(spectx->base_private->fd_spe_dir, "object-id", O_RDWR);
> +	if (objfd < 0)
> +		return;
> +
> +	len = sprintf(buf, "%p", program->elf_image);
> +	write(objfd, buf, len + 1);
> +	close(objfd);
> +
> +	__spe_context_update_event();
> +}
> +
> +/**
>   * Send data to a SPU in mbox when space is available.
>   *
>   * Helper function for internal libspe use.
> @@ -160,9 +188,11 @@ static int spe_start_emulated_isolated_a
>  
>  int _base_spe_program_load(spe_context_ptr_t spe, spe_program_handle_t *program)
>  {
> -	int rc = 0, objfd;
> +	int rc = 0;
>  	struct spe_ld_info ld_info;
>  
> +	spe->base_private->loaded_program = program;
> +
>  	if (spe->base_private->flags & SPE_ISOLATE) {
>  		rc = spe_start_isolated_app(spe, program);
>  
> @@ -170,8 +200,10 @@ int _base_spe_program_load(spe_context_p
>  		rc = spe_start_emulated_isolated_app(spe, program, &ld_info);
>  
>  	} else {
> -		rc = _base_spe_load_spe_elf(program, spe->base_private->mem_mmap_base,
> -					    &ld_info);
> +		rc = _base_spe_load_spe_elf(program,
> +				spe->base_private->mem_mmap_base, &ld_info);
> +		if (!rc)
> +			_base_spe_program_load_complete(spe);
>  	}
>  
>  	if (rc != 0) {
> @@ -179,17 +211,6 @@ int _base_spe_program_load(spe_context_p
>  		return -1;
>  	}
>  
> -	/* Register SPE image start address as "object-id" for oprofile.  */
> -	objfd = openat (spe->base_private->fd_spe_dir,"object-id", O_RDWR);
> -	if (objfd >= 0) {
> -		char buf[100];
> -		sprintf (buf, "%p", program->elf_image);
> -		write (objfd, buf, strlen (buf) + 1);
> -		close (objfd);
> -	}
> -	
> -	__spe_context_update_event();	
> -
>  	spe->base_private->entry = ld_info.entry;
>  	spe->base_private->emulated_entry = ld_info.entry;
>  
> Index: libspe2/spebase/create.c
> ===================================================================
> --- libspe2.orig/spebase/create.c
> +++ libspe2/spebase/create.c
> @@ -223,6 +223,7 @@ spe_context_ptr_t _base_spe_context_crea
>  	priv->cntl_mmap_base = MAP_FAILED;
>  	priv->signal1_mmap_base = MAP_FAILED;
>  	priv->signal2_mmap_base = MAP_FAILED;
> +	priv->loaded_program = NULL;
>  
>  	for (i = 0; i < NUM_MBOX_FDS; i++) {
>  		priv->spe_fds_array[i] = -1;
> Index: libspe2/spebase/run.c
> ===================================================================
> --- libspe2.orig/spebase/run.c
> +++ libspe2/spebase/run.c
> @@ -306,7 +306,7 @@ do_run:
>  			 * and restart
>  			 */
>  			if (stopcode == SPE_PROGRAM_ISO_LOAD_COMPLETE) {
> -				__spe_context_update_event();
> +				_base_spe_program_load_complete(spe);
>  				goto do_run;
>  			} else {
>  				stopinfo->stop_reason = SPE_ISOLATION_ERROR;
> Index: libspe2/spebase/spebase.h
> ===================================================================
> --- libspe2.orig/spebase/spebase.h
> +++ libspe2/spebase/spebase.h
> @@ -92,6 +92,12 @@ struct spe_context_base_priv {
>  	/* SPE program entry point generated by elf_load() */
>  	int		entry;
>  
> +	/* pointer to the program loaded/being loaded to the SPE. We need to
> +	 * store this to allow deferred updates to gdb, when loading is
> +	 * asynchronous (ie, isolated load, which is performed by the SPE).
> +	 */
> +	spe_program_handle_t *loaded_program;
> +
>  	/* We need to keep the entry point for emulated isolated contexts,
>  	 * and ignore the value provided to spe_context_run */
>  	int		emulated_entry;
> @@ -188,6 +194,17 @@ extern spe_gang_context_ptr_t _base_spe_
>  extern int _base_spe_program_load(spe_context_ptr_t spectx, spe_program_handle_t *program);
>  
>  /**
> + * Signal that the program load has completed. For normal apps, this is called
> + * directly in the load path. For (emulated) isolated apps, the load is
> + * asynchronous, so this needs to be called when we know that the load has
> + * completed
> + *
> + * @pre spe->base_priv->loaded_program is a valid SPE program
> + * @param spectx The spe context that has been loaded.
> + */
> +void _base_spe_program_load_complete(spe_context_ptr_t spectx);
> +
> +/**
>   * Check if the emulated loader is present in the filesystem
>   * @return Non-zero if the loader is available, otherwise zero.
>   */
> _______________________________________________
> cbe-oss-dev mailing list
> cbe-oss-dev at ozlabs.org
> https://ozlabs.org/mailman/listinfo/cbe-oss-dev