[Pdbg] [PATCH v2 20/24] libpdbg: Separate sbefifo chipops into a separate hwunit

Alistair Popple alistair at popple.id.au
Wed Nov 20 14:58:45 AEDT 2019


This came out looking pretty good without too much crazy casting/tree traversal.

Reviewed-by: Alistair Popple <alistair at popple.id.au>

On Thursday, 7 November 2019 1:28:04 PM AEDT Amitay Isaacs wrote:
> This makes sbefifo driver only responsible for the transport and ffdc.
> And chipop hwunit can now be independent of the backend.
> 
> Signed-off-by: Amitay Isaacs <amitay at ozlabs.org>
> ---
>  libpdbg/chip.c    | 32 ++++++++++-----------
>  libpdbg/hwunit.h  | 17 ++++++++----
>  libpdbg/sbefifo.c | 71 ++++++++++++++++++++++++++++-------------------
>  libpdbg/target.c  | 35 +++++++++++++++++------
>  libpdbg/target.h  |  1 +
>  5 files changed, 98 insertions(+), 58 deletions(-)
> 
> diff --git a/libpdbg/chip.c b/libpdbg/chip.c
> index 830c3ec..908b20d 100644
> --- a/libpdbg/chip.c
> +++ b/libpdbg/chip.c
> @@ -160,17 +160,17 @@ int thread_step_all(void)
>  	int rc = 0, count = 0;
>  
>  	pdbg_for_each_class_target("pib", pib) {
> -		struct sbefifo *sbefifo;
> +		struct chipop *chipop;
>  
> -		sbefifo = pib_to_sbefifo(pib);
> -		if (!sbefifo)
> +		chipop = pib_to_chipop(pib);
> +		if (!chipop)
>  			break;
>  
>  		/*
>  		 * core_id = 0xff (all SMT4 cores)
>  		 * thread_id = 0xf (all 4 threads in the SMT4 core)
>  		 */
> -		rc |= sbefifo->thread_step(sbefifo, 0xff, 0xf);
> +		rc |= chipop->thread_step(chipop, 0xff, 0xf);
>  		count++;
>  	}
>  
> @@ -193,17 +193,17 @@ int thread_start_all(void)
>  	int rc = 0, count = 0;
>  
>  	pdbg_for_each_class_target("pib", pib) {
> -		struct sbefifo *sbefifo;
> +		struct chipop *chipop;
>  
> -		sbefifo = pib_to_sbefifo(pib);
> -		if (!sbefifo)
> +		chipop = pib_to_chipop(pib);
> +		if (!chipop)
>  			break;
>  
>  		/*
>  		 * core_id = 0xff (all SMT4 cores)
>  		 * thread_id = 0xf (all 4 threads in the SMT4 core)
>  		 */
> -		rc |= sbefifo->thread_start(sbefifo, 0xff, 0xf);
> +		rc |= chipop->thread_start(chipop, 0xff, 0xf);
>  		count++;
>  	}
>  
> @@ -226,17 +226,17 @@ int thread_stop_all(void)
>  	int rc = 0, count = 0;
>  
>  	pdbg_for_each_class_target("pib", pib) {
> -		struct sbefifo *sbefifo;
> +		struct chipop *chipop;
>  
> -		sbefifo = pib_to_sbefifo(pib);
> -		if (!sbefifo)
> +		chipop = pib_to_chipop(pib);
> +		if (!chipop)
>  			break;
>  
>  		/*
>  		 * core_id = 0xff (all SMT4 cores)
>  		 * thread_id = 0xf (all 4 threads in the SMT4 core)
>  		 */
> -		rc |= sbefifo->thread_stop(sbefifo, 0xff, 0xf);
> +		rc |= chipop->thread_stop(chipop, 0xff, 0xf);
>  		count++;
>  	}
>  
> @@ -259,17 +259,17 @@ int thread_sreset_all(void)
>  	int rc = 0, count = 0;
>  
>  	pdbg_for_each_class_target("pib", pib) {
> -		struct sbefifo *sbefifo;
> +		struct chipop *chipop;
>  
> -		sbefifo = pib_to_sbefifo(pib);
> -		if (!sbefifo)
> +		chipop = pib_to_chipop(pib);
> +		if (!chipop)
>  			break;
>  
>  		/*
>  		 * core_id = 0xff (all SMT4 cores)
>  		 * thread_id = 0xf (all 4 threads in the SMT4 core)
>  		 */
> -		rc |= sbefifo->thread_sreset(sbefifo, 0xff, 0xf);
> +		rc |= chipop->thread_sreset(chipop, 0xff, 0xf);
>  		count++;
>  	}
>  
> diff --git a/libpdbg/hwunit.h b/libpdbg/hwunit.h
> index 06e5fca..40689df 100644
> --- a/libpdbg/hwunit.h
> +++ b/libpdbg/hwunit.h
> @@ -65,14 +65,19 @@ struct mem {
>  };
>  #define target_to_mem(x) container_of(x, struct mem, target)
>  
> +struct chipop {
> +	struct pdbg_target target;
> +	uint32_t (*ffdc_get)(struct chipop *, const uint8_t **, uint32_t *);
> +	int (*istep)(struct chipop *, uint32_t major, uint32_t minor);
> +	int (*thread_start)(struct chipop *, uint32_t core_id, uint32_t thread_id);
> +	int (*thread_stop)(struct chipop *, uint32_t core_id, uint32_t thread_id);
> +	int (*thread_step)(struct chipop *, uint32_t core_id, uint32_t thread_id);
> +	int (*thread_sreset)(struct chipop *, uint32_t core_id, uint32_t thread_id);
> +};
> +#define target_to_chipop(x) container_of(x, struct chipop, target)
> +
>  struct sbefifo {
>  	struct pdbg_target target;
> -	int (*istep)(struct sbefifo *, uint32_t major, uint32_t minor);
> -	int (*thread_start)(struct sbefifo *, uint32_t core_id, uint32_t thread_id);
> -	int (*thread_stop)(struct sbefifo *, uint32_t core_id, uint32_t thread_id);
> -	int (*thread_step)(struct sbefifo *, uint32_t core_id, uint32_t thread_id);
> -	int (*thread_sreset)(struct sbefifo *, uint32_t core_id, uint32_t thread_id);
> -	uint32_t (*ffdc_get)(struct sbefifo *, const uint8_t **, uint32_t *);
>  	struct sbefifo_context *sf_ctx;
>  };
>  #define target_to_sbefifo(x) container_of(x, struct sbefifo, target)
> diff --git a/libpdbg/sbefifo.c b/libpdbg/sbefifo.c
> index e53a4f8..98f3442 100644
> --- a/libpdbg/sbefifo.c
> +++ b/libpdbg/sbefifo.c
> @@ -25,19 +25,6 @@
>  #include "hwunit.h"
>  #include "debug.h"
>  
> -static uint32_t sbefifo_op_ffdc_get(struct sbefifo *sbefifo, const uint8_t **ffdc, uint32_t *ffdc_len)
> -{
> -	return sbefifo_ffdc_get(sbefifo->sf_ctx, ffdc, ffdc_len);
> -}
> -
> -static int sbefifo_op_istep(struct sbefifo *sbefifo,
> -			    uint32_t major, uint32_t minor)
> -{
> -	PR_NOTICE("sbefifo: istep %u.%u\n", major, minor);
> -
> -	return sbefifo_istep_execute(sbefifo->sf_ctx, major & 0xff, minor & 0xff);
> -}
> -
>  static int sbefifo_op_getmem(struct mem *sbefifo_mem,
>  			     uint64_t addr, uint8_t *data, uint64_t size,
>  			     uint8_t block_size, bool ci)
> @@ -105,10 +92,28 @@ static int sbefifo_op_putmem(struct mem *sbefifo_mem,
>  	return 0;
>  }
>  
> -static int sbefifo_op_control(struct sbefifo *sbefifo,
> +static uint32_t sbefifo_op_ffdc_get(struct chipop *chipop, const uint8_t **ffdc, uint32_t *ffdc_len)
> +{
> +	struct sbefifo *sbefifo = target_to_sbefifo(chipop->target.parent);
> +
> +	return sbefifo_ffdc_get(sbefifo->sf_ctx, ffdc, ffdc_len);
> +}
> +
> +static int sbefifo_op_istep(struct chipop *chipop,
> +			    uint32_t major, uint32_t minor)
> +{
> +	struct sbefifo *sbefifo = target_to_sbefifo(chipop->target.parent);
> +
> +	PR_NOTICE("sbefifo: istep %u.%u\n", major, minor);
> +
> +	return sbefifo_istep_execute(sbefifo->sf_ctx, major & 0xff, minor & 0xff);
> +}
> +
> +static int sbefifo_op_control(struct chipop *chipop,
>  			      uint32_t core_id, uint32_t thread_id,
>  			      uint32_t oper)
>  {
> +	struct sbefifo *sbefifo = target_to_sbefifo(chipop->target.parent);
>  	uint8_t mode = 0;
>  
>  	/* Enforce special-wakeup for thread stop and sreset */
> @@ -121,28 +126,28 @@ static int sbefifo_op_control(struct sbefifo *sbefifo,
>  	return sbefifo_control_insn(sbefifo->sf_ctx, core_id & 0xff, thread_id & 0xff, oper & 0xff, mode);
>  }
>  
> -static int sbefifo_op_thread_start(struct sbefifo *sbefifo,
> +static int sbefifo_op_thread_start(struct chipop *chipop,
>  				   uint32_t core_id, uint32_t thread_id)
>  {
> -	return sbefifo_op_control(sbefifo, core_id, thread_id, SBEFIFO_INSN_OP_START);
> +	return sbefifo_op_control(chipop, core_id, thread_id, SBEFIFO_INSN_OP_START);
>  }
>  
> -static int sbefifo_op_thread_stop(struct sbefifo *sbefifo,
> +static int sbefifo_op_thread_stop(struct chipop *chipop,
>  				  uint32_t core_id, uint32_t thread_id)
>  {
> -	return sbefifo_op_control(sbefifo, core_id, thread_id, SBEFIFO_INSN_OP_STOP);
> +	return sbefifo_op_control(chipop, core_id, thread_id, SBEFIFO_INSN_OP_STOP);
>  }
>  
> -static int sbefifo_op_thread_step(struct sbefifo *sbefifo,
> +static int sbefifo_op_thread_step(struct chipop *chipop,
>  				  uint32_t core_id, uint32_t thread_id)
>  {
> -	return sbefifo_op_control(sbefifo, core_id, thread_id, SBEFIFO_INSN_OP_STEP);
> +	return sbefifo_op_control(chipop, core_id, thread_id, SBEFIFO_INSN_OP_STEP);
>  }
>  
> -static int sbefifo_op_thread_sreset(struct sbefifo *sbefifo,
> +static int sbefifo_op_thread_sreset(struct chipop *chipop,
>  				    uint32_t core_id, uint32_t thread_id)
>  {
> -	return sbefifo_op_control(sbefifo, core_id, thread_id, SBEFIFO_INSN_OP_SRESET);
> +	return sbefifo_op_control(chipop, core_id, thread_id, SBEFIFO_INSN_OP_SRESET);
>  }
>  
>  static int sbefifo_probe(struct pdbg_target *target)
> @@ -181,6 +186,21 @@ static struct mem sbefifo_mem = {
>  };
>  DECLARE_HW_UNIT(sbefifo_mem);
>  
> +static struct chipop sbefifo_chipop = {
> +	.target = {
> +		.name = "SBE FIFO Chip-op engine",
> +		.compatible = "ibm,sbefifo-chipop",
> +		.class = "chipop",
> +	},
> +	.ffdc_get = sbefifo_op_ffdc_get,
> +	.istep = sbefifo_op_istep,
> +	.thread_start = sbefifo_op_thread_start,
> +	.thread_stop = sbefifo_op_thread_stop,
> +	.thread_step = sbefifo_op_thread_step,
> +	.thread_sreset = sbefifo_op_thread_sreset,
> +};
> +DECLARE_HW_UNIT(sbefifo_chipop);
> +
>  static struct sbefifo kernel_sbefifo = {
>  	.target = {
>  		.name =	"Kernel based FSI SBE FIFO",
> @@ -189,12 +209,6 @@ static struct sbefifo kernel_sbefifo = {
>  		.probe = sbefifo_probe,
>  		.release = sbefifo_release,
>  	},
> -	.istep = sbefifo_op_istep,
> -	.thread_start = sbefifo_op_thread_start,
> -	.thread_stop = sbefifo_op_thread_stop,
> -	.thread_step = sbefifo_op_thread_step,
> -	.thread_sreset = sbefifo_op_thread_sreset,
> -	.ffdc_get = sbefifo_op_ffdc_get,
>  };
>  DECLARE_HW_UNIT(kernel_sbefifo);
>  
> @@ -202,5 +216,6 @@ __attribute__((constructor))
>  static void register_sbefifo(void)
>  {
>  	pdbg_hwunit_register(&kernel_sbefifo_hw_unit);
> +	pdbg_hwunit_register(&sbefifo_chipop_hw_unit);
>  	pdbg_hwunit_register(&sbefifo_mem_hw_unit);
>  }
> diff --git a/libpdbg/target.c b/libpdbg/target.c
> index da7814b..603f206 100644
> --- a/libpdbg/target.c
> +++ b/libpdbg/target.c
> @@ -290,26 +290,45 @@ struct sbefifo *pib_to_sbefifo(struct pdbg_target *pib)
>  	return NULL;
>  }
>  
> +struct chipop *pib_to_chipop(struct pdbg_target *pib)
> +{
> +	struct pdbg_target *chipop;
> +	uint32_t index;
> +
> +	assert(pdbg_target_is_class(pib, "pib"));
> +	index = pdbg_target_index(pib);
> +
> +	pdbg_for_each_class_target("chipop", chipop) {
> +		if (pdbg_target_index(chipop) != index)
> +			continue;
> +
> +		if (pdbg_target_probe(chipop) == PDBG_TARGET_ENABLED)
> +			return target_to_chipop(chipop);
> +	}
> +
> +	return NULL;
> +}
> +
>  int sbe_istep(struct pdbg_target *target, uint32_t major, uint32_t minor)
>  {
> -	struct sbefifo *sbefifo;
> +	struct chipop *chipop;
>  
> -	sbefifo = pib_to_sbefifo(target);
> -	if (!sbefifo)
> +	chipop = pib_to_chipop(target);
> +	if (!chipop)
>  		return -1;
>  
> -	return sbefifo->istep(sbefifo, major, minor);
> +	return chipop->istep(chipop, major, minor);
>  }
>  
>  uint32_t sbe_ffdc_get(struct pdbg_target *target, const uint8_t **ffdc, uint32_t *ffdc_len)
>  {
> -	struct sbefifo *sbefifo;
> +	struct chipop *chipop;
>  
> -	sbefifo = pib_to_sbefifo(target);
> -	if (!sbefifo)
> +	chipop = pib_to_chipop(target);
> +	if (!chipop)
>  		return -1;
>  
> -	return sbefifo->ffdc_get(sbefifo, ffdc, ffdc_len);
> +	return chipop->ffdc_get(chipop, ffdc, ffdc_len);
>  }
>  
>  /* Finds the given class. Returns NULL if not found. */
> diff --git a/libpdbg/target.h b/libpdbg/target.h
> index 35e822b..ca79a84 100644
> --- a/libpdbg/target.h
> +++ b/libpdbg/target.h
> @@ -73,6 +73,7 @@ void pdbg_default_dtb(struct pdbg_dtb *pdtb);
>  const char *pdbg_get_backend_option(void);
>  
>  struct sbefifo *pib_to_sbefifo(struct pdbg_target *target);
> +struct chipop *pib_to_chipop(struct pdbg_target *target);
>  bool target_is_virtual(struct pdbg_target *target);
>  struct pdbg_target *target_to_real(struct pdbg_target *target, bool strict);
>  struct pdbg_target *target_to_virtual(struct pdbg_target *target, bool strict);
> 






More information about the Pdbg mailing list