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

[Xiang W <wxjstz@126.com>]

在 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