| Message ID | 20210812093356.1946-11-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm: Get rid of system_clock_scale global | expand |
On 8/12/21 11:33 AM, Peter Maydell wrote: > In the realize methods of the stm32f100 and stm32f205 SoC objects, we > call g_new() to create new MemoryRegion objjects for the sram, flash, > and flash_alias. This is unnecessary (and leaves open the > possibility of leaking the allocations if we exit from realize with > an error). Make these MemoryRegions member fields of the device > state struct instead, as stm32f405 already does. There is a typo in "objjects". This is something I had issue understanding as I was seeing both patterns in the codebase, thank you for making this clear. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > include/hw/arm/stm32f100_soc.h | 4 ++++ > include/hw/arm/stm32f205_soc.h | 4 ++++ > hw/arm/stm32f100_soc.c | 17 +++++++---------- > hw/arm/stm32f205_soc.c | 17 +++++++---------- > 4 files changed, 22 insertions(+), 20 deletions(-) > > diff --git a/include/hw/arm/stm32f100_soc.h b/include/hw/arm/stm32f100_soc.h > index 71bffcf4fd5..b7d71c6c634 100644 > --- a/include/hw/arm/stm32f100_soc.h > +++ b/include/hw/arm/stm32f100_soc.h > @@ -52,6 +52,10 @@ struct STM32F100State { > > STM32F2XXUsartState usart[STM_NUM_USARTS]; > STM32F2XXSPIState spi[STM_NUM_SPIS]; > + > + MemoryRegion sram; > + MemoryRegion flash; > + MemoryRegion flash_alias; > }; > > #endif > diff --git a/include/hw/arm/stm32f205_soc.h b/include/hw/arm/stm32f205_soc.h > index 985ff63aa9e..75251494917 100644 > --- a/include/hw/arm/stm32f205_soc.h > +++ b/include/hw/arm/stm32f205_soc.h > @@ -63,6 +63,10 @@ struct STM32F205State { > STM32F2XXSPIState spi[STM_NUM_SPIS]; > > qemu_or_irq *adc_irqs; > + > + MemoryRegion sram; > + MemoryRegion flash; > + MemoryRegion flash_alias; > }; > > #endif > diff --git a/hw/arm/stm32f100_soc.c b/hw/arm/stm32f100_soc.c > index 0c4a5c66451..0be92b2c475 100644 > --- a/hw/arm/stm32f100_soc.c > +++ b/hw/arm/stm32f100_soc.c > @@ -67,25 +67,22 @@ static void stm32f100_soc_realize(DeviceState *dev_soc, Error **errp) > int i; > > MemoryRegion *system_memory = get_system_memory(); > - MemoryRegion *sram = g_new(MemoryRegion, 1); > - MemoryRegion *flash = g_new(MemoryRegion, 1); > - MemoryRegion *flash_alias = g_new(MemoryRegion, 1); > > /* > * Init flash region > * Flash starts at 0x08000000 and then is aliased to boot memory at 0x0 > */ > - memory_region_init_rom(flash, OBJECT(dev_soc), "STM32F100.flash", > + memory_region_init_rom(&s->flash, OBJECT(dev_soc), "STM32F100.flash", > FLASH_SIZE, &error_fatal); > - memory_region_init_alias(flash_alias, OBJECT(dev_soc), > - "STM32F100.flash.alias", flash, 0, FLASH_SIZE); > - memory_region_add_subregion(system_memory, FLASH_BASE_ADDRESS, flash); > - memory_region_add_subregion(system_memory, 0, flash_alias); > + memory_region_init_alias(&s->flash_alias, OBJECT(dev_soc), > + "STM32F100.flash.alias", &s->flash, 0, FLASH_SIZE); > + memory_region_add_subregion(system_memory, FLASH_BASE_ADDRESS, &s->flash); > + memory_region_add_subregion(system_memory, 0, &s->flash_alias); > > /* Init SRAM region */ > - memory_region_init_ram(sram, NULL, "STM32F100.sram", SRAM_SIZE, > + memory_region_init_ram(&s->sram, NULL, "STM32F100.sram", SRAM_SIZE, > &error_fatal); > - memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram); > + memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, &s->sram); > > /* Init ARMv7m */ > armv7m = DEVICE(&s->armv7m); > diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c > index 9cd41bf56da..0bd215aebd7 100644 > --- a/hw/arm/stm32f205_soc.c > +++ b/hw/arm/stm32f205_soc.c > @@ -84,21 +84,18 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp) > int i; > > MemoryRegion *system_memory = get_system_memory(); > - MemoryRegion *sram = g_new(MemoryRegion, 1); > - MemoryRegion *flash = g_new(MemoryRegion, 1); > - MemoryRegion *flash_alias = g_new(MemoryRegion, 1); > > - memory_region_init_rom(flash, OBJECT(dev_soc), "STM32F205.flash", > + memory_region_init_rom(&s->flash, OBJECT(dev_soc), "STM32F205.flash", > FLASH_SIZE, &error_fatal); > - memory_region_init_alias(flash_alias, OBJECT(dev_soc), > - "STM32F205.flash.alias", flash, 0, FLASH_SIZE); > + memory_region_init_alias(&s->flash_alias, OBJECT(dev_soc), > + "STM32F205.flash.alias", &s->flash, 0, FLASH_SIZE); > > - memory_region_add_subregion(system_memory, FLASH_BASE_ADDRESS, flash); > - memory_region_add_subregion(system_memory, 0, flash_alias); > + memory_region_add_subregion(system_memory, FLASH_BASE_ADDRESS, &s->flash); > + memory_region_add_subregion(system_memory, 0, &s->flash_alias); > > - memory_region_init_ram(sram, NULL, "STM32F205.sram", SRAM_SIZE, > + memory_region_init_ram(&s->sram, NULL, "STM32F205.sram", SRAM_SIZE, > &error_fatal); > - memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram); > + memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, &s->sram); > > armv7m = DEVICE(&s->armv7m); > qdev_prop_set_uint32(armv7m, "num-irq", 96); > Reviewed-by: Alexandre Iooss <erdnaxe@crans.org> Thanks, -- Alexandre
On Thu, 12 Aug 2021 at 13:13, Alexandre IOOSS <erdnaxe@crans.org> wrote: > > On 8/12/21 11:33 AM, Peter Maydell wrote: > > In the realize methods of the stm32f100 and stm32f205 SoC objects, we > > call g_new() to create new MemoryRegion objjects for the sram, flash, > > and flash_alias. This is unnecessary (and leaves open the > > possibility of leaking the allocations if we exit from realize with > > an error). Make these MemoryRegions member fields of the device > > state struct instead, as stm32f405 already does. > > There is a typo in "objjects". > > This is something I had issue understanding as I was seeing both > patterns in the codebase, thank you for making this clear. Basically if there's a struct that the MemoryRegion can live in then that's the best place for it. For some board-level code where we haven't needed to subclass MachineState there is no convenient struct, so we just g_new(). In a few places like this one the board-code pattern has been copied into an SoC object. -- PMM
On Thu, Aug 12, 2021 at 7:37 PM Peter Maydell <peter.maydell@linaro.org> wrote: > > In the realize methods of the stm32f100 and stm32f205 SoC objects, we > call g_new() to create new MemoryRegion objjects for the sram, flash, > and flash_alias. This is unnecessary (and leaves open the > possibility of leaking the allocations if we exit from realize with > an error). Make these MemoryRegions member fields of the device > state struct instead, as stm32f405 already does. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > include/hw/arm/stm32f100_soc.h | 4 ++++ > include/hw/arm/stm32f205_soc.h | 4 ++++ > hw/arm/stm32f100_soc.c | 17 +++++++---------- > hw/arm/stm32f205_soc.c | 17 +++++++---------- > 4 files changed, 22 insertions(+), 20 deletions(-) > > diff --git a/include/hw/arm/stm32f100_soc.h b/include/hw/arm/stm32f100_soc.h > index 71bffcf4fd5..b7d71c6c634 100644 > --- a/include/hw/arm/stm32f100_soc.h > +++ b/include/hw/arm/stm32f100_soc.h > @@ -52,6 +52,10 @@ struct STM32F100State { > > STM32F2XXUsartState usart[STM_NUM_USARTS]; > STM32F2XXSPIState spi[STM_NUM_SPIS]; > + > + MemoryRegion sram; > + MemoryRegion flash; > + MemoryRegion flash_alias; > }; > > #endif > diff --git a/include/hw/arm/stm32f205_soc.h b/include/hw/arm/stm32f205_soc.h > index 985ff63aa9e..75251494917 100644 > --- a/include/hw/arm/stm32f205_soc.h > +++ b/include/hw/arm/stm32f205_soc.h > @@ -63,6 +63,10 @@ struct STM32F205State { > STM32F2XXSPIState spi[STM_NUM_SPIS]; > > qemu_or_irq *adc_irqs; > + > + MemoryRegion sram; > + MemoryRegion flash; > + MemoryRegion flash_alias; > }; > > #endif > diff --git a/hw/arm/stm32f100_soc.c b/hw/arm/stm32f100_soc.c > index 0c4a5c66451..0be92b2c475 100644 > --- a/hw/arm/stm32f100_soc.c > +++ b/hw/arm/stm32f100_soc.c > @@ -67,25 +67,22 @@ static void stm32f100_soc_realize(DeviceState *dev_soc, Error **errp) > int i; > > MemoryRegion *system_memory = get_system_memory(); > - MemoryRegion *sram = g_new(MemoryRegion, 1); > - MemoryRegion *flash = g_new(MemoryRegion, 1); > - MemoryRegion *flash_alias = g_new(MemoryRegion, 1); > > /* > * Init flash region > * Flash starts at 0x08000000 and then is aliased to boot memory at 0x0 > */ > - memory_region_init_rom(flash, OBJECT(dev_soc), "STM32F100.flash", > + memory_region_init_rom(&s->flash, OBJECT(dev_soc), "STM32F100.flash", > FLASH_SIZE, &error_fatal); > - memory_region_init_alias(flash_alias, OBJECT(dev_soc), > - "STM32F100.flash.alias", flash, 0, FLASH_SIZE); > - memory_region_add_subregion(system_memory, FLASH_BASE_ADDRESS, flash); > - memory_region_add_subregion(system_memory, 0, flash_alias); > + memory_region_init_alias(&s->flash_alias, OBJECT(dev_soc), > + "STM32F100.flash.alias", &s->flash, 0, FLASH_SIZE); > + memory_region_add_subregion(system_memory, FLASH_BASE_ADDRESS, &s->flash); > + memory_region_add_subregion(system_memory, 0, &s->flash_alias); > > /* Init SRAM region */ > - memory_region_init_ram(sram, NULL, "STM32F100.sram", SRAM_SIZE, > + memory_region_init_ram(&s->sram, NULL, "STM32F100.sram", SRAM_SIZE, > &error_fatal); > - memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram); > + memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, &s->sram); > > /* Init ARMv7m */ > armv7m = DEVICE(&s->armv7m); > diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c > index 9cd41bf56da..0bd215aebd7 100644 > --- a/hw/arm/stm32f205_soc.c > +++ b/hw/arm/stm32f205_soc.c > @@ -84,21 +84,18 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp) > int i; > > MemoryRegion *system_memory = get_system_memory(); > - MemoryRegion *sram = g_new(MemoryRegion, 1); > - MemoryRegion *flash = g_new(MemoryRegion, 1); > - MemoryRegion *flash_alias = g_new(MemoryRegion, 1); > > - memory_region_init_rom(flash, OBJECT(dev_soc), "STM32F205.flash", > + memory_region_init_rom(&s->flash, OBJECT(dev_soc), "STM32F205.flash", > FLASH_SIZE, &error_fatal); > - memory_region_init_alias(flash_alias, OBJECT(dev_soc), > - "STM32F205.flash.alias", flash, 0, FLASH_SIZE); > + memory_region_init_alias(&s->flash_alias, OBJECT(dev_soc), > + "STM32F205.flash.alias", &s->flash, 0, FLASH_SIZE); > > - memory_region_add_subregion(system_memory, FLASH_BASE_ADDRESS, flash); > - memory_region_add_subregion(system_memory, 0, flash_alias); > + memory_region_add_subregion(system_memory, FLASH_BASE_ADDRESS, &s->flash); > + memory_region_add_subregion(system_memory, 0, &s->flash_alias); > > - memory_region_init_ram(sram, NULL, "STM32F205.sram", SRAM_SIZE, > + memory_region_init_ram(&s->sram, NULL, "STM32F205.sram", SRAM_SIZE, > &error_fatal); > - memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram); > + memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, &s->sram); > > armv7m = DEVICE(&s->armv7m); > qdev_prop_set_uint32(armv7m, "num-irq", 96); > -- > 2.20.1 > >
On 8/12/21 2:27 PM, Peter Maydell wrote: > On Thu, 12 Aug 2021 at 13:13, Alexandre IOOSS <erdnaxe@crans.org> wrote: >> >> On 8/12/21 11:33 AM, Peter Maydell wrote: >>> In the realize methods of the stm32f100 and stm32f205 SoC objects, we >>> call g_new() to create new MemoryRegion objjects for the sram, flash, >>> and flash_alias. This is unnecessary (and leaves open the >>> possibility of leaking the allocations if we exit from realize with >>> an error). Make these MemoryRegions member fields of the device >>> state struct instead, as stm32f405 already does. >> >> There is a typo in "objjects". >> >> This is something I had issue understanding as I was seeing both >> patterns in the codebase, thank you for making this clear. > > Basically if there's a struct that the MemoryRegion can live > in then that's the best place for it. For some board-level code > where we haven't needed to subclass MachineState there is no > convenient struct, so we just g_new(). In a few places like this > one the board-code pattern has been copied into an SoC object. FYI since more than 2 years now Igor recommends using DEFINE_TYPES() for newer QOM style, which makes adding board-specific fields to MachineState quite easy (see hw/avr/arduino.c or hw/arm/raspi.c).
On 10:33 Thu 12 Aug , Peter Maydell wrote: > In the realize methods of the stm32f100 and stm32f205 SoC objects, we > call g_new() to create new MemoryRegion objjects for the sram, flash, > and flash_alias. This is unnecessary (and leaves open the > possibility of leaking the allocations if we exit from realize with > an error). Make these MemoryRegions member fields of the device > state struct instead, as stm32f405 already does. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> (with the typo fixed) Reviewed-by: Luc Michel <luc@lmichel.fr> > --- > include/hw/arm/stm32f100_soc.h | 4 ++++ > include/hw/arm/stm32f205_soc.h | 4 ++++ > hw/arm/stm32f100_soc.c | 17 +++++++---------- > hw/arm/stm32f205_soc.c | 17 +++++++---------- > 4 files changed, 22 insertions(+), 20 deletions(-) > > diff --git a/include/hw/arm/stm32f100_soc.h b/include/hw/arm/stm32f100_soc.h > index 71bffcf4fd5..b7d71c6c634 100644 > --- a/include/hw/arm/stm32f100_soc.h > +++ b/include/hw/arm/stm32f100_soc.h > @@ -52,6 +52,10 @@ struct STM32F100State { > > STM32F2XXUsartState usart[STM_NUM_USARTS]; > STM32F2XXSPIState spi[STM_NUM_SPIS]; > + > + MemoryRegion sram; > + MemoryRegion flash; > + MemoryRegion flash_alias; > }; > > #endif > diff --git a/include/hw/arm/stm32f205_soc.h b/include/hw/arm/stm32f205_soc.h > index 985ff63aa9e..75251494917 100644 > --- a/include/hw/arm/stm32f205_soc.h > +++ b/include/hw/arm/stm32f205_soc.h > @@ -63,6 +63,10 @@ struct STM32F205State { > STM32F2XXSPIState spi[STM_NUM_SPIS]; > > qemu_or_irq *adc_irqs; > + > + MemoryRegion sram; > + MemoryRegion flash; > + MemoryRegion flash_alias; > }; > > #endif > diff --git a/hw/arm/stm32f100_soc.c b/hw/arm/stm32f100_soc.c > index 0c4a5c66451..0be92b2c475 100644 > --- a/hw/arm/stm32f100_soc.c > +++ b/hw/arm/stm32f100_soc.c > @@ -67,25 +67,22 @@ static void stm32f100_soc_realize(DeviceState *dev_soc, Error **errp) > int i; > > MemoryRegion *system_memory = get_system_memory(); > - MemoryRegion *sram = g_new(MemoryRegion, 1); > - MemoryRegion *flash = g_new(MemoryRegion, 1); > - MemoryRegion *flash_alias = g_new(MemoryRegion, 1); > > /* > * Init flash region > * Flash starts at 0x08000000 and then is aliased to boot memory at 0x0 > */ > - memory_region_init_rom(flash, OBJECT(dev_soc), "STM32F100.flash", > + memory_region_init_rom(&s->flash, OBJECT(dev_soc), "STM32F100.flash", > FLASH_SIZE, &error_fatal); > - memory_region_init_alias(flash_alias, OBJECT(dev_soc), > - "STM32F100.flash.alias", flash, 0, FLASH_SIZE); > - memory_region_add_subregion(system_memory, FLASH_BASE_ADDRESS, flash); > - memory_region_add_subregion(system_memory, 0, flash_alias); > + memory_region_init_alias(&s->flash_alias, OBJECT(dev_soc), > + "STM32F100.flash.alias", &s->flash, 0, FLASH_SIZE); > + memory_region_add_subregion(system_memory, FLASH_BASE_ADDRESS, &s->flash); > + memory_region_add_subregion(system_memory, 0, &s->flash_alias); > > /* Init SRAM region */ > - memory_region_init_ram(sram, NULL, "STM32F100.sram", SRAM_SIZE, > + memory_region_init_ram(&s->sram, NULL, "STM32F100.sram", SRAM_SIZE, > &error_fatal); > - memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram); > + memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, &s->sram); > > /* Init ARMv7m */ > armv7m = DEVICE(&s->armv7m); > diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c > index 9cd41bf56da..0bd215aebd7 100644 > --- a/hw/arm/stm32f205_soc.c > +++ b/hw/arm/stm32f205_soc.c > @@ -84,21 +84,18 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp) > int i; > > MemoryRegion *system_memory = get_system_memory(); > - MemoryRegion *sram = g_new(MemoryRegion, 1); > - MemoryRegion *flash = g_new(MemoryRegion, 1); > - MemoryRegion *flash_alias = g_new(MemoryRegion, 1); > > - memory_region_init_rom(flash, OBJECT(dev_soc), "STM32F205.flash", > + memory_region_init_rom(&s->flash, OBJECT(dev_soc), "STM32F205.flash", > FLASH_SIZE, &error_fatal); > - memory_region_init_alias(flash_alias, OBJECT(dev_soc), > - "STM32F205.flash.alias", flash, 0, FLASH_SIZE); > + memory_region_init_alias(&s->flash_alias, OBJECT(dev_soc), > + "STM32F205.flash.alias", &s->flash, 0, FLASH_SIZE); > > - memory_region_add_subregion(system_memory, FLASH_BASE_ADDRESS, flash); > - memory_region_add_subregion(system_memory, 0, flash_alias); > + memory_region_add_subregion(system_memory, FLASH_BASE_ADDRESS, &s->flash); > + memory_region_add_subregion(system_memory, 0, &s->flash_alias); > > - memory_region_init_ram(sram, NULL, "STM32F205.sram", SRAM_SIZE, > + memory_region_init_ram(&s->sram, NULL, "STM32F205.sram", SRAM_SIZE, > &error_fatal); > - memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram); > + memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, &s->sram); > > armv7m = DEVICE(&s->armv7m); > qdev_prop_set_uint32(armv7m, "num-irq", 96); > -- > 2.20.1 > --
diff --git a/include/hw/arm/stm32f100_soc.h b/include/hw/arm/stm32f100_soc.h index 71bffcf4fd5..b7d71c6c634 100644 --- a/include/hw/arm/stm32f100_soc.h +++ b/include/hw/arm/stm32f100_soc.h @@ -52,6 +52,10 @@ struct STM32F100State { STM32F2XXUsartState usart[STM_NUM_USARTS]; STM32F2XXSPIState spi[STM_NUM_SPIS]; + + MemoryRegion sram; + MemoryRegion flash; + MemoryRegion flash_alias; }; #endif diff --git a/include/hw/arm/stm32f205_soc.h b/include/hw/arm/stm32f205_soc.h index 985ff63aa9e..75251494917 100644 --- a/include/hw/arm/stm32f205_soc.h +++ b/include/hw/arm/stm32f205_soc.h @@ -63,6 +63,10 @@ struct STM32F205State { STM32F2XXSPIState spi[STM_NUM_SPIS]; qemu_or_irq *adc_irqs; + + MemoryRegion sram; + MemoryRegion flash; + MemoryRegion flash_alias; }; #endif diff --git a/hw/arm/stm32f100_soc.c b/hw/arm/stm32f100_soc.c index 0c4a5c66451..0be92b2c475 100644 --- a/hw/arm/stm32f100_soc.c +++ b/hw/arm/stm32f100_soc.c @@ -67,25 +67,22 @@ static void stm32f100_soc_realize(DeviceState *dev_soc, Error **errp) int i; MemoryRegion *system_memory = get_system_memory(); - MemoryRegion *sram = g_new(MemoryRegion, 1); - MemoryRegion *flash = g_new(MemoryRegion, 1); - MemoryRegion *flash_alias = g_new(MemoryRegion, 1); /* * Init flash region * Flash starts at 0x08000000 and then is aliased to boot memory at 0x0 */ - memory_region_init_rom(flash, OBJECT(dev_soc), "STM32F100.flash", + memory_region_init_rom(&s->flash, OBJECT(dev_soc), "STM32F100.flash", FLASH_SIZE, &error_fatal); - memory_region_init_alias(flash_alias, OBJECT(dev_soc), - "STM32F100.flash.alias", flash, 0, FLASH_SIZE); - memory_region_add_subregion(system_memory, FLASH_BASE_ADDRESS, flash); - memory_region_add_subregion(system_memory, 0, flash_alias); + memory_region_init_alias(&s->flash_alias, OBJECT(dev_soc), + "STM32F100.flash.alias", &s->flash, 0, FLASH_SIZE); + memory_region_add_subregion(system_memory, FLASH_BASE_ADDRESS, &s->flash); + memory_region_add_subregion(system_memory, 0, &s->flash_alias); /* Init SRAM region */ - memory_region_init_ram(sram, NULL, "STM32F100.sram", SRAM_SIZE, + memory_region_init_ram(&s->sram, NULL, "STM32F100.sram", SRAM_SIZE, &error_fatal); - memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram); + memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, &s->sram); /* Init ARMv7m */ armv7m = DEVICE(&s->armv7m); diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c index 9cd41bf56da..0bd215aebd7 100644 --- a/hw/arm/stm32f205_soc.c +++ b/hw/arm/stm32f205_soc.c @@ -84,21 +84,18 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp) int i; MemoryRegion *system_memory = get_system_memory(); - MemoryRegion *sram = g_new(MemoryRegion, 1); - MemoryRegion *flash = g_new(MemoryRegion, 1); - MemoryRegion *flash_alias = g_new(MemoryRegion, 1); - memory_region_init_rom(flash, OBJECT(dev_soc), "STM32F205.flash", + memory_region_init_rom(&s->flash, OBJECT(dev_soc), "STM32F205.flash", FLASH_SIZE, &error_fatal); - memory_region_init_alias(flash_alias, OBJECT(dev_soc), - "STM32F205.flash.alias", flash, 0, FLASH_SIZE); + memory_region_init_alias(&s->flash_alias, OBJECT(dev_soc), + "STM32F205.flash.alias", &s->flash, 0, FLASH_SIZE); - memory_region_add_subregion(system_memory, FLASH_BASE_ADDRESS, flash); - memory_region_add_subregion(system_memory, 0, flash_alias); + memory_region_add_subregion(system_memory, FLASH_BASE_ADDRESS, &s->flash); + memory_region_add_subregion(system_memory, 0, &s->flash_alias); - memory_region_init_ram(sram, NULL, "STM32F205.sram", SRAM_SIZE, + memory_region_init_ram(&s->sram, NULL, "STM32F205.sram", SRAM_SIZE, &error_fatal); - memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram); + memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, &s->sram); armv7m = DEVICE(&s->armv7m); qdev_prop_set_uint32(armv7m, "num-irq", 96);
In the realize methods of the stm32f100 and stm32f205 SoC objects, we call g_new() to create new MemoryRegion objjects for the sram, flash, and flash_alias. This is unnecessary (and leaves open the possibility of leaking the allocations if we exit from realize with an error). Make these MemoryRegions member fields of the device state struct instead, as stm32f405 already does. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/stm32f100_soc.h | 4 ++++ include/hw/arm/stm32f205_soc.h | 4 ++++ hw/arm/stm32f100_soc.c | 17 +++++++---------- hw/arm/stm32f205_soc.c | 17 +++++++---------- 4 files changed, 22 insertions(+), 20 deletions(-) -- 2.20.1