Re: [PULL 04/16] aspeed/smc: Cache AspeedSMCClass

[Cédric Le Goater]

Hello,

On 5/12/23 06:00, Philippe Mathieu-Daudé wrote:
> Hi Cédric,
>
> On 25/10/22 17:20, Cédric Le Goater wrote:
> > Store a reference on the AspeedSMC class under the flash object and
> > use it when accessing the flash contents. Avoiding the class cast
> > checkers in these hot paths improves performance by 10% when running
> > the aspeed avocado tests.
>
> I doubt you still have your benchmark number, but do you remember
> if you were using --enable-qom-cast-debug ?

It is relatively easy to run.

Grab :

        
https://github.com/legoater/qemu-aspeed-boot/raw/master/images/ast2500-evb/buildroot-2023.02/flash.img

and run :

        qemu-system-arm -M ast2500-evb,execute-in-place=true -net user -drive 
file=./flash.img,format=raw,if=mtd -nographic

I tried with and without --enable-qom-cast-debug. It doesn't make
much difference.

C.


> > Message-Id: <20220923084803.498337-7-clg@kaod.org>
> > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > ---
> >   include/hw/ssi/aspeed_smc.h | 2 ++
> >   hw/ssi/aspeed_smc.c         | 9 ++++-----
> >   2 files changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
> > index 2d5f8f3d8f68..8e1dda556b91 100644
> > --- a/include/hw/ssi/aspeed_smc.h
> > +++ b/include/hw/ssi/aspeed_smc.h
> > @@ -30,6 +30,7 @@
> >   #include "qom/object.h"
> >   struct AspeedSMCState;
> > +struct AspeedSMCClass;
> >   #define TYPE_ASPEED_SMC_FLASH "aspeed.smc.flash"
> >   OBJECT_DECLARE_SIMPLE_TYPE(AspeedSMCFlash, ASPEED_SMC_FLASH)
> > @@ -37,6 +38,7 @@ struct AspeedSMCFlash {
> >       SysBusDevice parent_obj;
> >       struct AspeedSMCState *controller;
> > +    struct AspeedSMCClass *asc;
> >       uint8_t cs;
> >       MemoryRegion mmio;
> > diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> > index faed7e0cbe17..22df4be528a7 100644
> > --- a/hw/ssi/aspeed_smc.c
> > +++ b/hw/ssi/aspeed_smc.c
> > @@ -388,7 +388,7 @@ static inline int aspeed_smc_flash_cmd(const AspeedSMCFlash 
> > *fl)
> >   static inline int aspeed_smc_flash_addr_width(const AspeedSMCFlash *fl)
> >   {
> >       const AspeedSMCState *s = fl->controller;
> > -    AspeedSMCClass *asc = ASPEED_SMC_GET_CLASS(s);
> > +    AspeedSMCClass *asc = fl->asc;
> >       if (asc->addr_width) {
> >           return asc->addr_width(s);
> > @@ -420,7 +420,7 @@ static uint32_t aspeed_smc_check_segment_addr(const 
> > AspeedSMCFlash *fl,
> >                                                 uint32_t addr)
> >   {
> >       const AspeedSMCState *s = fl->controller;
> > -    AspeedSMCClass *asc = ASPEED_SMC_GET_CLASS(s);
> > +    AspeedSMCClass *asc = fl->asc;
> >       AspeedSegments seg;
> >       asc->reg_to_segment(s, s->regs[R_SEG_ADDR0 + fl->cs], &seg);
> > @@ -1234,7 +1234,6 @@ static const TypeInfo aspeed_smc_info = {
> >   static void aspeed_smc_flash_realize(DeviceState *dev, Error **errp)
> >   {
> >       AspeedSMCFlash *s = ASPEED_SMC_FLASH(dev);
> > -    AspeedSMCClass *asc;
> >       g_autofree char *name = g_strdup_printf(TYPE_ASPEED_SMC_FLASH ".%d", 
> > s->cs);
> >       if (!s->controller) {
> > @@ -1242,14 +1241,14 @@ static void aspeed_smc_flash_realize(DeviceState *dev, 
> > Error **errp)
> >           return;
> >       }
> > -    asc = ASPEED_SMC_GET_CLASS(s->controller);
> > +    s->asc = ASPEED_SMC_GET_CLASS(s->controller);
> >       /*
> >        * Use the default segment value to size the memory region. This
> >        * can be changed by FW at runtime.
> >        */
> >       memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_smc_flash_ops,
> > -                          s, name, asc->segments[s->cs].size);
> > +                          s, name, s->asc->segments[s->cs].size);
> >       sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mmio);
> >   }