| Message ID | 20221025152042.278287-5-clg@kaod.org |
|---|---|
| State | New |
| Headers | show |
| Series | [PULL,01/16] hw/i2c/aspeed: Fix old reg slave receive | expand |
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); > } >
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); >> } >
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); }
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. 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(-)