[PATCH v3] generic: mips: support harts to boot from mips_warm_boot

[PATCH v3] generic: mips: support harts to boot from mips_warm_boot

[Chao-ying Fu]

We program reset base for harts (other than hart 0) to boot at
mips_warm_boot that jumps to _start_warm. This helps to skip some code
sequence to speed up.

Signed-off-by: Chao-ying Fu <cfu@mips.com>
---
 platform/generic/mips/mips_warm_boot.S | 20 ++++++++++++++++++++
 platform/generic/mips/objects.mk       |  2 +-
 platform/generic/mips/p8700.c          |  5 +++++
 3 files changed, 26 insertions(+), 1 deletion(-)
 create mode 100644 platform/generic/mips/mips_warm_boot.S

diff --git a/platform/generic/mips/mips_warm_boot.S b/platform/generic/mips/mips_warm_boot.S
new file mode 100644
index 0000000..25cdb83
--- /dev/null
+++ b/platform/generic/mips/mips_warm_boot.S
@@ -0,0 +1,20 @@
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2025 MIPS
+ *
+ */
+	.text
+	.align	12
+	.globl	mips_warm_boot
+mips_warm_boot:
+	j	_start_warm
+	.align	2
+nmi_vector:
+	j	_start_warm
+	.align	2
+cacheerr_vector:
+	j	_start_warm
+	.align	2
+debugexc_vector:
+	j	_start_warm
diff --git a/platform/generic/mips/objects.mk b/platform/generic/mips/objects.mk
index bbbc15a..a08c4c6 100644
--- a/platform/generic/mips/objects.mk
+++ b/platform/generic/mips/objects.mk
@@ -4,5 +4,5 @@
 
 ifeq ($(PLATFORM_RISCV_XLEN), 64)
 carray-platform_override_modules-$(CONFIG_PLATFORM_MIPS_P8700) += mips_p8700
-platform-objs-$(CONFIG_PLATFORM_MIPS_P8700) += mips/p8700.o
+platform-objs-$(CONFIG_PLATFORM_MIPS_P8700) += mips/p8700.o mips/mips_warm_boot.o
 endif
diff --git a/platform/generic/mips/p8700.c b/platform/generic/mips/p8700.c
index 888a45c..d3e015b 100644
--- a/platform/generic/mips/p8700.c
+++ b/platform/generic/mips/p8700.c
@@ -16,6 +16,8 @@
 #include <mips/p8700.h>
 #include <mips/mips-cm.h>
 
+extern void mips_warm_boot(void);
+
 static unsigned long mips_csr_read_num(int csr_num)
 {
 #define switchcase_csr_read(__csr_num, __val)		\
@@ -150,6 +152,9 @@ static int mips_hart_start(u32 hartid, ulong saddr)
 	if (hartid == 0)
 		return SBI_ENOTSUPP;
 
+	/* Change reset base to mips_warm_boot */
+	write_gcr_co_reset_base(hartid, (unsigned long)mips_warm_boot, local_p);
+
 	if (cpu_hart(hartid) == 0) {
 		/* Ensure its coherency is disabled */
 		write_gcr_co_coherence(hartid, 0, local_p);
-- 
2.47.1


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

Re: [PATCH v3] generic: mips: support harts to boot from mips_warm_boot

[Chao-ying Fu]

On Thu, Aug 28, 2025 at 8:05 AM Anup Patel <apatel@ventanamicro.com> wrote:
>
> On Thu, Aug 28, 2025 at 12:17 PM Jessica Clarke <jrtc27@jrtc27.com> wrote:
> >
> > On 28 Aug 2025, at 07:20, Jessica Clarke <jrtc27@jrtc27.com> wrote:
> > > On 28 Aug 2025, at 06:32, Anup Patel <anup@brainfault.org> wrote:
> > >> On Thu, Jul 24, 2025 at 2:10 AM Chao-ying Fu <icebergfu@gmail.com> wrote:
> > >>>
> > >>> We program reset base for harts (other than hart 0) to boot at
> > >>> mips_warm_boot that jumps to _start_warm. This helps to skip some code
> > >>> sequence to speed up.
> > >>>
> > >>> Signed-off-by: Chao-ying Fu <cfu@mips.com>
> > >>> ---
> > >>> platform/generic/mips/mips_warm_boot.S | 20 ++++++++++++++++++++
> > >>> platform/generic/mips/objects.mk       |  2 +-
> > >>> platform/generic/mips/p8700.c          |  5 +++++
> > >>> 3 files changed, 26 insertions(+), 1 deletion(-)
> > >>> create mode 100644 platform/generic/mips/mips_warm_boot.S
> > >>>
> > >>> diff --git a/platform/generic/mips/mips_warm_boot.S b/platform/generic/mips/mips_warm_boot.S
> > >>> new file mode 100644
> > >>> index 0000000..25cdb83
> > >>> --- /dev/null
> > >>> +++ b/platform/generic/mips/mips_warm_boot.S
> > >>> @@ -0,0 +1,20 @@
> > >>> +/*
> > >>> + * SPDX-License-Identifier: BSD-2-Clause
> > >>> + *
> > >>> + * Copyright (c) 2025 MIPS
> > >>> + *
> > >>> + */
> > >>> +       .text
> > >>> +       .align  12
> > >>> +       .globl  mips_warm_boot
> > >>> +mips_warm_boot:
> > >>> +       j       _start_warm
> > >>> +       .align  2
> > >>> +nmi_vector:
> > >>> +       j       _start_warm
> > >>> +       .align  2
> > >>> +cacheerr_vector:
> > >>> +       j       _start_warm
> > >>> +       .align  2
> > >>> +debugexc_vector:
> > >>> +       j       _start_warm
> > >>
> > >> nmi_vector, cacheerr_vector, and debugexc_vector are not used
> > >> anywhere so I have dropped these at the time of merging. Please
> > >> send separate patches to add these when they are being used
> > >> somewhere.
> > >>
> > >> Reviewed-by: Anup Patel <anup@brainfault.org>
> > >>
> > >> Applied this patch to the riscv/opensbi repo.
> > >
> > > Uh I’m pretty sure this is a reset vector, and each label is naming the
> > > different entries in the vector, much like for mtvec.MODE=Vectored, and
> > > therefore the result of deleting those vector entries is that, if any
> > > of those cases occur, it will end up running whatever random code
> > > happens to be after the first entry.
> > >
> > > I’ve noticed you are quite eager to modify people’s patches before
> > > applying them. Reformatting is one thing, but deleting assembly code is
> > > another that I think goes too far. Please be more considerate of the
> > > fact that you may not be correct in your interpretation, and so it is
> > > not appropriate to unilaterally commit whatever unreviewed changes you
> > > think are best, especially when it comes to vendor-specific code for
> > > which you haven’t read any of the corresponding manuals.
> >
> > Especially since you keep the author intact, don’t add any kind of
> > Co-authored-by and don’t document any of the changes you have made
> > between the author’s Signed-of-by and yours. This means it looks like
> > it’s the submitter’s fault that there’s a bug in the commit, when in
> > reality they submitted a correct patch and you broke it. That’s not a
> > particularly nice thing to have done to one’s work.
> >
> > Might I suggest, if the goal is to avoid the author needing to submit a
> > vN+1, at least confirm with the author that your intended fixes are
> > correct before folding them into the patch you commit? And, for more
> > minor cases, document that you’ve made changes, whether in the Linux
> > kernel committer style that states what you’ve actually done, or at
> > least by adding your own Co-authored-by to state you’ve done something
> > unspecified.
> >
>
> I don't mind adding Co-developed-by and more comments about
> my changes.
>
> Regards,
> Anup

Hi All,

  We will see if we need to add support for NMI, cache error, and
debug exceptions later. The previous patch just treats them the same
as "reset" and lets them jump to the same entry: _start_warm.

  Thanks a lot for reviewing and merging our patches!

Regards,
Chao-ying

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

Re: [PATCH v3] generic: mips: support harts to boot from mips_warm_boot

[Anup Patel]

On Thu, Jul 24, 2025 at 2:10 AM Chao-ying Fu <icebergfu@gmail.com> wrote:
>
> We program reset base for harts (other than hart 0) to boot at
> mips_warm_boot that jumps to _start_warm. This helps to skip some code
> sequence to speed up.
>
> Signed-off-by: Chao-ying Fu <cfu@mips.com>
> ---
>  platform/generic/mips/mips_warm_boot.S | 20 ++++++++++++++++++++
>  platform/generic/mips/objects.mk       |  2 +-
>  platform/generic/mips/p8700.c          |  5 +++++
>  3 files changed, 26 insertions(+), 1 deletion(-)
>  create mode 100644 platform/generic/mips/mips_warm_boot.S
>
> diff --git a/platform/generic/mips/mips_warm_boot.S b/platform/generic/mips/mips_warm_boot.S
> new file mode 100644
> index 0000000..25cdb83
> --- /dev/null
> +++ b/platform/generic/mips/mips_warm_boot.S
> @@ -0,0 +1,20 @@
> +/*
> + * SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright (c) 2025 MIPS
> + *
> + */
> +       .text
> +       .align  12
> +       .globl  mips_warm_boot
> +mips_warm_boot:
> +       j       _start_warm
> +       .align  2
> +nmi_vector:
> +       j       _start_warm
> +       .align  2
> +cacheerr_vector:
> +       j       _start_warm
> +       .align  2
> +debugexc_vector:
> +       j       _start_warm

nmi_vector, cacheerr_vector, and debugexc_vector are not used
anywhere so I have dropped these at the time of merging. Please
send separate patches to add these when they are being used
somewhere.

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

Applied this patch to the riscv/opensbi repo.

Thanks,
Anup

> diff --git a/platform/generic/mips/objects.mk b/platform/generic/mips/objects.mk
> index bbbc15a..a08c4c6 100644
> --- a/platform/generic/mips/objects.mk
> +++ b/platform/generic/mips/objects.mk
> @@ -4,5 +4,5 @@
>
>  ifeq ($(PLATFORM_RISCV_XLEN), 64)
>  carray-platform_override_modules-$(CONFIG_PLATFORM_MIPS_P8700) += mips_p8700
> -platform-objs-$(CONFIG_PLATFORM_MIPS_P8700) += mips/p8700.o
> +platform-objs-$(CONFIG_PLATFORM_MIPS_P8700) += mips/p8700.o mips/mips_warm_boot.o
>  endif
> diff --git a/platform/generic/mips/p8700.c b/platform/generic/mips/p8700.c
> index 888a45c..d3e015b 100644
> --- a/platform/generic/mips/p8700.c
> +++ b/platform/generic/mips/p8700.c
> @@ -16,6 +16,8 @@
>  #include <mips/p8700.h>
>  #include <mips/mips-cm.h>
>
> +extern void mips_warm_boot(void);
> +
>  static unsigned long mips_csr_read_num(int csr_num)
>  {
>  #define switchcase_csr_read(__csr_num, __val)          \
> @@ -150,6 +152,9 @@ static int mips_hart_start(u32 hartid, ulong saddr)
>         if (hartid == 0)
>                 return SBI_ENOTSUPP;
>
> +       /* Change reset base to mips_warm_boot */
> +       write_gcr_co_reset_base(hartid, (unsigned long)mips_warm_boot, local_p);
> +
>         if (cpu_hart(hartid) == 0) {
>                 /* Ensure its coherency is disabled */
>                 write_gcr_co_coherence(hartid, 0, local_p);
> --
> 2.47.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

Re: [PATCH v3] generic: mips: support harts to boot from mips_warm_boot

[Jessica Clarke]

On 28 Aug 2025, at 06:32, Anup Patel <anup@brainfault.org> wrote:
> On Thu, Jul 24, 2025 at 2:10 AM Chao-ying Fu <icebergfu@gmail.com> wrote:
>> 
>> We program reset base for harts (other than hart 0) to boot at
>> mips_warm_boot that jumps to _start_warm. This helps to skip some code
>> sequence to speed up.
>> 
>> Signed-off-by: Chao-ying Fu <cfu@mips.com>
>> ---
>> platform/generic/mips/mips_warm_boot.S | 20 ++++++++++++++++++++
>> platform/generic/mips/objects.mk       |  2 +-
>> platform/generic/mips/p8700.c          |  5 +++++
>> 3 files changed, 26 insertions(+), 1 deletion(-)
>> create mode 100644 platform/generic/mips/mips_warm_boot.S
>> 
>> diff --git a/platform/generic/mips/mips_warm_boot.S b/platform/generic/mips/mips_warm_boot.S
>> new file mode 100644
>> index 0000000..25cdb83
>> --- /dev/null
>> +++ b/platform/generic/mips/mips_warm_boot.S
>> @@ -0,0 +1,20 @@
>> +/*
>> + * SPDX-License-Identifier: BSD-2-Clause
>> + *
>> + * Copyright (c) 2025 MIPS
>> + *
>> + */
>> +       .text
>> +       .align  12
>> +       .globl  mips_warm_boot
>> +mips_warm_boot:
>> +       j       _start_warm
>> +       .align  2
>> +nmi_vector:
>> +       j       _start_warm
>> +       .align  2
>> +cacheerr_vector:
>> +       j       _start_warm
>> +       .align  2
>> +debugexc_vector:
>> +       j       _start_warm
> 
> nmi_vector, cacheerr_vector, and debugexc_vector are not used
> anywhere so I have dropped these at the time of merging. Please
> send separate patches to add these when they are being used
> somewhere.
> 
> Reviewed-by: Anup Patel <anup@brainfault.org>
> 
> Applied this patch to the riscv/opensbi repo.

Uh I’m pretty sure this is a reset vector, and each label is naming the
different entries in the vector, much like for mtvec.MODE=Vectored, and
therefore the result of deleting those vector entries is that, if any
of those cases occur, it will end up running whatever random code
happens to be after the first entry.

I’ve noticed you are quite eager to modify people’s patches before
applying them. Reformatting is one thing, but deleting assembly code is
another that I think goes too far. Please be more considerate of the
fact that you may not be correct in your interpretation, and so it is
not appropriate to unilaterally commit whatever unreviewed changes you
think are best, especially when it comes to vendor-specific code for
which you haven’t read any of the corresponding manuals.

Jessica


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

Re: [PATCH v3] generic: mips: support harts to boot from mips_warm_boot

[Jessica Clarke]

On 28 Aug 2025, at 07:20, Jessica Clarke <jrtc27@jrtc27.com> wrote:
> On 28 Aug 2025, at 06:32, Anup Patel <anup@brainfault.org> wrote:
>> On Thu, Jul 24, 2025 at 2:10 AM Chao-ying Fu <icebergfu@gmail.com> wrote:
>>> 
>>> We program reset base for harts (other than hart 0) to boot at
>>> mips_warm_boot that jumps to _start_warm. This helps to skip some code
>>> sequence to speed up.
>>> 
>>> Signed-off-by: Chao-ying Fu <cfu@mips.com>
>>> ---
>>> platform/generic/mips/mips_warm_boot.S | 20 ++++++++++++++++++++
>>> platform/generic/mips/objects.mk       |  2 +-
>>> platform/generic/mips/p8700.c          |  5 +++++
>>> 3 files changed, 26 insertions(+), 1 deletion(-)
>>> create mode 100644 platform/generic/mips/mips_warm_boot.S
>>> 
>>> diff --git a/platform/generic/mips/mips_warm_boot.S b/platform/generic/mips/mips_warm_boot.S
>>> new file mode 100644
>>> index 0000000..25cdb83
>>> --- /dev/null
>>> +++ b/platform/generic/mips/mips_warm_boot.S
>>> @@ -0,0 +1,20 @@
>>> +/*
>>> + * SPDX-License-Identifier: BSD-2-Clause
>>> + *
>>> + * Copyright (c) 2025 MIPS
>>> + *
>>> + */
>>> +       .text
>>> +       .align  12
>>> +       .globl  mips_warm_boot
>>> +mips_warm_boot:
>>> +       j       _start_warm
>>> +       .align  2
>>> +nmi_vector:
>>> +       j       _start_warm
>>> +       .align  2
>>> +cacheerr_vector:
>>> +       j       _start_warm
>>> +       .align  2
>>> +debugexc_vector:
>>> +       j       _start_warm
>> 
>> nmi_vector, cacheerr_vector, and debugexc_vector are not used
>> anywhere so I have dropped these at the time of merging. Please
>> send separate patches to add these when they are being used
>> somewhere.
>> 
>> Reviewed-by: Anup Patel <anup@brainfault.org>
>> 
>> Applied this patch to the riscv/opensbi repo.
> 
> Uh I’m pretty sure this is a reset vector, and each label is naming the
> different entries in the vector, much like for mtvec.MODE=Vectored, and
> therefore the result of deleting those vector entries is that, if any
> of those cases occur, it will end up running whatever random code
> happens to be after the first entry.
> 
> I’ve noticed you are quite eager to modify people’s patches before
> applying them. Reformatting is one thing, but deleting assembly code is
> another that I think goes too far. Please be more considerate of the
> fact that you may not be correct in your interpretation, and so it is
> not appropriate to unilaterally commit whatever unreviewed changes you
> think are best, especially when it comes to vendor-specific code for
> which you haven’t read any of the corresponding manuals.

Especially since you keep the author intact, don’t add any kind of
Co-authored-by and don’t document any of the changes you have made
between the author’s Signed-of-by and yours. This means it looks like
it’s the submitter’s fault that there’s a bug in the commit, when in
reality they submitted a correct patch and you broke it. That’s not a
particularly nice thing to have done to one’s work.

Might I suggest, if the goal is to avoid the author needing to submit a
vN+1, at least confirm with the author that your intended fixes are
correct before folding them into the patch you commit? And, for more
minor cases, document that you’ve made changes, whether in the Linux
kernel committer style that states what you’ve actually done, or at
least by adding your own Co-authored-by to state you’ve done something
unspecified.

Jessica


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

Re: [PATCH v3] generic: mips: support harts to boot from mips_warm_boot

[Anup Patel]

On Thu, Aug 28, 2025 at 12:17 PM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>
> On 28 Aug 2025, at 07:20, Jessica Clarke <jrtc27@jrtc27.com> wrote:
> > On 28 Aug 2025, at 06:32, Anup Patel <anup@brainfault.org> wrote:
> >> On Thu, Jul 24, 2025 at 2:10 AM Chao-ying Fu <icebergfu@gmail.com> wrote:
> >>>
> >>> We program reset base for harts (other than hart 0) to boot at
> >>> mips_warm_boot that jumps to _start_warm. This helps to skip some code
> >>> sequence to speed up.
> >>>
> >>> Signed-off-by: Chao-ying Fu <cfu@mips.com>
> >>> ---
> >>> platform/generic/mips/mips_warm_boot.S | 20 ++++++++++++++++++++
> >>> platform/generic/mips/objects.mk       |  2 +-
> >>> platform/generic/mips/p8700.c          |  5 +++++
> >>> 3 files changed, 26 insertions(+), 1 deletion(-)
> >>> create mode 100644 platform/generic/mips/mips_warm_boot.S
> >>>
> >>> diff --git a/platform/generic/mips/mips_warm_boot.S b/platform/generic/mips/mips_warm_boot.S
> >>> new file mode 100644
> >>> index 0000000..25cdb83
> >>> --- /dev/null
> >>> +++ b/platform/generic/mips/mips_warm_boot.S
> >>> @@ -0,0 +1,20 @@
> >>> +/*
> >>> + * SPDX-License-Identifier: BSD-2-Clause
> >>> + *
> >>> + * Copyright (c) 2025 MIPS
> >>> + *
> >>> + */
> >>> +       .text
> >>> +       .align  12
> >>> +       .globl  mips_warm_boot
> >>> +mips_warm_boot:
> >>> +       j       _start_warm
> >>> +       .align  2
> >>> +nmi_vector:
> >>> +       j       _start_warm
> >>> +       .align  2
> >>> +cacheerr_vector:
> >>> +       j       _start_warm
> >>> +       .align  2
> >>> +debugexc_vector:
> >>> +       j       _start_warm
> >>
> >> nmi_vector, cacheerr_vector, and debugexc_vector are not used
> >> anywhere so I have dropped these at the time of merging. Please
> >> send separate patches to add these when they are being used
> >> somewhere.
> >>
> >> Reviewed-by: Anup Patel <anup@brainfault.org>
> >>
> >> Applied this patch to the riscv/opensbi repo.
> >
> > Uh I’m pretty sure this is a reset vector, and each label is naming the
> > different entries in the vector, much like for mtvec.MODE=Vectored, and
> > therefore the result of deleting those vector entries is that, if any
> > of those cases occur, it will end up running whatever random code
> > happens to be after the first entry.
> >
> > I’ve noticed you are quite eager to modify people’s patches before
> > applying them. Reformatting is one thing, but deleting assembly code is
> > another that I think goes too far. Please be more considerate of the
> > fact that you may not be correct in your interpretation, and so it is
> > not appropriate to unilaterally commit whatever unreviewed changes you
> > think are best, especially when it comes to vendor-specific code for
> > which you haven’t read any of the corresponding manuals.
>
> Especially since you keep the author intact, don’t add any kind of
> Co-authored-by and don’t document any of the changes you have made
> between the author’s Signed-of-by and yours. This means it looks like
> it’s the submitter’s fault that there’s a bug in the commit, when in
> reality they submitted a correct patch and you broke it. That’s not a
> particularly nice thing to have done to one’s work.
>
> Might I suggest, if the goal is to avoid the author needing to submit a
> vN+1, at least confirm with the author that your intended fixes are
> correct before folding them into the patch you commit? And, for more
> minor cases, document that you’ve made changes, whether in the Linux
> kernel committer style that states what you’ve actually done, or at
> least by adding your own Co-authored-by to state you’ve done something
> unspecified.
>

I don't mind adding Co-developed-by and more comments about
my changes.

Regards,
Anup

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