[Skiboot] [PATCH 1/3] errorlog: Simplify log calling convention

Samuel Mendoza-Jonas sam.mj at au1.ibm.com
Tue Jul 28 14:43:11 AEST 2015


Please ignore this one - some combination of git and perl choked on
the larger patches. Sorry for the noise!

On 28/07/15 14:21, Samuel Mendoza-Jonas wrote:
> Remove the callback functionality from log_error() and replace it with
> the ability to append to a user data section, or add addtional user
> data sections to an error log.
> 
> For multiline or otherwise complex logging the convention is now to call
> opal_elog_create() to obtain a errorlog buffer and use log_append_msg()
> and log_append_data() to append to the user data section. Additional
> user data sections can be added to the error log via log_add_section().
> The caller is then responsible for calling log_commit().
> 
> For simple logs log_simple_error() takes care of creating and committing
> the error log as before.
> 
> Signed-off-by: Samuel Mendoza-Jonas <sam.mj at au1.ibm.com>
> ---
>  core/errorlog.c      | 132 ++++++++++++++++++++++++++++++++-------------------
>  hw/fsp/fsp-mem-err.c |  29 ++++++-----
>  include/errorlog.h   |  16 ++++---
>  3 files changed, 109 insertions(+), 68 deletions(-)
> 
> diff --git a/core/errorlog.c b/core/errorlog.c
> index d0ca71f..7afd16c 100644
> --- a/core/errorlog.c
> +++ b/core/errorlog.c
> @@ -59,35 +59,8 @@ static struct errorlog *get_write_buffer(int opal_event_severity)
>  	return buf;
>  }
>  
> -int opal_elog_update_user_dump(struct errorlog *buf, unsigned char *data,
> -					      uint32_t tag, uint16_t size)
> -{
> -	char *buffer;
> -	struct elog_user_data_section *tmp;
> -
> -	if (!buf) {
> -		prerror("ELOG: Cannot update user data. Buffer is invalid\n");
> -		return -1;
> -	}
> -
> -	buffer = (char *)buf->user_data_dump + buf->user_section_size;
> -	if ((buf->user_section_size + size) > OPAL_LOG_MAX_DUMP) {
> -		prerror("ELOG: Size of dump data overruns buffer\n");
> -		return -1;
> -	}
> -
> -	tmp = (struct elog_user_data_section *)buffer;
> -	tmp->tag = tag;
> -	tmp->size = size + sizeof(struct elog_user_data_section) - 1;
> -	memcpy(tmp->data_dump, data, size);
> -
> -	buf->user_section_size += tmp->size;
> -	buf->user_section_count++;
> -	return 0;
> -}
> -
>  /* Reporting of error via struct errorlog */
> -struct errorlog *opal_elog_create(struct opal_err_info *e_info)
> +struct errorlog *opal_elog_create(struct opal_err_info *e_info, uint32_t tag)
>  {
>  	struct errorlog *buf;
>  
> @@ -104,11 +77,41 @@ struct errorlog *opal_elog_create(struct opal_err_info *e_info)
>  		lock(&elog_lock);
>  		buf->plid = ++sapphire_elog_id;
>  		unlock(&elog_lock);
> +
> +		/* Initialise the first user dump section */
> +		log_add_section(buf, tag);
>  	}
>  
>  	return buf;
>  }
>  
> +/* Add a new user data section to an existing error log */
> +void log_add_section(struct errorlog *buf, uint32_t tag)
> +{
> +	size_t size = sizeof(struct elog_user_data_section) - 1;
> +	struct elog_user_data_section *tmp;
> +
> +	if (!buf) {
> +		prerror("ELOG: Cannot add user data section. "
> +			"Buffer is invalid\n");
> +		return;
> +	}
> +
> +	if ((buf->user_section_size + size) > OPAL_LOG_MAX_DUMP) {
> +		prerror("ELOG: Size of dump data overruns buffer\n");
> +		return;
> +	}
> +
> +	tmp = (struct elog_user_data_section *)(buf->user_data_dump +
> +						buf->user_section_size);
> +	/* Use DESC if no other tag provided */
> +	tmp->tag = tag ? tag : 0x44455343;
> +	tmp->size = size;
> +
> +	buf->user_section_size += tmp->size;
> +	buf->user_section_count++;
> +}
> +
>  void opal_elog_complete(struct errorlog *buf, bool success)
>  {
>  	if (!success)
> @@ -119,10 +122,13 @@ void opal_elog_complete(struct errorlog *buf, bool success)
>  	unlock(&elog_lock);
>  }
>  
> -static void elog_commit(struct errorlog *elog)
> +void log_commit(struct errorlog *elog)
>  {
>  	int rc;
>  
> +	if (!elog)
> +		return;
> +
>  	if (platform.elog_commit) {
>  		rc = platform.elog_commit(elog);
>  		if (rc)
> @@ -132,13 +138,53 @@ static void elog_commit(struct errorlog *elog)
>  	opal_elog_complete(elog, false);
>  }
>  
> -void log_error(struct opal_err_info *e_info, void *data, uint16_t size,
> -	       const char *fmt, ...)
> +void log_append_data(struct errorlog *buf, unsigned char *data, uint16_t size)
> +{
> +	struct elog_user_data_section *section;
> +	uint8_t n_sections;
> +	char *buffer;
> +
> +	if (!buf) {
> +		prerror("ELOG: Cannot update user data. Buffer is invalid\n");
> +		return;
> +	}
> +
> +	if ((buf->user_section_size + size) > OPAL_LOG_MAX_DUMP) {
> +		prerror("ELOG: Size of dump data overruns buffer\n");
> +		return;
> +	}
> +
> +	/* Step through user sections to find latest dump section */
> +	buffer = buf->user_data_dump;
> +	n_sections = buf->user_section_count;
> +
> +	if (!n_sections) {
> +		prerror("ELOG: User section invalid\n");
> +		return;
> +	}
> +
> +	while (--n_sections) {
> +		section = (struct elog_user_data_section *)buffer;
> +		buffer += section->size;
> +	}
> +
> +	section = (struct elog_user_data_section *)buffer;
> +	buffer += section->size;
> +	memcpy(buffer, data, size);
> +
> +	section->size += size;
> +	buf->user_section_size += size;
> +}
> +
> +void log_append_msg(struct errorlog *buf, const char *fmt, ...)
>  {
> -	struct errorlog *buf;
> -	int tag = 0x44455343;  /* ASCII of DESC */
> -	va_list list;
>  	char err_msg[250];
> +	va_list list;
> +
> +	if (!buf) {
> +		prerror("Tried to append log to NULL buffer\n");
> +		return;
> +	}
>  
>  	va_start(list, fmt);
>  	vsnprintf(err_msg, sizeof(err_msg), fmt, list);
> @@ -147,22 +193,12 @@ void log_error(struct opal_err_info *e_info, void *data, uint16_t size,
>  	/* Log the error on to Sapphire console */
>  	prerror("%s", err_msg);
>  
> -	buf = opal_elog_create(e_info);
> -	if (buf == NULL)
> -		prerror("ELOG: Error getting buffer to log error\n");
> -	else {
> -		opal_elog_update_user_dump(buf, err_msg, tag, strlen(err_msg));
> -		/* Append any number of call out dumps */
> -		if (e_info->call_out)
> -			e_info->call_out(buf, data, size);
> -		elog_commit(buf);
> -	}
> +	log_append_data(buf, err_msg, strlen(err_msg));
>  }
>  
>  void log_simple_error(struct opal_err_info *e_info, const char *fmt, ...)
>  {
>  	struct errorlog *buf;
> -	int tag = 0x44455343;  /* ASCII of DESC */
>  	va_list list;
>  	char err_msg[250];
>  
> @@ -173,12 +209,12 @@ void log_simple_error(struct opal_err_info *e_info, const char *fmt, ...)
>  	/* Log the error on to Sapphire console */
>  	prerror("%s", err_msg);
>  
> -	buf = opal_elog_create(e_info);
> +	buf = opal_elog_create(e_info, 0);
>  	if (buf == NULL)
>  		prerror("ELOG: Error getting buffer to log error\n");
>  	else {
> -		opal_elog_update_user_dump(buf, err_msg, tag, strlen(err_msg));
> -		elog_commit(buf);
> +		log_append_data(buf, err_msg, strlen(err_msg));
> +		log_commit(buf);
>  	}
>  }
>  
> diff --git a/hw/fsp/fsp-mem-err.c b/hw/fsp/fsp-mem-err.c
> index 3eaa1a3..a4b45fc 100644
> --- a/hw/fsp/fsp-mem-err.c
> +++ b/hw/fsp/fsp-mem-err.c
> @@ -41,20 +41,13 @@ static LIST_HEAD(mem_error_list);
>   */
>  static struct lock mem_err_lock = LOCK_UNLOCKED;
>  
> -static void mem_err_info_dump(struct errorlog *buf, void *data, uint16_t size);
> -
>  DEFINE_LOG_ENTRY(OPAL_RC_MEM_ERR_RES, OPAL_PLATFORM_ERR_EVT, OPAL_MEM_ERR,
>  			OPAL_MISC_SUBSYSTEM, OPAL_PREDICTIVE_ERR_GENERAL,
> -			OPAL_NA, mem_err_info_dump);
> +			OPAL_NA, NULL);
>  
>  DEFINE_LOG_ENTRY(OPAL_RC_MEM_ERR_DEALLOC, OPAL_PLATFORM_ERR_EVT, OPAL_MEM_ERR,
>  			OPAL_MISC_SUBSYSTEM, OPAL_PREDICTIVE_ERR_GENERAL,
> -			OPAL_NA, mem_err_info_dump);
> -
> -static void mem_err_info_dump(struct errorlog *buf, void *data, uint16_t size)
> -{
> -	opal_elog_update_user_dump(buf, data, 0x44455350, size);
> -}
> +			OPAL_NA, NULL);
>  
>  static bool send_response_to_fsp(u32 cmd_sub_mod)
>  {
> @@ -192,6 +185,7 @@ static bool handle_memory_resilience(u32 cmd_sub_mod, u64 paddr)
>  {
>  	int rc = 0;
>  	struct OpalMemoryErrorData mem_err_evt;
> +	struct errorlog *buf;
>  
>  	memset(&mem_err_evt, 0, sizeof(struct OpalMemoryErrorData));
>  	/* Check arguments */
> @@ -243,10 +237,14 @@ send_response:
>  	if (!rc)
>  		return send_response_to_fsp(FSP_RSP_MEM_RES);
>  	else {
> -		log_error(&e_info(OPAL_RC_MEM_ERR_RES),
> -			&mem_err_evt, sizeof(struct OpalMemoryErrorData),
> +		buf = opal_elog_create(&e_info(OPAL_RC_MEM_ERR_RES), 0);
> +		log_append_msg(buf,
>  			"OPAL_MEM_ERR: Cannot queue up memory "
>  			"resilience error event to the OS");
> +		log_add_section(buf, 0x44455350);
> +		log_append_data(buf, (char *) &mem_err_evt,
> +					   sizeof(struct OpalMemoryErrorData));
> +		log_commit(buf);
>  		return false;
>  	}
>  }
> @@ -290,6 +288,7 @@ static bool handle_memory_deallocation(u64 paddr_start, u64 paddr_end)
>  	int rc = 0;
>  	u8 err = 0;
>  	struct OpalMemoryErrorData mem_err_evt;
> +	struct errorlog *buf;
>  
>  	memset(&mem_err_evt, 0, sizeof(struct OpalMemoryErrorData));
>  	/* Check arguments */
> @@ -327,10 +326,14 @@ send_response:
>  	if (!rc)
>  		return send_response_to_fsp(FSP_RSP_MEM_DYN_DEALLOC);
>  	else {
> -		log_error(&e_info(OPAL_RC_MEM_ERR_DEALLOC),
> -			&mem_err_evt, sizeof(struct OpalMemoryErrorData),
> +		buf = opal_elog_create(&e_info(OPAL_RC_MEM_ERR_DEALLOC), 0);
> +		log_append_msg(buf,
>  			"OPAL_MEM_ERR: Cannot queue up memory "
>  			"deallocation error event to the OS");
> +		log_add_section(buf, 0x44455350);
> +		log_append_data(buf, (char *)&mem_err_evt,
> +					   sizeof(struct OpalMemoryErrorData));
> +		log_commit(buf);
>  		return false;
>  	}
>  }
> diff --git a/include/errorlog.h b/include/errorlog.h
> index af9b441..eb37452 100644
> --- a/include/errorlog.h
> +++ b/include/errorlog.h
> @@ -335,16 +335,18 @@ severity, subtype, callout_func) static struct opal_err_info err_##reason =	\
>   * and commits the error to FSP.
>   * Used for simple error logging
>   */
> -void log_simple_error(struct opal_err_info *e_info, const char *fmt, ...) __attribute__ ((format (printf, 2, 3)));
> -void log_error(struct opal_err_info *e_info, void *data, uint16_t size,
> -		const char *fmt, ...) __attribute__ ((format (printf, 4, 5)));
> +void log_simple_error(struct opal_err_info *e_info,
> +		const char *fmt, ...) __attribute__ ((format (printf, 2, 3)));
>  
>  #define e_info(reason_code) err_##reason_code
>  
> -struct errorlog *opal_elog_create(struct opal_err_info *e_info) __warn_unused_result;
> -
> -int opal_elog_update_user_dump(struct errorlog *buf, unsigned char *data,
> -						uint32_t tag, uint16_t size);
> +struct errorlog *opal_elog_create(struct opal_err_info *e_info,
> +				  uint32_t tag) __warn_unused_result;
> +void log_add_section(struct errorlog *buf, uint32_t tag);
> +void log_append_data(struct errorlog *buf, unsigned char *data, uint16_t size);
> +void log_append_msg(struct errorlog *buf,
> +		const char *fmt, ...) __attribute__ ((format (printf, 2, 3)));
> +void log_commit(struct errorlog *elog);
>  
>  /* Called by the backend after an error has been logged by the
>   * backend. If the error could not be logged successfully success is
> 


-- 
-----------
LTC Ozlabs
IBM



More information about the Skiboot mailing list