[Skiboot] [PATCH 4/4] phb4: Eliminate peltv_cache
Oliver
oohall at gmail.com
Wed Nov 28 16:21:56 AEDT 2018
On Wed, Nov 28, 2018 at 4:01 PM Oliver <oohall at gmail.com> wrote:
>
> On Wed, Nov 28, 2018 at 3:48 PM Andrew Donnellan
> <andrew.donnellan at au1.ibm.com> wrote:
> >
> > On 27/11/18 5:21 pm, Oliver O'Halloran wrote:
> > > The PELT-V is also an in-memory table and there is no reason to have two
> > > copies of it. Removing the cache shaves another 128KB off the size of
> > > each struct phb4.
> > >
> > > Signed-off-by: Oliver O'Halloran <oohall at gmail.com>
> >
> > One comment below, otherwise
> >
> > Reviewed-by: Andrew Donnellan <andrew.donnellan at au1.ibm.com>
> >
> > > ---
> > > hw/phb4.c | 30 +++++++++++++-----------------
> > > include/phb4.h | 3 +--
> > > 2 files changed, 14 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/hw/phb4.c b/hw/phb4.c
> > > index 14fa94b50f75..c52e0b537e56 100644
> > > --- a/hw/phb4.c
> > > +++ b/hw/phb4.c
> > > @@ -934,7 +934,6 @@ static void phb4_init_ioda_cache(struct phb4 *p)
> > > * ever let a live FF RTT even temporarily when resetting
> > > * for EEH etc... (HW278969).
> > > */
> >
> > With this and the previous patch, how accurate is the above comment block?
>
> It would appear that this comment is a relic of the PHB3 code and it
> probably never applied to PHB4.
Actually, come to think of it the top half of that comment should be
moved into phb4_ioda_reset() where the table is actually initialised.
The "WARNING" bit is junk though.
>
> > > - memset(p->peltv_cache, 0x0, sizeof(p->peltv_cache));
> > > memset(p->tve_cache, 0x0, sizeof(p->tve_cache));
> > >
> > > /* XXX Should we mask them ? */
> > > @@ -1162,11 +1161,11 @@ static int64_t phb4_ioda_reset(struct phb *phb, bool purge)
> > >
> > > /* Additional OPAL specific inits */
> > >
> > > - /* Clear RTT and PELTV and PEST */
> > > + /* Clear RTT and PELTV */
> > > for (i = 0; i < RTT_TABLE_ENTRIES; i++)
> > > p->tbl_rtt[i] = PHB4_RESERVED_PE_NUM(p);
> > >
> > > - memcpy((void *)p->tbl_peltv, p->peltv_cache, p->tbl_peltv_size);
> > > + memset(p->tbl_peltv, 0x0, p->tbl_peltv_size);
> > >
> > > /* Clear PEST & PEEV */
> > > for (i = 0; i < p->max_num_pes; i++) {
> > > @@ -2206,7 +2205,6 @@ static int64_t phb4_set_peltv(struct phb *phb,
> > > uint8_t state)
> > > {
> > > struct phb4 *p = phb_to_phb4(phb);
> > > - uint8_t *peltv;
> > > uint32_t idx, mask;
> > >
> > > /* Sanity check */
> > > @@ -2218,15 +2216,10 @@ static int64_t phb4_set_peltv(struct phb *phb,
> > > idx += (child_pe / 8);
> > > mask = 0x1 << (7 - (child_pe % 8));
> > >
> > > - peltv = (uint8_t *)p->tbl_peltv;
> > > - peltv += idx;
> > > - if (state) {
> > > - *peltv |= mask;
> > > - p->peltv_cache[idx] |= mask;
> > > - } else {
> > > - *peltv &= ~mask;
> > > - p->peltv_cache[idx] &= ~mask;
> > > - }
> > > + if (state)
> > > + p->tbl_peltv[idx] |= mask;
> > > + else
> > > + p->tbl_peltv[idx] &= ~mask;
> > >
> > > return OPAL_SUCCESS;
> > > }
> > > @@ -4596,7 +4589,8 @@ static void phb4_init_ioda3(struct phb4 *p)
> > > out_be64(p->regs + PHB_RTT_BAR, (u64) p->tbl_rtt | PHB_RTT_BAR_ENABLE);
> > >
> > > /* Init_21 - PELT-V BAR */
> > > - out_be64(p->regs + PHB_PELTV_BAR, p->tbl_peltv | PHB_PELTV_BAR_ENABLE);
> > > + out_be64(p->regs + PHB_PELTV_BAR,
> > > + (u64) p->tbl_peltv | PHB_PELTV_BAR_ENABLE);
> > >
> > > /* Init_22 - Setup M32 starting address */
> > > out_be64(p->regs + PHB_M32_START_ADDR, M32_PCI_START);
> > > @@ -5121,9 +5115,9 @@ static void phb4_allocate_tables(struct phb4 *p)
> > > for (i = 0; i < RTT_TABLE_ENTRIES; i++)
> > > p->tbl_rtt[i] = PHB4_RESERVED_PE_NUM(p);
> > >
> > > - p->tbl_peltv = (uint64_t)local_alloc(p->chip_id, p->tbl_peltv_size, p->tbl_peltv_size);
> > > + p->tbl_peltv = local_alloc(p->chip_id, p->tbl_peltv_size, p->tbl_peltv_size);
> > > assert(p->tbl_peltv);
> > > - memset((void *)p->tbl_peltv, 0, p->tbl_peltv_size);
> > > + memset(p->tbl_peltv, 0, p->tbl_peltv_size);
> > >
> > > p->tbl_pest = (uint64_t)local_alloc(p->chip_id, p->tbl_pest_size, p->tbl_pest_size);
> > > assert(p->tbl_pest);
> > > @@ -5231,7 +5225,9 @@ static void phb4_add_properties(struct phb4 *p)
> > > hi32((u64) p->tbl_rtt), lo32((u64) p->tbl_rtt), RTT_TABLE_SIZE);
> > >
> > > dt_add_property_cells(np, "ibm,opal-peltv-table",
> > > - hi32(p->tbl_peltv), lo32(p->tbl_peltv), p->tbl_peltv_size);
> > > + hi32((u64) p->tbl_peltv), lo32((u64) p->tbl_peltv),
> > > + p->tbl_peltv_size);
> > > +
> > > dt_add_property_cells(np, "ibm,opal-pest-table",
> > > hi32(p->tbl_pest), lo32(p->tbl_pest), p->tbl_pest_size);
> > >
> > > diff --git a/include/phb4.h b/include/phb4.h
> > > index 6a5e9d5c8723..c70f713b2d63 100644
> > > --- a/include/phb4.h
> > > +++ b/include/phb4.h
> > > @@ -197,7 +197,7 @@ struct phb4 {
> > >
> > > /* SkiBoot owned in-memory tables */
> > > uint16_t *tbl_rtt;
> > > - uint64_t tbl_peltv;
> > > + uint8_t *tbl_peltv;
> > > uint64_t tbl_peltv_size;
> > > uint64_t tbl_pest;
> > > uint64_t tbl_pest_size;
> > > @@ -216,7 +216,6 @@ struct phb4 {
> > >
> > > /* FIXME: dynamically allocate only what's needed below */
> > > uint64_t tve_cache[1024];
> > > - uint8_t peltv_cache[PELTV_TABLE_SIZE_MAX];
> > > uint64_t mbt_cache[32][2];
> > > uint64_t mdt_cache[512]; /* max num of PEs */
> > > uint64_t mist_cache[4096/4];/* max num of MSIs */
> > >
> >
> > --
> > Andrew Donnellan OzLabs, ADL Canberra
> > andrew.donnellan at au1.ibm.com IBM Australia Limited
> >
More information about the Skiboot
mailing list