[PATCH] platform: openpiton: fix uninitialized plic_data struct

[PATCH] platform: openpiton: fix uninitialized plic_data struct

[Manuel Hernández Méndez]

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.

Fixes: 4c37451 ("platform: openpiton: Read the device configurations from
device tree")
Signed-off-by: Manuel Hernández Méndez <maherme.dev@gmail.com>
---
 platform/fpga/openpiton/platform.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/platform/fpga/openpiton/platform.c b/platform/fpga/openpiton/platform.c
index d2cf3e32..faa299ce 100644
--- a/platform/fpga/openpiton/platform.c
+++ b/platform/fpga/openpiton/platform.c
@@ -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
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH] platform: openpiton: fix uninitialized plic_data struct

[Xiang W]

在 2025-07-08二的 20:09 +0200，Manuel Hernández Méndez写道：
> 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.
> 
> Fixes: 4c37451 ("platform: openpiton: Read the device configurations from
> device tree")
> Signed-off-by: Manuel Hernández Méndez <maherme.dev@gmail.com>

Good catch. Maybe we can directy pass uart/plic to 
fdt_parse_uart8250/fdt_parse_plic

Regards,
Xiang W
> ---
>  platform/fpga/openpiton/platform.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/platform/fpga/openpiton/platform.c b/platform/fpga/openpiton/platform.c
> index d2cf3e32..faa299ce 100644
> --- a/platform/fpga/openpiton/platform.c
> +++ b/platform/fpga/openpiton/platform.c
> @@ -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
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH] platform: openpiton: fix uninitialized plic_data struct

[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?


On Wed, Jul 9, 2025 at 8:04 AM Xiang W <wxjstz@126.com> wrote:
>
> 在 2025-07-08二的 20:09 +0200，Manuel Hernández Méndez写道：
> > 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.
> >
> > Fixes: 4c37451 ("platform: openpiton: Read the device configurations from
> > device tree")
> > Signed-off-by: Manuel Hernández Méndez <maherme.dev@gmail.com>
>
> Good catch. Maybe we can directy pass uart/plic to
> fdt_parse_uart8250/fdt_parse_plic
>
> Regards,
> Xiang W
> > ---
> >  platform/fpga/openpiton/platform.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/platform/fpga/openpiton/platform.c b/platform/fpga/openpiton/platform.c
> > index d2cf3e32..faa299ce 100644
> > --- a/platform/fpga/openpiton/platform.c
> > +++ b/platform/fpga/openpiton/platform.c
> > @@ -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
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH] platform: openpiton: fix uninitialized plic_data struct

[Xiang W]

在 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
> 
> 
> On Wed, Jul 9, 2025 at 8:04 AM Xiang W <wxjstz@126.com> wrote:
> > 
> > 在 2025-07-08二的 20:09 +0200，Manuel Hernández Méndez写道：
> > > 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.
> > > 
> > > Fixes: 4c37451 ("platform: openpiton: Read the device configurations from
> > > device tree")
> > > Signed-off-by: Manuel Hernández Méndez <maherme.dev@gmail.com>
> > 
> > Good catch. Maybe we can directy pass uart/plic to
> > fdt_parse_uart8250/fdt_parse_plic
> > 
> > Regards,
> > Xiang W
> > > ---
> > >  platform/fpga/openpiton/platform.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/platform/fpga/openpiton/platform.c b/platform/fpga/openpiton/platform.c
> > > index d2cf3e32..faa299ce 100644
> > > --- a/platform/fpga/openpiton/platform.c
> > > +++ b/platform/fpga/openpiton/platform.c
> > > @@ -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
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH] platform: openpiton: fix uninitialized plic_data struct

[Xiang W]

在 2025-07-08二的 20:09 +0200，Manuel Hernández Méndez写道：
> 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.
> 
> Fixes: 4c37451 ("platform: openpiton: Read the device configurations from
> device tree")
> Signed-off-by: Manuel Hernández Méndez <maherme.dev@gmail.com>
LGTM

Reviewed-by: Xiang W <wxjstz@126.com>
> ---
>  platform/fpga/openpiton/platform.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/platform/fpga/openpiton/platform.c b/platform/fpga/openpiton/platform.c
> index d2cf3e32..faa299ce 100644
> --- a/platform/fpga/openpiton/platform.c
> +++ b/platform/fpga/openpiton/platform.c
> @@ -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
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH] platform: openpiton: fix uninitialized plic_data struct

[Anup Patel]

On Wed, Jul 9, 2025 at 1:10 AM Manuel Hernández Méndez
<maherme.dev@gmail.com> wrote:
>
> 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.
>
> Fixes: 4c37451 ("platform: openpiton: Read the device configurations from
> device tree")
> Signed-off-by: Manuel Hernández Méndez <maherme.dev@gmail.com>

LGTM.

Reviewed-by: Anup Patel <anup@brainfault.org>

Applied this patch to the riscv/opensbi repo.

Thanks,
Anup

> ---
>  platform/fpga/openpiton/platform.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/platform/fpga/openpiton/platform.c b/platform/fpga/openpiton/platform.c
> index d2cf3e32..faa299ce 100644
> --- a/platform/fpga/openpiton/platform.c
> +++ b/platform/fpga/openpiton/platform.c
> @@ -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
> opensbi@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi