* [PATCH v3] generic: mips: support harts to boot from mips_warm_boot @ 2025-07-23 20:40 Chao-ying Fu 2025-08-28 5:32 ` Anup Patel 0 siblings, 1 reply; 6+ messages in thread From: Chao-ying Fu @ 2025-07-23 20:40 UTC (permalink / raw) To: opensbi; +Cc: 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3] generic: mips: support harts to boot from mips_warm_boot 2025-07-23 20:40 [PATCH v3] generic: mips: support harts to boot from mips_warm_boot Chao-ying Fu @ 2025-08-28 5:32 ` Anup Patel 2025-08-28 6:20 ` Jessica Clarke 0 siblings, 1 reply; 6+ messages in thread From: Anup Patel @ 2025-08-28 5:32 UTC (permalink / raw) To: Chao-ying Fu; +Cc: opensbi, Chao-ying Fu 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] generic: mips: support harts to boot from mips_warm_boot 2025-08-28 5:32 ` Anup Patel @ 2025-08-28 6:20 ` Jessica Clarke 2025-08-28 6:47 ` Jessica Clarke 0 siblings, 1 reply; 6+ messages in thread From: Jessica Clarke @ 2025-08-28 6:20 UTC (permalink / raw) To: Anup Patel; +Cc: Chao-ying Fu, opensbi, Chao-ying Fu 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] generic: mips: support harts to boot from mips_warm_boot 2025-08-28 6:20 ` Jessica Clarke @ 2025-08-28 6:47 ` Jessica Clarke 2025-08-28 15:05 ` Anup Patel 0 siblings, 1 reply; 6+ messages in thread From: Jessica Clarke @ 2025-08-28 6:47 UTC (permalink / raw) To: Anup Patel; +Cc: Chao-ying Fu, opensbi, Chao-ying Fu 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] generic: mips: support harts to boot from mips_warm_boot 2025-08-28 6:47 ` Jessica Clarke @ 2025-08-28 15:05 ` Anup Patel 2025-08-26 0:06 ` Chao-ying Fu 0 siblings, 1 reply; 6+ messages in thread From: Anup Patel @ 2025-08-28 15:05 UTC (permalink / raw) To: Jessica Clarke; +Cc: Anup Patel, Chao-ying Fu, opensbi, Chao-ying Fu 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] generic: mips: support harts to boot from mips_warm_boot 2025-08-28 15:05 ` Anup Patel @ 2025-08-26 0:06 ` Chao-ying Fu 0 siblings, 0 replies; 6+ messages in thread From: Chao-ying Fu @ 2025-08-26 0:06 UTC (permalink / raw) To: Anup Patel; +Cc: Jessica Clarke, Anup Patel, opensbi, 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-08-28 23:27 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-07-23 20:40 [PATCH v3] generic: mips: support harts to boot from mips_warm_boot Chao-ying Fu 2025-08-28 5:32 ` Anup Patel 2025-08-28 6:20 ` Jessica Clarke 2025-08-28 6:47 ` Jessica Clarke 2025-08-28 15:05 ` Anup Patel 2025-08-26 0:06 ` Chao-ying Fu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox