[PATCH v4 3/3] generic/starfive: Add Starfive JH7110 platform implementation

[Cheehong Ang]

> -----Original Message-----
> From: Anup Patel <apatel at ventanamicro.com>
> Sent: Tuesday, December 20, 2022 7:05 PM
> To: WeiLiang Lim <weiliang.lim at starfivetech.com>
> Cc: opensbi at lists.infradead.org; Cheehong Ang
> <cheehong.ang at starfivetech.com>; JunLiang Tan
> <junliang.tan at starfivetech.com>
> Subject: Re: [PATCH v4 3/3] generic/starfive: Add Starfive JH7110 platform
> implementation
> 
> On Tue, Dec 20, 2022 at 9:12 AM Wei Liang Lim
> <weiliang.lim at starfivetech.com> wrote:
> >
> > Add Starfive JH7110 platform implementation
> >
> > Signed-off-by: Wei Liang Lim <weiliang.lim at starfivetech.com>
> > Reviewed-by: Chee Hong Ang <cheehong.ang at starfivetech.com>
> > Reviewed-by: Jun Liang Tan <junliang.tan at starfivetech.com>
> > ---
> >  platform/generic/Kconfig             |  4 ++
> >  platform/generic/configs/defconfig   |  1 +
> >  platform/generic/starfive/jh7110.c   | 57 ++++++++++++++++++++++++++++
> >  platform/generic/starfive/objects.mk |  6 +++
> >  4 files changed, 68 insertions(+)
> >  create mode 100644 platform/generic/starfive/jh7110.c
> >  create mode 100644 platform/generic/starfive/objects.mk
> >
> > diff --git a/platform/generic/Kconfig b/platform/generic/Kconfig index
> > 62c7a2d..47c10e5 100644
> > --- a/platform/generic/Kconfig
> > +++ b/platform/generic/Kconfig
> > @@ -45,4 +45,8 @@ config PLATFORM_SIFIVE_FU740
> >         depends on FDT_RESET && FDT_I2C
> >         default n
> >
> > +config PLATFORM_STARFIVE_JH7110
> > +       bool "StarFive JH7110 support"
> > +       default n
> > +
> >  endif
> > diff --git a/platform/generic/configs/defconfig
> > b/platform/generic/configs/defconfig
> > index 47fca95..4b0842e 100644
> > --- a/platform/generic/configs/defconfig
> > +++ b/platform/generic/configs/defconfig
> > @@ -3,6 +3,7 @@ CONFIG_PLATFORM_ANDES_AE350=y
> > CONFIG_PLATFORM_RENESAS_RZFIVE=y
> CONFIG_PLATFORM_SIFIVE_FU540=y
> > CONFIG_PLATFORM_SIFIVE_FU740=y
> > +CONFIG_PLATFORM_STARFIVE_JH7110=y
> >  CONFIG_FDT_GPIO=y
> >  CONFIG_FDT_GPIO_SIFIVE=y
> >  CONFIG_FDT_I2C=y
> > diff --git a/platform/generic/starfive/jh7110.c
> > b/platform/generic/starfive/jh7110.c
> > new file mode 100644
> > index 0000000..5d3ab6f
> > --- /dev/null
> > +++ b/platform/generic/starfive/jh7110.c
> > @@ -0,0 +1,57 @@
> > +/*
> > + * SPDX-License-Identifier: BSD-2-Clause
> > + *
> > + * Copyright (c) 2022 StarFive
> > + *
> > + * Authors:
> > + *   Wei Liang Lim <weiliang.lim at starfivetech.com>
> > + */
> > +
> > +#include <libfdt.h>
> > +#include <platform_override.h>
> > +#include <sbi_utils/fdt/fdt_helper.h>
> > +
> > +static bool hart_cold_booted = FALSE;
> > +
> > +static bool starfive_jh7110_cold_boot_allowed(u32 hartid,
> > +                                  const struct fdt_match *match) {
> > +       const fdt32_t *val;
> > +       int len, coff;
> > +
> > +       if (hart_cold_booted)
> > +               return FALSE;
> > +
> > +       void *fdt = fdt_get_address();
> 
> starfive_jh7110_cold_boot_allowed() will be called at runtime during CPU
> hotplug or CPU non-retentive suspend so don't access FDT in this function
> because it might have been corrupted/over-written by S-mode software.
> 
> You can save data from FDT in starfive_jh7110_early_init() when cold_boot ==
> TRUE.
> 
The problem is early_init() is called from init_coldboot() and init_warm_startup() which is right after cold_boot_allowed(). Therefore, we have to access the FDT here, this is why we add a
'hart_cold_booted' boolean to ensure the FDT will not be accessed again once the FDT is read in cold boot path for the first time.
This is to avoid the issue you mentioned above. The code here already ensure the FDT will not be accessed again when the cold_boot_allowed() function is called during
HSM SBI calls. This is the reason in the beginning of the cold_boot_allowed() function, we check whether the 'hart_cold_booted' is set to TRUE which mean the FDT should not be accessed
again.

> Regards,
> Anup
> 
> > +
> > +       coff = fdt_path_offset(fdt, "/chosen");
> > +       if (-1 < coff) {
> > +               val = fdt_getprop(fdt, coff, "boot-hart", &len);
> > +               if (val && len) {
> > +                       u32 selected_hartid = (u32) fdt32_to_cpu(*val);
> > +                       return (selected_hartid == hartid);
> > +               }
> > +       }
> > +
> > +       return TRUE;
> > +}
> > +
> > +static int starfive_jh7110_early_init(bool cold_boot,
> > +                                  const struct fdt_match *match) {
> > +       if (cold_boot)
> > +               hart_cold_booted = TRUE;
> > +
> > +       return 0;
> > +}
> > +
> > +static const struct fdt_match starfive_jh7110_match[] = {
> > +       { .compatible = "starfive,jh7110" },
> > +       { },
> > +};
> > +
> > +const struct platform_override starfive_jh7110 = {
> > +       .match_table = starfive_jh7110_match,
> > +       .early_init = starfive_jh7110_early_init,
> > +       .cold_boot_allowed = starfive_jh7110_cold_boot_allowed,
> > +};
> > diff --git a/platform/generic/starfive/objects.mk
> > b/platform/generic/starfive/objects.mk
> > new file mode 100644
> > index 0000000..0b900fb
> > --- /dev/null
> > +++ b/platform/generic/starfive/objects.mk
> > @@ -0,0 +1,6 @@
> > +#
> > +# SPDX-License-Identifier: BSD-2-Clause #
> > +
> > +carray-platform_override_modules-
> $(CONFIG_PLATFORM_STARFIVE_JH7110)
> > ++= starfive_jh7110
> > +platform-objs-$(CONFIG_PLATFORM_STARFIVE_JH7110) +=
> starfive/jh7110.o
> > --
> > 2.25.1
> >