[Pdbg] [RFC 03/12] sbefifo: Rework thread functions
Amitay Isaacs
amitay at ozlabs.org
Tue Aug 20 15:25:23 AEST 2019
On Tue, 2019-08-20 at 14:55 +1000, Alistair Popple wrote:
> 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
As per the document the core-id is the pervasive chiplet id for the
core (0x20 - 0x37). So that's not quite the core index, but chiplet
index. Thread id is local to each core (0x0 - 0x3).
>
> > >
> > > 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.
> >
>
>
>
Amitay.
--
When you were born, you cried and world rejoiced. Live your life in such a
manner that when you die the world cries and you rejoice. - Old Indian
Saying
More information about the Pdbg
mailing list