1. Patchew
  2. QEMU
  3. [PATCH 0/2] hw/arm: Add support for STM32 H405 and fix STM32F405 memory layout
  4. View patch

[PATCH 1/2] hw/arm/stm32f405: correctly describe the memory layout

Felipe Balbi posted 2 patches 3 years, 1 month ago
Maintainers: Peter Maydell <peter.maydell@linaro.org>, Felipe Balbi <balbi@kernel.org>, Alistair Francis <alistair@alistair23.me>
There is a newer version of this series
[PATCH 1/2] hw/arm/stm32f405: correctly describe the memory layout
Posted by Felipe Balbi 3 years, 1 month ago 
STM32F405 has 128K of SRAM and another 64K of CCM (Core-coupled
Memory) at a different base address. Correctly describe the memory
layout to give existing FW images have a chance to run unmodified.

Signed-off-by: Felipe Balbi <balbi@kernel.org>
---
 hw/arm/stm32f405_soc.c         | 8 ++++++++
 include/hw/arm/stm32f405_soc.h | 5 ++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/hw/arm/stm32f405_soc.c b/hw/arm/stm32f405_soc.c
index c07947d9f8b1..cef23d7ee41a 100644
--- a/hw/arm/stm32f405_soc.c
+++ b/hw/arm/stm32f405_soc.c
@@ -139,6 +139,14 @@ static void stm32f405_soc_realize(DeviceState *dev_soc, Error **errp)
     }
     memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, &s->sram);
 
+    memory_region_init_ram(&s->ccm, NULL, "STM32F405.ccm", CCM_SIZE,
+                           &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
+    }
+    memory_region_add_subregion(system_memory, CCM_BASE_ADDRESS, &s->ccm);
+
     armv7m = DEVICE(&s->armv7m);
     qdev_prop_set_uint32(armv7m, "num-irq", 96);
     qdev_prop_set_string(armv7m, "cpu-type", s->cpu_type);
diff --git a/include/hw/arm/stm32f405_soc.h b/include/hw/arm/stm32f405_soc.h
index 5bb0c8d56979..249ab5434ec7 100644
--- a/include/hw/arm/stm32f405_soc.h
+++ b/include/hw/arm/stm32f405_soc.h
@@ -46,7 +46,9 @@ OBJECT_DECLARE_SIMPLE_TYPE(STM32F405State, STM32F405_SOC)
 #define FLASH_BASE_ADDRESS 0x08000000
 #define FLASH_SIZE (1024 * 1024)
 #define SRAM_BASE_ADDRESS 0x20000000
-#define SRAM_SIZE (192 * 1024)
+#define SRAM_SIZE (128 * 1024)
+#define CCM_BASE_ADDRESS 0x10000000
+#define CCM_SIZE (64 * 1024)
 
 struct STM32F405State {
     /*< private >*/
@@ -65,6 +67,7 @@ struct STM32F405State {
     STM32F2XXADCState adc[STM_NUM_ADCS];
     STM32F2XXSPIState spi[STM_NUM_SPIS];
 
+    MemoryRegion ccm;
     MemoryRegion sram;
     MemoryRegion flash;
     MemoryRegion flash_alias;
-- 
2.38.1
Re: [PATCH 1/2] hw/arm/stm32f405: correctly describe the memory layout
Posted by Philippe Mathieu-Daudé 3 years, 1 month ago 
Hi Felipe,

On 18/12/22 08:12, Felipe Balbi wrote:
> STM32F405 has 128K of SRAM and another 64K of CCM (Core-coupled
> Memory) at a different base address. Correctly describe the memory
> layout to give existing FW images have a chance to run unmodified.
> 
> Signed-off-by: Felipe Balbi <balbi@kernel.org>
> ---
>   hw/arm/stm32f405_soc.c         | 8 ++++++++
>   include/hw/arm/stm32f405_soc.h | 5 ++++-
>   2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/stm32f405_soc.c b/hw/arm/stm32f405_soc.c
> index c07947d9f8b1..cef23d7ee41a 100644
> --- a/hw/arm/stm32f405_soc.c
> +++ b/hw/arm/stm32f405_soc.c
> @@ -139,6 +139,14 @@ static void stm32f405_soc_realize(DeviceState *dev_soc, Error **errp)
>       }
>       memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, &s->sram);
>   
> +    memory_region_init_ram(&s->ccm, NULL, "STM32F405.ccm", CCM_SIZE,

Including the machine name in the memory description seems a bad
habit from old days. What do you think about renaming as
'core-coupled-memory'?

> +                           &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +    memory_region_add_subregion(system_memory, CCM_BASE_ADDRESS, &s->ccm);
> +
>       armv7m = DEVICE(&s->armv7m);
>       qdev_prop_set_uint32(armv7m, "num-irq", 96);
>       qdev_prop_set_string(armv7m, "cpu-type", s->cpu_type);
> diff --git a/include/hw/arm/stm32f405_soc.h b/include/hw/arm/stm32f405_soc.h
> index 5bb0c8d56979..249ab5434ec7 100644
> --- a/include/hw/arm/stm32f405_soc.h
> +++ b/include/hw/arm/stm32f405_soc.h
> @@ -46,7 +46,9 @@ OBJECT_DECLARE_SIMPLE_TYPE(STM32F405State, STM32F405_SOC)
>   #define FLASH_BASE_ADDRESS 0x08000000
>   #define FLASH_SIZE (1024 * 1024)
>   #define SRAM_BASE_ADDRESS 0x20000000
> -#define SRAM_SIZE (192 * 1024)
> +#define SRAM_SIZE (128 * 1024)
> +#define CCM_BASE_ADDRESS 0x10000000
> +#define CCM_SIZE (64 * 1024)

Since the CCM_SIZE won't be used elsewhere, we can simply use '64 * KiB'
in the memory_region_init_ram() in the source file. Up to the maintainer
:)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Re: [PATCH 1/2] hw/arm/stm32f405: correctly describe the memory layout
Posted by Felipe Balbi 3 years, 1 month ago 
Hi,

Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> On 18/12/22 08:12, Felipe Balbi wrote:
>> STM32F405 has 128K of SRAM and another 64K of CCM (Core-coupled
>> Memory) at a different base address. Correctly describe the memory
>> layout to give existing FW images have a chance to run unmodified.
>> 
>> Signed-off-by: Felipe Balbi <balbi@kernel.org>
>> ---
>>   hw/arm/stm32f405_soc.c         | 8 ++++++++
>>   include/hw/arm/stm32f405_soc.h | 5 ++++-
>>   2 files changed, 12 insertions(+), 1 deletion(-)
>> 
>> diff --git a/hw/arm/stm32f405_soc.c b/hw/arm/stm32f405_soc.c
>> index c07947d9f8b1..cef23d7ee41a 100644
>> --- a/hw/arm/stm32f405_soc.c
>> +++ b/hw/arm/stm32f405_soc.c
>> @@ -139,6 +139,14 @@ static void stm32f405_soc_realize(DeviceState *dev_soc, Error **errp)
>>       }
>>       memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, &s->sram);
>>   
>> +    memory_region_init_ram(&s->ccm, NULL, "STM32F405.ccm", CCM_SIZE,
>
> Including the machine name in the memory description seems a bad
> habit from old days. What do you think about renaming as
> 'core-coupled-memory'?

I don't oppose it, but I was merely following the model from
`netduino2plus'.

>> +                           &err);
>> +    if (err != NULL) {
>> +        error_propagate(errp, err);
>> +        return;
>> +    }
>> +    memory_region_add_subregion(system_memory, CCM_BASE_ADDRESS, &s->ccm);
>> +
>>       armv7m = DEVICE(&s->armv7m);
>>       qdev_prop_set_uint32(armv7m, "num-irq", 96);
>>       qdev_prop_set_string(armv7m, "cpu-type", s->cpu_type);
>> diff --git a/include/hw/arm/stm32f405_soc.h b/include/hw/arm/stm32f405_soc.h
>> index 5bb0c8d56979..249ab5434ec7 100644
>> --- a/include/hw/arm/stm32f405_soc.h
>> +++ b/include/hw/arm/stm32f405_soc.h
>> @@ -46,7 +46,9 @@ OBJECT_DECLARE_SIMPLE_TYPE(STM32F405State, STM32F405_SOC)
>>   #define FLASH_BASE_ADDRESS 0x08000000
>>   #define FLASH_SIZE (1024 * 1024)
>>   #define SRAM_BASE_ADDRESS 0x20000000
>> -#define SRAM_SIZE (192 * 1024)
>> +#define SRAM_SIZE (128 * 1024)
>> +#define CCM_BASE_ADDRESS 0x10000000
>> +#define CCM_SIZE (64 * 1024)
>
> Since the CCM_SIZE won't be used elsewhere, we can simply use '64 * KiB'
> in the memory_region_init_ram() in the source file. Up to the maintainer
> :)

Also not opposed. I'll wait a couple days before respinning the patch to
give the rest of the community time to reply.

-- 
balbi
Re: [PATCH 1/2] hw/arm/stm32f405: correctly describe the memory layout
Posted by Alistair Francis 3 years, 1 month ago 
On Mon, Dec 19, 2022 at 1:24 AM Felipe Balbi <balbi@kernel.org> wrote:
>
> STM32F405 has 128K of SRAM and another 64K of CCM (Core-coupled
> Memory) at a different base address. Correctly describe the memory
> layout to give existing FW images have a chance to run unmodified.
>
> Signed-off-by: Felipe Balbi <balbi@kernel.org>

Reviewed-by: Alistair Francis <alistair@alistair23.me>

Alistair

> ---
>  hw/arm/stm32f405_soc.c         | 8 ++++++++
>  include/hw/arm/stm32f405_soc.h | 5 ++++-
>  2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/stm32f405_soc.c b/hw/arm/stm32f405_soc.c
> index c07947d9f8b1..cef23d7ee41a 100644
> --- a/hw/arm/stm32f405_soc.c
> +++ b/hw/arm/stm32f405_soc.c
> @@ -139,6 +139,14 @@ static void stm32f405_soc_realize(DeviceState *dev_soc, Error **errp)
>      }
>      memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, &s->sram);
>
> +    memory_region_init_ram(&s->ccm, NULL, "STM32F405.ccm", CCM_SIZE,
> +                           &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +    memory_region_add_subregion(system_memory, CCM_BASE_ADDRESS, &s->ccm);
> +
>      armv7m = DEVICE(&s->armv7m);
>      qdev_prop_set_uint32(armv7m, "num-irq", 96);
>      qdev_prop_set_string(armv7m, "cpu-type", s->cpu_type);
> diff --git a/include/hw/arm/stm32f405_soc.h b/include/hw/arm/stm32f405_soc.h
> index 5bb0c8d56979..249ab5434ec7 100644
> --- a/include/hw/arm/stm32f405_soc.h
> +++ b/include/hw/arm/stm32f405_soc.h
> @@ -46,7 +46,9 @@ OBJECT_DECLARE_SIMPLE_TYPE(STM32F405State, STM32F405_SOC)
>  #define FLASH_BASE_ADDRESS 0x08000000
>  #define FLASH_SIZE (1024 * 1024)
>  #define SRAM_BASE_ADDRESS 0x20000000
> -#define SRAM_SIZE (192 * 1024)
> +#define SRAM_SIZE (128 * 1024)
> +#define CCM_BASE_ADDRESS 0x10000000
> +#define CCM_SIZE (64 * 1024)
>
>  struct STM32F405State {
>      /*< private >*/
> @@ -65,6 +67,7 @@ struct STM32F405State {
>      STM32F2XXADCState adc[STM_NUM_ADCS];
>      STM32F2XXSPIState spi[STM_NUM_SPIS];
>
> +    MemoryRegion ccm;
>      MemoryRegion sram;
>      MemoryRegion flash;
>      MemoryRegion flash_alias;
> --
> 2.38.1
>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.