[Pdbg] [PATCH 14/17] libpdbg: Expand sbefifo data marshalling and calling chip-op
Alistair Popple
alistair at popple.id.au
Tue Nov 5 15:18:41 AEDT 2019
On Thursday, 31 October 2019 5:34:25 PM AEDT Amitay Isaacs wrote:
> This will allow to separate sbefifo transport layer to use something other
> than sbefifo kernel device driver.
>
> Signed-off-by: Amitay Isaacs <amitay at ozlabs.org>
> ---
> libpdbg/sbefifo.c | 132 ++++++++++++++++++++++++++++------------------
> 1 file changed, 80 insertions(+), 52 deletions(-)
>
> diff --git a/libpdbg/sbefifo.c b/libpdbg/sbefifo.c
> index 030e6e9..9e3532b 100644
> --- a/libpdbg/sbefifo.c
> +++ b/libpdbg/sbefifo.c
> @@ -30,12 +30,29 @@ static uint32_t sbefifo_op_ffdc_get(struct sbefifo *sbefifo,
const uint8_t **ffd
> return sbefifo_ffdc_get(sbefifo->sf_ctx, ffdc, ffdc_len);
> }
>
> -static int sbefifo_op_istep(struct sbefifo *sbefifo,
> - uint32_t major, uint32_t minor)
> +static int sbefifo_op_istep(struct sbefifo *sbefifo, uint32_t major, uint32_t
minor)
> {
> + uint8_t *msg, *out;
> + uint32_t msg_len, out_len;
> + int rc;
> +
> PR_NOTICE("sbefifo: istep %u.%u\n", major, minor);
>
> - return sbefifo_istep_execute(sbefifo->sf_ctx, major & 0xff, minor & 0xff);
> + rc = sbefifo_istep_execute_push(major & 0xff, minor & 0xff, &msg, &msg_len);
> + if (rc)
> + return rc;
> +
> + out_len = 0;
> + rc = sbefifo_operation(sbefifo->sf_ctx, msg, msg_len, &out, &out_len);
This code looks pretty much the same as what is in libsbefifo, so why not just
call that?
I see that later on in the series you change the sbefifo_operation() call to
sbefifo->chipop(), but given you have defined the struct sbefifo_context and it's
still required for calls to libsbefifo why not just define a callback in the
struct? That way pdbg can just call sbefifo_op_istep() and the libsbefifo
library can call something like sf_ctx->transport() which does the right thing
based on how the context is initialised.
- Alistair
> + free(msg);
> + if (rc)
> + return rc;
> +
> + rc = sbefifo_istep_execute_pull(out, out_len);
> + if (out)
> + free(out);
> +
> + return rc;
> }
>
> static int sbefifo_op_getmem(struct mem *sbefifo_mem,
> @@ -43,48 +60,45 @@ static int sbefifo_op_getmem(struct mem *sbefifo_mem,
> uint8_t block_size, bool ci)
> {
> struct sbefifo *sbefifo = target_to_sbefifo(sbefifo_mem->target.parent);
> - uint8_t *out;
> - uint64_t start_addr, end_addr;
> - uint32_t align, offset, len;
> + uint8_t *msg, *out;
> + uint32_t msg_len, out_len, 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;
> + if (size > UINT32_MAX) {
> + PR_ERROR("sbefifo: Invalid size for getmem\n");
> + return EINVAL;
> }
>
> - offset = addr - start_addr;
> - len = end_addr - start_addr;
> + len = size & 0xffffffff;
>
> - PR_NOTICE("sbefifo: getmem addr=0x%016" PRIx64 ", len=%u\n",
> - start_addr, len);
> + PR_NOTICE("sbefifo: getmem addr=0x%016" PRIx64 ", len=%u\n", 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);
> -
> - pdbg_progress_tick(len, len);
> + rc = sbefifo_mem_get_push(addr, len, flags, &msg, &msg_len);
> + if (rc)
> + return rc;
>
> + out_len = len + 4;
> + rc = sbefifo_operation(sbefifo->sf_ctx, msg, msg_len, &out, &out_len);
> + free(msg);
> if (rc)
> return rc;
>
> - memcpy(data, out+offset, size);
> - free(out);
> + rc = sbefifo_mem_get_pull(out, out_len, addr, size, flags, &msg);
> + if (out)
> + free(out);
>
> - return 0;
> + if (!rc) {
> + memcpy(data, msg, len);
> + free(msg);
> + pdbg_progress_tick(len, len);
> + }
> +
> + return rc;
> }
>
> static int sbefifo_op_putmem(struct mem *sbefifo_mem,
> @@ -92,30 +106,14 @@ static int sbefifo_op_putmem(struct mem *sbefifo_mem,
> uint8_t block_size, bool ci)
> {
> struct sbefifo *sbefifo = target_to_sbefifo(sbefifo_mem->target.parent);
> - uint32_t align, len;
> + uint8_t *msg, *out;
> + uint32_t msg_len, out_len, 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;
> - };
> -
> - if (addr & (align-1)) {
> - PR_ERROR("sbefifo: Address must be aligned to %d bytes\n", align);
> - return -1;
> - }
> -
> - if (size & (align-1)) {
> - PR_ERROR("sbefifo: Data must be multiple of %d bytes\n", align);
> - return -1;
> - }
> -
> if (size > UINT32_MAX) {
> - PR_ERROR("sbefifo: size too large\n");
> - return -1;
> + PR_ERROR("sbefifo: Invalid size for putmem\n");
> + return EINVAL;
> }
>
> len = size & 0xffffffff;
> @@ -126,9 +124,22 @@ static int sbefifo_op_putmem(struct mem *sbefifo_mem,
> if (ci)
> flags |= SBEFIFO_MEMORY_FLAG_CI;
>
> - rc = sbefifo_mem_put(sbefifo->sf_ctx, addr, data, len, flags);
> + rc = sbefifo_mem_put_push(addr, data, len, flags, &msg, &msg_len);
> + if (rc)
> + return rc;
> +
> + out_len = 4;
> + rc = sbefifo_operation(sbefifo->sf_ctx, msg, msg_len, &out, &out_len);
> + free(msg);
> + if (rc)
> + return rc;
>
> - pdbg_progress_tick(len, len);
> + rc = sbefifo_mem_put_pull(out, out_len);
> + if (out)
> + free(out);
> +
> + if (!rc)
> + pdbg_progress_tick(len, len);
>
> return rc;
> }
> @@ -137,6 +148,9 @@ static int sbefifo_op_control(struct sbefifo *sbefifo,
> uint32_t core_id, uint32_t thread_id,
> uint32_t oper)
> {
> + uint8_t *msg, *out;
> + uint32_t msg_len, out_len;
> + int rc;
> uint8_t mode = 0;
>
> /* Enforce special-wakeup for thread stop and sreset */
> @@ -146,7 +160,21 @@ static int sbefifo_op_control(struct sbefifo *sbefifo,
>
> PR_NOTICE("sbefifo: control c:0x%x, t:0x%x, op:%u mode:%u\n", core_id,
thread_id, oper, mode);
>
> - return sbefifo_control_insn(sbefifo->sf_ctx, core_id & 0xff, thread_id &
0xff, oper & 0xff, mode);
> + rc = sbefifo_control_insn_push(core_id & 0xff, thread_id & 0xff, oper & 0xff,
mode, &msg, &msg_len);
> + if (rc)
> + return rc;
> +
> + out_len = 0;
> + rc = sbefifo_operation(sbefifo->sf_ctx, msg, msg_len, &out, &out_len);
> + free(msg);
> + if (rc)
> + return rc;
> +
> + rc = sbefifo_control_insn_pull(out, out_len);
> + if (out)
> + free(out);
> +
> + return rc;
> }
>
> static int sbefifo_op_thread_start(struct sbefifo *sbefifo,
>
More information about the Pdbg
mailing list