[Pdbg] [PATCH v2 5/6] libpdbg: Add sbefifo driver for cronus backend

[Alistair Popple]

On Thursday, 24 October 2019 12:53:30 PM AEDT Amitay Isaacs wrote:
> Signed-off-by: Amitay Isaacs <amitay at ozlabs.org>
> ---
>  libpdbg/cronus.c | 320 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 316 insertions(+), 4 deletions(-)
> 
> diff --git a/libpdbg/cronus.c b/libpdbg/cronus.c
> index 7931611..1475622 100644
> --- a/libpdbg/cronus.c
> +++ b/libpdbg/cronus.c
> @@ -14,15 +14,14 @@
>   * limitations under the License.
>   */
>  #include <stdint.h>
> +#include <inttypes.h>
>  #include <stdio.h>
>  #include <unistd.h>
>  #include <stdlib.h>
> -#include <sys/types.h>
> -#include <sys/socket.h>
> -#include <netinet/in.h>
> -#include <arpa/inet.h>
> +#include <errno.h>
>  
>  #include <libcronus/libcronus.h>
> +#include <libsbefifo/libsbefifo.h>
>  
>  #include "hwunit.h"
>  #include "debug.h"
> @@ -138,6 +137,278 @@ static int cronus_fsi_write(struct fsi *fsi, uint32_t 
addr, uint32_t value)
>  	return 0;
>  }
>  
> +static uint32_t cronus_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 cronus_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 cronus_op_getmem(struct mem *sbefifo_mem,
> +			     uint64_t addr, uint8_t *data, uint64_t size,
> +			     uint8_t block_size, bool ci)

We shouldn't need a separate cronus_op_getmem, it should use the same hardware 
unit as the kernel sbefifo driver. Specifically the device tree should look 
something like this for the Cronus backend:

               sbefifo at 2400 { /* Bogus address */
                       reg = <0x0 0x2400 0x7>;
                       index = <0x0>;
                       compatible = "ibm,cronus-sbefifo";
                       #address-cells = <0x2>;
                       #size-cells = <0x1>;

                       sbefifo-mem at 0 {
                               compatible = "ibm,sbefifo-mem";
                               reg = <0x0 0x0 0x0>;
                       };
	       };

The HW unit implemented by the "ibm,cronus-sbefifo" driver should provide the 
correct callbacks to completely abstract the actual transport so that the 
"ibm,sbefifo-mem" unit only has to format the chip-op and pass it on to the 
sbefifo for transport.

> +{
> +	struct sbefifo *sbefifo = target_to_sbefifo(sbefifo_mem->target.parent);
> +	uint8_t *out;
> +	uint64_t start_addr, end_addr;
> +	uint32_t align, offset, len;
> +	uint16_t flags;
> +	int rc;
> +
> +	align = 8;
> +
> +	if (block_size && block_size != 8) {
> +		PR_ERROR("sbefifo: Only 8 byte block sizes are supported\n");
> +		return -1;
> +	};
> +
> +	start_addr = addr & (~(uint64_t)(align-1));
> +	end_addr = (addr + size + (align-1)) & (~(uint64_t)(align-1));
> +
> +	if (end_addr - start_addr > UINT32_MAX) {
> +		PR_ERROR("sbefifo: size too large\n");
> +		return -EINVAL;
> +	}
> +
> +	offset = addr - start_addr;
> +	len = end_addr - start_addr;
> +
> +	PR_NOTICE("sbefifo: getmem addr=0x%016" PRIx64 ", len=%u\n",
> +		  start_addr, len);
> +
> +	flags = SBEFIFO_MEMORY_FLAG_PROC;
> +	if (ci)
> +		flags |= SBEFIFO_MEMORY_FLAG_CI;
> +
> +	rc = sbefifo_mem_get(sbefifo->sf_ctx, start_addr, len, flags, &out);
> +	if (rc)
> +		return rc;
> +
> +	pdbg_progress_tick(len, len);
> +
> +	memcpy(data, out+offset, size);
> +	free(out);
> +
> +	return 0;
> +}

<snip>

> +static int cronus_op_thread_sreset(struct sbefifo *sbefifo,
> +				    uint32_t core_id, uint32_t thread_id)
> +{
> +	return cronus_op_control(sbefifo, core_id, thread_id, 
SBEFIFO_INSN_OP_SRESET);
> +}

It looks like most of this code is just a direct copy of the implementations 
in sbefifo.c, so we should just be calling those. Hence why the sbefifo-mem 
driver should be the same regardless of backend/transport.

> +static int cronus_sbefifo_probe(struct pdbg_target *target)
> +{
> +	struct sbefifo *sf = target_to_sbefifo(target);
> +	int rc;
> +
> +	rc = sbefifo_connect_transport(cronus_transport, target, &sf->sf_ctx);

As the only bit that really needs to be different is the library/context 
initialisation.

- Alistair

> +	if (rc) {
> +		PR_ERROR("Unable to initialize sbefifo driver\n");
> +		return rc;
> +	}
> +
> +	return 0;
> +}
> +
>  static struct pib cronus_pib = {
>  	.target = {
>  		.name =	"Cronus Client based PIB",
> @@ -164,9 +435,50 @@ static struct fsi cronus_fsi = {
>  };
>  DECLARE_HW_UNIT(cronus_fsi);
>  
> +struct mem cronus_sbefifo_mem = {
> +	.target = {
> +		.name = "Cronus Client based SBE FIFO memory access",
> +		.compatible = "ibm,cronus-sbefifo-mem",
> +		.class = "mem",
> +	},
> +	.read = cronus_op_getmem,
> +	.write = cronus_op_putmem,
> +};
> +DECLARE_HW_UNIT(cronus_sbefifo_mem);
> +
> +struct pba cronus_sbefifo_pba = {
> +	.target = {
> +		.name = "Cronus Client based SBE FIFO PBA memory access",
> +		.compatible = "ibm,cronus-sbefifo-pba",
> +		.class = "pba",
> +	},
> +	.read = cronus_op_getmem_pba,
> +	.write = cronus_op_putmem_pba,
> +};
> +DECLARE_HW_UNIT(cronus_sbefifo_pba);
> +
> +struct sbefifo cronus_sbefifo = {
> +	.target = {
> +		.name =	"Cronus Client based SBE FIFO",
> +		.compatible = "ibm,cronus-sbefifo",
> +		.class = "sbefifo",
> +		.probe = cronus_sbefifo_probe,
> +	},
> +	.istep = cronus_op_istep,
> +	.thread_start = cronus_op_thread_start,
> +	.thread_stop = cronus_op_thread_stop,
> +	.thread_step = cronus_op_thread_step,
> +	.thread_sreset = cronus_op_thread_sreset,
> +	.ffdc_get = cronus_op_ffdc_get,
> +};
> +DECLARE_HW_UNIT(cronus_sbefifo);
> +
>  __attribute__((constructor))
>  static void register_cronus(void)
>  {
>  	pdbg_hwunit_register(&cronus_pib_hw_unit);
>  	pdbg_hwunit_register(&cronus_fsi_hw_unit);
> +	pdbg_hwunit_register(&cronus_sbefifo_hw_unit);
> +	pdbg_hwunit_register(&cronus_sbefifo_mem_hw_unit);
> +	pdbg_hwunit_register(&cronus_sbefifo_pba_hw_unit);
>  }
>