* [PATCH 0/2] platform: improve heap size handle @ 2024-09-16 7:24 Inochi Amaoto 2024-09-16 7:25 ` [PATCH 1/2] platform: ensure enough heap size with debug triggers extension Inochi Amaoto 2024-09-16 7:25 ` [PATCH 2/2] platform: introduce DT-based configurable heap size Inochi Amaoto 0 siblings, 2 replies; 6+ messages in thread From: Inochi Amaoto @ 2024-09-16 7:24 UTC (permalink / raw) To: opensbi This patch is split into two parts: 1. the first part is used to adjust the default heap size and make it more driven by the CPU as most allocation is percpu. I think this could reduce the further enlarge for the default heap size. 2. the second part is introduce a DT-based "heap-size", so the platform can set the suitable heap-size. This is serve as a more generic solution for some platform. Inochi Amaoto (2): platform: ensure enough heap size with debug triggers extension platform: introduce DT-based configurable heap size docs/opensbi_config.md | 4 ++++ include/sbi/sbi_platform.h | 2 +- platform/generic/platform.c | 28 +++++++++++++++++++++++++--- 3 files changed, 30 insertions(+), 4 deletions(-) -- 2.46.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] platform: ensure enough heap size with debug triggers extension 2024-09-16 7:24 [PATCH 0/2] platform: improve heap size handle Inochi Amaoto @ 2024-09-16 7:25 ` Inochi Amaoto 2024-09-20 5:33 ` Xiang W 2024-09-16 7:25 ` [PATCH 2/2] platform: introduce DT-based configurable heap size Inochi Amaoto 1 sibling, 1 reply; 6+ messages in thread From: Inochi Amaoto @ 2024-09-16 7:25 UTC (permalink / raw) To: opensbi DBTR introduce a big heap allocation to store hart state, this is allocated percpu and needs big space when CPU number is high. Increase the percpu part to fix this problem, and decrease the fixed part to avoid too big heap size. Fixes: 97f234f (lib: sbi: Introduce the SBI debug triggers extension support) Signed-off-by: Inochi Amaoto <inochiama@gmail.com> --- include/sbi/sbi_platform.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h index 7b3ac4b..d1c556c 100644 --- a/include/sbi/sbi_platform.h +++ b/include/sbi/sbi_platform.h @@ -152,7 +152,7 @@ struct sbi_platform_operations { /** Platform default heap size */ #define SBI_PLATFORM_DEFAULT_HEAP_SIZE(__num_hart) \ - (0x8000 + 0x800 * (__num_hart)) + (0x4000 + 0x2000 * (__num_hart)) /** Representation of a platform */ struct sbi_platform { -- 2.46.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 1/2] platform: ensure enough heap size with debug triggers extension 2024-09-16 7:25 ` [PATCH 1/2] platform: ensure enough heap size with debug triggers extension Inochi Amaoto @ 2024-09-20 5:33 ` Xiang W 0 siblings, 0 replies; 6+ messages in thread From: Xiang W @ 2024-09-20 5:33 UTC (permalink / raw) To: opensbi ? 2024-09-16???? 15:25 +0800?Inochi Amaoto??? > DBTR introduce a big heap allocation to store hart state, this is > allocated percpu and needs big space when CPU number is high. > > Increase the percpu part to fix this problem, and decrease the > fixed part to avoid too big heap size. > > Fixes: 97f234f (lib: sbi: Introduce the SBI debug triggers extension support) > Signed-off-by: Inochi Amaoto <inochiama@gmail.com> LGTM Reviewed-by: Xiang W <wxjstz@126.com> > --- > ?include/sbi/sbi_platform.h | 2 +- > ?1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h > index 7b3ac4b..d1c556c 100644 > --- a/include/sbi/sbi_platform.h > +++ b/include/sbi/sbi_platform.h > @@ -152,7 +152,7 @@ struct sbi_platform_operations { > ? > ?/** Platform default heap size */ > ?#define SBI_PLATFORM_DEFAULT_HEAP_SIZE(__num_hart) \ > - (0x8000 + 0x800 * (__num_hart)) > + (0x4000 + 0x2000 * (__num_hart)) > ? > ?/** Representation of a platform */ > ?struct sbi_platform { ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] platform: introduce DT-based configurable heap size 2024-09-16 7:24 [PATCH 0/2] platform: improve heap size handle Inochi Amaoto 2024-09-16 7:25 ` [PATCH 1/2] platform: ensure enough heap size with debug triggers extension Inochi Amaoto @ 2024-09-16 7:25 ` Inochi Amaoto 2024-09-20 5:33 ` Xiang W 1 sibling, 1 reply; 6+ messages in thread From: Inochi Amaoto @ 2024-09-16 7:25 UTC (permalink / raw) To: opensbi The default heap size will work for most platforms, but for some special platforms, the heap is too small to hold all the information or is too big so that it take too much ram. Introduce configurable heap should solve this problem and make all generic platforms happy. Add DT-based heap-size for the generic platform. Signed-off-by: Inochi Amaoto <inochiama@gmail.com> --- docs/opensbi_config.md | 4 ++++ platform/generic/platform.c | 28 +++++++++++++++++++++++++--- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/docs/opensbi_config.md b/docs/opensbi_config.md index 1b710f1..c620379 100644 --- a/docs/opensbi_config.md +++ b/docs/opensbi_config.md @@ -23,6 +23,9 @@ The DT properties of a domain configuration DT node are as follows: set of harts is permitted to perform a cold boot. Otherwise, all harts are allowed to cold boot. +* **heap-size** (Optional) - When present, the SBI will initialize + the heap with the set value (This value is in byte). + * **system-suspend-test** (Optional) - When present, enable a system suspend test implementation which simply waits five seconds and issues a WFI. @@ -36,6 +39,7 @@ The OpenSBI Configuration Node will be deleted at the end of cold boot opensbi-config { compatible = "opensbi,config"; cold-boot-harts = <&cpu1 &cpu2 &cpu3 &cpu4>; + heap-size = <0x400000>; system-suspend-test; }; }; diff --git a/platform/generic/platform.c b/platform/generic/platform.c index 49d877d..c8e988e 100644 --- a/platform/generic/platform.c +++ b/platform/generic/platform.c @@ -69,6 +69,28 @@ static u32 fw_platform_calculate_heap_size(u32 hart_count) return BIT_ALIGN(heap_size, HEAP_BASE_ALIGN); } +static u32 fw_platform_get_heap_size(const void *fdt, u32 hart_count) +{ + int chosen_offset, config_offset, len; + const fdt32_t *val; + + /* Get the heap size from device tree */ + chosen_offset = fdt_path_offset(fdt, "/chosen"); + if (chosen_offset < 0) + goto default_config; + + config_offset = fdt_node_offset_by_compatible(fdt, chosen_offset, "opensbi,config"); + if (config_offset < 0) + goto default_config; + + val = (fdt32_t *)fdt_getprop(fdt, config_offset, "heap-size", &len); + if (len > 0 && val) + return fdt32_to_cpu(*val); + +default_config: + return fw_platform_calculate_heap_size(hart_count); +} + extern struct sbi_platform platform; static bool platform_has_mlevel_imsic = false; static u32 generic_hart_index2id[SBI_HARTMASK_MAX_BITS] = { 0 }; @@ -76,9 +98,9 @@ static u32 generic_hart_index2id[SBI_HARTMASK_MAX_BITS] = { 0 }; static DECLARE_BITMAP(generic_coldboot_harts, SBI_HARTMASK_MAX_BITS); /* - * The fw_platform_coldboot_harts_init() function is called by fw_platform_init() + * The fw_platform_coldboot_harts_init() function is called by fw_platform_init() * function to initialize the cold boot harts allowed by the generic platform - * according to the DT property "cold-boot-harts" in "/chosen/opensbi-config" + * according to the DT property "cold-boot-harts" in "/chosen/opensbi-config" * DT node. If there is no "cold-boot-harts" in DT, all harts will be allowed. */ static void fw_platform_coldboot_harts_init(const void *fdt) @@ -185,7 +207,7 @@ unsigned long fw_platform_init(unsigned long arg0, unsigned long arg1, } platform.hart_count = hart_count; - platform.heap_size = fw_platform_calculate_heap_size(hart_count); + platform.heap_size = fw_platform_get_heap_size(fdt, hart_count); platform_has_mlevel_imsic = fdt_check_imsic_mlevel(fdt); fw_platform_coldboot_harts_init(fdt); -- 2.46.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] platform: introduce DT-based configurable heap size 2024-09-16 7:25 ` [PATCH 2/2] platform: introduce DT-based configurable heap size Inochi Amaoto @ 2024-09-20 5:33 ` Xiang W 2024-09-20 7:40 ` Inochi Amaoto 0 siblings, 1 reply; 6+ messages in thread From: Xiang W @ 2024-09-20 5:33 UTC (permalink / raw) To: opensbi ? 2024-09-16???? 15:25 +0800?Inochi Amaoto??? > The default heap size will work for most platforms, but for some > special platforms, the heap is too small to hold all the information > or is too big so that it take too much ram. Introduce configurable > heap should solve this problem and make all generic platforms happy. > > Add DT-based heap-size for the generic platform. > > Signed-off-by: Inochi Amaoto <inochiama@gmail.com> LGTM Reviewed-by: Xiang W <wxjstz@126.com> > --- > ?docs/opensbi_config.md????? |? 4 ++++ > ?platform/generic/platform.c | 28 +++++++++++++++++++++++++--- > ?2 files changed, 29 insertions(+), 3 deletions(-) > > diff --git a/docs/opensbi_config.md b/docs/opensbi_config.md > index 1b710f1..c620379 100644 > --- a/docs/opensbi_config.md > +++ b/docs/opensbi_config.md > @@ -23,6 +23,9 @@ The DT properties of a domain configuration DT node are as follows: > ?? set of harts is permitted to perform a cold boot. Otherwise, all > ?? harts are allowed to cold boot. > ? > +* **heap-size** (Optional) - When present, the SBI will initialize > +? the heap with the set value (This value is in byte). > + > ?* **system-suspend-test** (Optional) - When present, enable a system > ?? suspend test implementation which simply waits five seconds and issues a WFI. > ? > @@ -36,6 +39,7 @@ The OpenSBI Configuration Node will be deleted at the end of cold boot > ???????? opensbi-config { > ???????????? compatible = "opensbi,config"; > ???????????? cold-boot-harts = <&cpu1 &cpu2 &cpu3 &cpu4>; > +??????????? heap-size = <0x400000>; > ???????????? system-suspend-test; > ???????? }; > ???? }; > diff --git a/platform/generic/platform.c b/platform/generic/platform.c > index 49d877d..c8e988e 100644 > --- a/platform/generic/platform.c > +++ b/platform/generic/platform.c > @@ -69,6 +69,28 @@ static u32 fw_platform_calculate_heap_size(u32 hart_count) > ? return BIT_ALIGN(heap_size, HEAP_BASE_ALIGN); > ?} > ? > +static u32 fw_platform_get_heap_size(const void *fdt, u32 hart_count) > +{ > + int chosen_offset, config_offset, len; > + const fdt32_t *val; > + > + /* Get the heap size from device tree */ > + chosen_offset = fdt_path_offset(fdt, "/chosen"); > + if (chosen_offset < 0) > + goto default_config; > + > + config_offset = fdt_node_offset_by_compatible(fdt, chosen_offset, "opensbi,config"); > + if (config_offset < 0) > + goto default_config; > + > + val = (fdt32_t *)fdt_getprop(fdt, config_offset, "heap-size", &len); > + if (len > 0 && val) > + return fdt32_to_cpu(*val); > + > +default_config: > + return fw_platform_calculate_heap_size(hart_count); > +} > + > ?extern struct sbi_platform platform; > ?static bool platform_has_mlevel_imsic = false; > ?static u32 generic_hart_index2id[SBI_HARTMASK_MAX_BITS] = { 0 }; > @@ -76,9 +98,9 @@ static u32 generic_hart_index2id[SBI_HARTMASK_MAX_BITS] = { 0 }; > ?static DECLARE_BITMAP(generic_coldboot_harts, SBI_HARTMASK_MAX_BITS); > ? > ?/* > - * The fw_platform_coldboot_harts_init() function is called by fw_platform_init() > + * The fw_platform_coldboot_harts_init() function is called by fw_platform_init() > ? * function to initialize the cold boot harts allowed by the generic platform > - * according to the DT property "cold-boot-harts" in "/chosen/opensbi-config" > + * according to the DT property "cold-boot-harts" in "/chosen/opensbi-config" > ? * DT node. If there is no "cold-boot-harts" in DT, all harts will be allowed. > ? */ > ?static void fw_platform_coldboot_harts_init(const void *fdt) > @@ -185,7 +207,7 @@ unsigned long fw_platform_init(unsigned long arg0, unsigned long arg1, > ? } > ? > ? platform.hart_count = hart_count; > - platform.heap_size = fw_platform_calculate_heap_size(hart_count); > + platform.heap_size = fw_platform_get_heap_size(fdt, hart_count); > ? platform_has_mlevel_imsic = fdt_check_imsic_mlevel(fdt); > ? > ? fw_platform_coldboot_harts_init(fdt); ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] platform: introduce DT-based configurable heap size 2024-09-20 5:33 ` Xiang W @ 2024-09-20 7:40 ` Inochi Amaoto 0 siblings, 0 replies; 6+ messages in thread From: Inochi Amaoto @ 2024-09-20 7:40 UTC (permalink / raw) To: opensbi On Fri, Sep 20, 2024 at 01:33:51PM GMT, Xiang W wrote: > ? 2024-09-16???? 15:25 +0800?Inochi Amaoto??? > > The default heap size will work for most platforms, but for some > > special platforms, the heap is too small to hold all the information > > or is too big so that it take too much ram. Introduce configurable > > heap should solve this problem and make all generic platforms happy. > > > > Add DT-based heap-size for the generic platform. > > > > Signed-off-by: Inochi Amaoto <inochiama@gmail.com> > LGTM > > Reviewed-by: Xiang W <wxjstz@126.com> As I have found I forgot to set BIT_ALIGN, a new version is sent to fix it. Can your re-review the new version? Sorry for this inconvenience. Regards, Inochi > > --- > > ?docs/opensbi_config.md????? |? 4 ++++ > > ?platform/generic/platform.c | 28 +++++++++++++++++++++++++--- > > ?2 files changed, 29 insertions(+), 3 deletions(-) > > > > diff --git a/docs/opensbi_config.md b/docs/opensbi_config.md > > index 1b710f1..c620379 100644 > > --- a/docs/opensbi_config.md > > +++ b/docs/opensbi_config.md > > @@ -23,6 +23,9 @@ The DT properties of a domain configuration DT node are as follows: > > ?? set of harts is permitted to perform a cold boot. Otherwise, all > > ?? harts are allowed to cold boot. > > ? > > +* **heap-size** (Optional) - When present, the SBI will initialize > > +? the heap with the set value (This value is in byte). > > + > > ?* **system-suspend-test** (Optional) - When present, enable a system > > ?? suspend test implementation which simply waits five seconds and issues a WFI. > > ? > > @@ -36,6 +39,7 @@ The OpenSBI Configuration Node will be deleted at the end of cold boot > > ???????? opensbi-config { > > ???????????? compatible = "opensbi,config"; > > ???????????? cold-boot-harts = <&cpu1 &cpu2 &cpu3 &cpu4>; > > +??????????? heap-size = <0x400000>; > > ???????????? system-suspend-test; > > ???????? }; > > ???? }; > > diff --git a/platform/generic/platform.c b/platform/generic/platform.c > > index 49d877d..c8e988e 100644 > > --- a/platform/generic/platform.c > > +++ b/platform/generic/platform.c > > @@ -69,6 +69,28 @@ static u32 fw_platform_calculate_heap_size(u32 hart_count) > > ? return BIT_ALIGN(heap_size, HEAP_BASE_ALIGN); > > ?} > > ? > > +static u32 fw_platform_get_heap_size(const void *fdt, u32 hart_count) > > +{ > > + int chosen_offset, config_offset, len; > > + const fdt32_t *val; > > + > > + /* Get the heap size from device tree */ > > + chosen_offset = fdt_path_offset(fdt, "/chosen"); > > + if (chosen_offset < 0) > > + goto default_config; > > + > > + config_offset = fdt_node_offset_by_compatible(fdt, chosen_offset, "opensbi,config"); > > + if (config_offset < 0) > > + goto default_config; > > + > > + val = (fdt32_t *)fdt_getprop(fdt, config_offset, "heap-size", &len); > > + if (len > 0 && val) > > + return fdt32_to_cpu(*val); > > + > > +default_config: > > + return fw_platform_calculate_heap_size(hart_count); > > +} > > + > > ?extern struct sbi_platform platform; > > ?static bool platform_has_mlevel_imsic = false; > > ?static u32 generic_hart_index2id[SBI_HARTMASK_MAX_BITS] = { 0 }; > > @@ -76,9 +98,9 @@ static u32 generic_hart_index2id[SBI_HARTMASK_MAX_BITS] = { 0 }; > > ?static DECLARE_BITMAP(generic_coldboot_harts, SBI_HARTMASK_MAX_BITS); > > ? > > ?/* > > - * The fw_platform_coldboot_harts_init() function is called by fw_platform_init() > > + * The fw_platform_coldboot_harts_init() function is called by fw_platform_init() > > ? * function to initialize the cold boot harts allowed by the generic platform > > - * according to the DT property "cold-boot-harts" in "/chosen/opensbi-config" > > + * according to the DT property "cold-boot-harts" in "/chosen/opensbi-config" > > ? * DT node. If there is no "cold-boot-harts" in DT, all harts will be allowed. > > ? */ > > ?static void fw_platform_coldboot_harts_init(const void *fdt) > > @@ -185,7 +207,7 @@ unsigned long fw_platform_init(unsigned long arg0, unsigned long arg1, > > ? } > > ? > > ? platform.hart_count = hart_count; > > - platform.heap_size = fw_platform_calculate_heap_size(hart_count); > > + platform.heap_size = fw_platform_get_heap_size(fdt, hart_count); > > ? platform_has_mlevel_imsic = fdt_check_imsic_mlevel(fdt); > > ? > > ? fw_platform_coldboot_harts_init(fdt); > > > -- > opensbi mailing list > opensbi at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/opensbi ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-09-20 7:40 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-09-16 7:24 [PATCH 0/2] platform: improve heap size handle Inochi Amaoto 2024-09-16 7:25 ` [PATCH 1/2] platform: ensure enough heap size with debug triggers extension Inochi Amaoto 2024-09-20 5:33 ` Xiang W 2024-09-16 7:25 ` [PATCH 2/2] platform: introduce DT-based configurable heap size Inochi Amaoto 2024-09-20 5:33 ` Xiang W 2024-09-20 7:40 ` Inochi Amaoto
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox