platform: openpiton: fix uninitialized plic_data struct
PATCH 1/1 platform: openpiton: fix uninitialized plic_data struct
The plic_data struct was uninitialized. This led to misfunction behavior since it was subsequently assigned to the global plic struct, and some struct fields, such as flags and irqchip, contained random values. The fix proposes to initialize the plic_data to the global plic struct, so, after parsing the fdt, the fields of the struct will be set to the default values set in global plic struct definition, or the parsed values in the fdt, or zero.
device tree")
--- platform/fpga/openpiton/platform.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
platform/fpga/openpiton/platform.c +1 -1
@@ -79,7 +79,7 @@ static int openpiton_early_init(bool cold_boot) {const void *fdt;struct platform_uart_data uart_data = {0 }; -struct plic_data plic_data; +struct plic_data plic_data = plic;unsigned long aclint_freq;uint64_t clint_addr;int rc;
Signature · 7 lines
2.34.1 -- opensbi mailing list [email protected] http://lists.infradead.org/mailman/listinfo/opensbi
Quoted 11 lines · 在 2025-07-08二的 20:09 +0200，Manuel Hernández Méndez写道：
The
plic_datastruct was uninitialized. This led to misfunction behavior since it was subsequently assigned to the global plic struct, and some struct fields, such as flags and irqchip, contained random values. The fix proposes to initialize theplic_datato the global plic struct, so, after parsing the fdt, the fields of the struct will be set to the default values set in global plic struct definition, or the parsed values in the fdt, or zero.Fixes 4c37451 ("platform: openpiton: Read the device configurations fromdevice tree")
Signed-off-by Manuel Hernández Méndez <[email protected]>
Good catch. Maybe we can directy pass uart/plic to fdt_parse_uart8250/fdt_parse_plic
Regards,
Xiang W
Quoted 20 lines
---
platform/fpga/openpiton/platform.c | 2 +-1 file changed, 1 insertion(+), 1 deletion(-)platform/fpga/openpiton/platform.c +1
-1@@ -79,7 +79,7 @@ static int openpiton_early_init(bool cold_boot) { const void *fdt; struct platform_uart_data uart_data = { 0 }; -struct plic_data plic_data; +struct plic_data plic_data = plic; unsigned long aclint_freq; uint64_t clint_addr; int rc;Signature · 1 line
2.34.1
Signature · 3 lines
opensbi mailing list [email protected] http://lists.infradead.org/mailman/listinfo/opensbi
With this approach, the struct will be set either to all the parsed values from the FDT or to the default ones, but not to an intermediate state. If we directly pass uart/plic to fdt_parse_uart8250/fdt_parse_plic and some failure happens during the parsing process, we end up with an intermediate state (some values in the struct will be set from the FDT, while others will remain at their default values).
After all, don't we prefer to leave the structures in a well-defined state rather than in an intermediate one?
Quoted 40 lines · Xiang W
Quoted 11 lines · 在 2025-07-08二的 20:09 +0200，Manuel Hernández Méndez写道：
The
plic_datastruct was uninitialized. This led to misfunction behavior since it was subsequently assigned to the global plic struct, and some struct fields, such as flags and irqchip, contained random values. The fix proposes to initialize theplic_datato the global plic struct, so, after parsing the fdt, the fields of the struct will be set to the default values set in global plic struct definition, or the parsed values in the fdt, or zero.Fixes 4c37451 ("platform: openpiton: Read the device configurations fromdevice tree")
Signed-off-by Manuel Hernández Méndez <[email protected]>Good catch. Maybe we can directy pass uart/plic to fdt_parse_uart8250/fdt_parse_plic
Regards,
Xiang WQuoted 20 lines
--- platform/fpga/openpiton/platform.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)platform/fpga/openpiton/platform.c +1
-2@@ -79,7 +79,7 @@ static int openpiton_early_init(bool cold_boot) {const void *fdt;struct platform_uart_data uart_data = {0 }; -struct plic_data plic_data; +struct plic_data plic_data = plic;unsigned long aclint_freq;uint64_t clint_addr;int rc; --2.34.1
Signature · 3 lines
opensbi mailing list [email protected] http://lists.infradead.org/mailman/listinfo/opensbi
Quoted 8 lines · 在 2025-07-09三的 10:33 +0200，Manuel Hernández Méndez写道：
With this approach, the struct will be set either to all the parsed values from the FDT or to the default ones, but not to an intermediate state. If we directly pass uart/plic to fdt_parse_uart8250/fdt_parse_plic and some failure happens during the parsing process, we end up with an intermediate state (some values in the struct will be set from the FDT, while others will remain at their default values).
After all, don't we prefer to leave the structures in a well-defined state rather than in an intermediate one?
These two functions will not produce intermediate states. But in order to facilitate future modifications, intermediate states may be introduced. So you are right.
Regards,
Xiang W
Quoted 43 lines · Xiang W
Quoted 40 lines · 在 2025-07-08二的 20:09 +0200，Manuel Hernández Méndez写道：
Quoted 11 lines
The
plic_datastruct was uninitialized. This led to misfunction behavior since it was subsequently assigned to the global plic struct, and some struct fields, such as flags and irqchip, contained random values. The fix proposes to initialize theplic_datato the global plic struct, so, after parsing the fdt, the fields of the struct will be set to the default values set in global plic struct definition, or the parsed values in the fdt, or zero.Fixes 4c37451 ("platform: openpiton: Read the device configurations fromdevice tree")
Signed-off-by Manuel Hernández Méndez <[email protected]>Good catch. Maybe we can directy pass uart/plic to fdt_parse_uart8250/fdt_parse_plic
Regards,
Xiang WQuoted 20 lines
---
platform/fpga/openpiton/platform.c | 2 +-1 file changed, 1 insertion(+), 1 deletion(-)platform/fpga/openpiton/platform.c +1
-2@@ -79,7 +79,7 @@ static int openpiton_early_init(bool cold_boot) { const void *fdt; struct platform_uart_data uart_data = { 0 }; - struct plic_data plic_data; + struct plic_data plic_data = plic; unsigned long aclint_freq; uint64_t clint_addr; int rc; --2.34.1
Signature · 3 lines
opensbi mailing list [email protected] http://lists.infradead.org/mailman/listinfo/opensbi
Quoted 11 lines · 在 2025-07-08二的 20:09 +0200，Manuel Hernández Méndez写道：
The
plic_datastruct was uninitialized. This led to misfunction behavior since it was subsequently assigned to the global plic struct, and some struct fields, such as flags and irqchip, contained random values. The fix proposes to initialize theplic_datato the global plic struct, so, after parsing the fdt, the fields of the struct will be set to the default values set in global plic struct definition, or the parsed values in the fdt, or zero.Fixes 4c37451 ("platform: openpiton: Read the device configurations fromdevice tree")
Signed-off-by Manuel Hernández Méndez <[email protected]>
LGTM
Quoted 20 lines
---
platform/fpga/openpiton/platform.c | 2 +-1 file changed, 1 insertion(+), 1 deletion(-)platform/fpga/openpiton/platform.c +1
-1@@ -79,7 +79,7 @@ static int openpiton_early_init(bool cold_boot) { const void *fdt; struct platform_uart_data uart_data = { 0 }; -struct plic_data plic_data; +struct plic_data plic_data = plic; unsigned long aclint_freq; uint64_t clint_addr; int rc;Signature · 1 line
2.34.1
Signature · 3 lines
opensbi mailing list [email protected] http://lists.infradead.org/mailman/listinfo/opensbi
Quoted 9 lines · Manuel Hernández Méndez
The
plic_datastruct was uninitialized. This led to misfunction behavior since it was subsequently assigned to the global plic struct, and some struct fields, such as flags and irqchip, contained random values. The fix proposes to initialize theplic_datato the global plic struct, so, after parsing the fdt, the fields of the struct will be set to the default values set in global plic struct definition, or the parsed values in the fdt, or zero.
Quoted 3 lines
Fixes 4c37451 ("platform: openpiton: Read the device configurations fromdevice tree")
Signed-off-by Manuel Hernández Méndez <[email protected]>
LGTM.
Applied this patch to the riscv/opensbi repo.
Thanks,
Anup
Quoted 25 lines
--- platform/fpga/openpiton/platform.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)platform/fpga/openpiton/platform.c +1
-2@@ -79,7 +79,7 @@ static int openpiton_early_init(bool cold_boot) {const void *fdt;struct platform_uart_data uart_data = {0 }; -struct plic_data plic_data; +struct plic_data plic_data = plic;unsigned long aclint_freq;uint64_t clint_addr;int rc; --2.34.1
--
opensbi mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/opensbi
Signature · 3 lines
opensbi mailing list [email protected] http://lists.infradead.org/mailman/listinfo/opensbi