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

[Philippe Mathieu-Daudé]

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 ?

> 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);
>   }
>