[Pdbg] [RFC 03/12] sbefifo: Rework thread functions
Alistair Popple
alistair at popple.id.au
Tue Aug 20 14:55:54 AEST 2019
On Tuesday, 20 August 2019 1:50:33 PM AEST Amitay Isaacs wrote:
> On Tue, 2019-08-06 at 11:37 +1000, Alistair Popple wrote:
> > The thread control functions should implement the standard thread
> > control interface rather than an sbefifo specific version. Rework the
> > existing functions to match.
> >
> > Signed-off-by: Alistair Popple <alistair at popple.id.au>
> > ---
> > libpdbg/chip.c | 6 +---
> > libpdbg/hwunit.h | 5 +---
> > libpdbg/sbefifo.c | 76 +++++++++++++++++++++++++++++++++++++------
> > ----
> > 3 files changed, 62 insertions(+), 25 deletions(-)
> >
> > diff --git a/libpdbg/chip.c b/libpdbg/chip.c
> > index 67e3afa..259b106 100644
> > --- a/libpdbg/chip.c
> > +++ b/libpdbg/chip.c
> > @@ -166,11 +166,7 @@ int thread_sreset_all(void)
> > if (!sbefifo)
> > 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 |= sbefifo->sreset_all(sbefifo);
> > count++;
> > }
> >
> > diff --git a/libpdbg/hwunit.h b/libpdbg/hwunit.h
> > index a359af0..b8a2dca 100644
> > --- a/libpdbg/hwunit.h
> > +++ b/libpdbg/hwunit.h
> > @@ -68,11 +68,8 @@ struct mem {
> > 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);
> > int (*chipop)(struct sbefifo *, uint32_t *, uint32_t, uint8_t
> > **, uint32_t *, uint32_t *);
> > + int (*sreset_all)(struct sbefifo *);
> > uint32_t (*ffdc_get)(struct sbefifo *, const uint8_t **,
> > uint32_t *);
> > int fd;
> > uint32_t status;
> > diff --git a/libpdbg/sbefifo.c b/libpdbg/sbefifo.c
> > index b7f52a1..ade2d62 100644
> > --- a/libpdbg/sbefifo.c
> > +++ b/libpdbg/sbefifo.c
> > @@ -418,28 +418,60 @@ static int sbefifo_op_control(struct sbefifo
> > *sbefifo,
> > return 0;
> > }
> >
> > -static int sbefifo_op_thread_start(struct sbefifo *sbefifo,
> > - uint32_t core_id, uint32_t
> > thread_id)
> > +static int sbefifo_op_thread(struct thread *thread, uint32_t op)
> > {
> > - return sbefifo_op_control(sbefifo, core_id, thread_id,
> > SBEFIFO_INSN_OP_START);
> > + return sbefifo_op_control(target_to_sbefifo(thread-
> > >target.parent),
> > + thread->id >> 8, thread->id & 0xff,
> > op);
> > }
> >
> > -static int sbefifo_op_thread_stop(struct sbefifo *sbefifo,
> > - uint32_t core_id, uint32_t thread_id)
> > +static int sbefifo_op_thread_start(struct thread *thread)
> > {
> > - return sbefifo_op_control(sbefifo, core_id, thread_id,
> > SBEFIFO_INSN_OP_STOP);
> > + return sbefifo_op_thread(thread, SBEFIFO_INSN_OP_START);
> > }
> >
> > -static int sbefifo_op_thread_step(struct sbefifo *sbefifo,
> > - uint32_t core_id, uint32_t thread_id)
> > +static int sbefifo_op_thread_stop(struct thread *thread)
> > {
> > - return sbefifo_op_control(sbefifo, core_id, thread_id,
> > SBEFIFO_INSN_OP_STEP);
> > + return sbefifo_op_thread(thread, SBEFIFO_INSN_OP_STOP);
> > }
> >
> > -static int sbefifo_op_thread_sreset(struct sbefifo *sbefifo,
> > - uint32_t core_id, uint32_t
> > thread_id)
> > +static int sbefifo_op_thread_step(struct thread *thread, int count)
> > {
> > - return sbefifo_op_control(sbefifo, core_id, thread_id,
> > SBEFIFO_INSN_OP_SRESET);
> > + int i, rc;
> > +
> > + for (i = 0; i < count; i++)
> > + if ((rc = sbefifo_op_thread(thread,
> > SBEFIFO_INSN_OP_STEP)))
> > + return rc;
> > +
> > + return 0;
> > +}
> > +
> > +static int sbefifo_op_thread_sreset(struct thread *thread)
> > +{
> > + return sbefifo_op_thread(thread, SBEFIFO_INSN_OP_SRESET);
> > +}
> > +
> > +static int sbefifo_op_sreset_all(struct sbefifo *sbefifo)
> > +{
> > + /*
> > + * core_id = 0xff (all SMT4 cores)
> > + * thread_id = 0xf (all 4 threads in the SMT4 core)
> > + */
> > + return sbefifo_op_control(sbefifo, 0xff, 0xf,
> > SBEFIFO_INSN_OP_SRESET);
> > +}
> > +
> > +static int sbefifo_thread_probe(struct pdbg_target *target)
> > +{
> > + struct thread *thread = target_to_thread(target);
> > + uint32_t core_id, thread_id;
> > +
> > + if (pdbg_target_u32_property(target, "core-id", &core_id))
> > + return -1;
> > +
> > + if (pdbg_target_u32_property(target, "thread-id", &thread_id))
> > + return -1;
> > +
> > + thread->id = (core_id << 8) | (thread_id & 0xff);
> > + return 0;
> > }
>
> Do we really need to add two more properties for each thread? core-id
> and thread-id are the same as core-index (provided core-index is the
> right chiplet-index I think! We need to test this.) and thread-index.
> We can calculate them run-time; is there a reason to do it at probe
> time?
It depends on what values the SBE chip-op expects. If you know how we can
calculate these at runtime based on the chiplet index and local core index
(0-4) then that would certainly be the better option. I could see any examples
of this in the existing code though, do you know what values the SBE expects
here (other than 0xfff..)?
- Alistair
> >
> > static int sbefifo_op_chipop(struct sbefifo *sbefifo,
> > @@ -502,6 +534,20 @@ struct mem sbefifo_mem = {
> > };
> > DECLARE_HW_UNIT(sbefifo_mem);
> >
> > +struct thread sbefifo_thread = {
> > + .target = {
> > + .name = "SBE FIFO Chip-op based thread access",
> > + .compatible = "ibm,sbefifo-thread",
> > + .class = "thread",
> > + .probe = sbefifo_thread_probe,
> > + },
> > + .start = sbefifo_op_thread_start,
> > + .stop = sbefifo_op_thread_stop,
> > + .step = sbefifo_op_thread_step,
> > + .sreset = sbefifo_op_thread_sreset,
> > +};
> > +DECLARE_HW_UNIT(sbefifo_thread);
> > +
> > struct sbefifo kernel_sbefifo = {
> > .target = {
> > .name = "Kernel based FSI SBE FIFO",
> > @@ -510,10 +556,7 @@ struct sbefifo kernel_sbefifo = {
> > .probe = sbefifo_probe,
> > },
> > .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,
> > + .sreset_all = sbefifo_op_sreset_all,
> > .chipop = sbefifo_op_chipop,
> > .ffdc_get = sbefifo_ffdc_get,
> > .fd = -1,
> > @@ -525,4 +568,5 @@ static void register_sbefifo(void)
> > {
> > pdbg_hwunit_register(&kernel_sbefifo_hw_unit);
> > pdbg_hwunit_register(&sbefifo_mem_hw_unit);
> > + pdbg_hwunit_register(&sbefifo_thread_hw_unit);
> > }
>
> Amitay.
>
More information about the Pdbg
mailing list