[Pdbg] [RFC 03/12] sbefifo: Rework thread functions
Alistair Popple
alistair at popple.id.au
Tue Aug 20 15:37:50 AEST 2019
On Tuesday, 20 August 2019 3:25:23 PM AEST Amitay Isaacs wrote:
> 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).
Ok, thanks. It looks like at the moment we have incorrect chiplet indices at
the moment so I will fix that up and get rid of the extra fields.
- 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.
> > >
> >
> >
> >
>
> Amitay.
>
More information about the Pdbg
mailing list