diff mbox series

[03/16] image: Take entry point as an output of setup_booti

Message ID 20240522-loongarch-v1-3-1407e0b69678@flygoat.com
State Changes Requested
Delegated to: Tom Rini
Headers show
Series LoongArch initial support | expand

Commit Message

Jiaxun Yang May 22, 2024, 3:34 p.m. UTC
For LoongArch the start of the image is not the entry
point to the image.

We refactor the code base to allow entry point to be
supplied by setup_booti.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 arch/arm/lib/image.c     | 3 ++-
 arch/riscv/lib/image.c   | 4 +++-
 arch/sandbox/lib/bootm.c | 2 +-
 boot/bootm.c             | 5 +++--
 cmd/booti.c              | 5 +++--
 common/spl/spl.c         | 9 +++++----
 include/image.h          | 3 ++-
 7 files changed, 19 insertions(+), 12 deletions(-)

Comments

Heinrich Schuchardt June 11, 2024, 1:02 p.m. UTC | #1
On 22.05.24 17:34, Jiaxun Yang wrote:
> For LoongArch the start of the image is not the entry
> point to the image.

Looking at arch/loongarch/kernel/head.S there seem to be two cases:

* The kernel has an EFI stub (CONFIG_EFI_STUB=y).
   The legacy physical entry point is available at offset 0x08 of the
header.
* The kernel has no EFI stub.
   The kernel entry point matches the start of the image.

Where do you differentiate between the cases?

Best regards

Heinrich

>
> We refactor the code base to allow entry point to be
> supplied by setup_booti.
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>   arch/arm/lib/image.c     | 3 ++-
>   arch/riscv/lib/image.c   | 4 +++-
>   arch/sandbox/lib/bootm.c | 2 +-
>   boot/bootm.c             | 5 +++--
>   cmd/booti.c              | 5 +++--
>   common/spl/spl.c         | 9 +++++----
>   include/image.h          | 3 ++-
>   7 files changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/lib/image.c b/arch/arm/lib/image.c
> index e394c1ad9093..024b6adc75e7 100644
> --- a/arch/arm/lib/image.c
> +++ b/arch/arm/lib/image.c
> @@ -30,7 +30,7 @@ struct Image_header {
>   };
>
>   int booti_setup(ulong image, ulong *relocated_addr, ulong *size,
> -		bool force_reloc)
> +		ulong *entry, bool force_reloc)
>   {
>   	struct Image_header *ih;
>   	uint64_t dst;
> @@ -73,6 +73,7 @@ int booti_setup(ulong image, ulong *relocated_addr, ulong *size,
>   		dst = gd->bd->bi_dram[0].start;
>
>   	*relocated_addr = ALIGN(dst, SZ_2M) + text_offset;
> +	*entry = *relocated_addr;
>
>   	unmap_sysmem(ih);
>
> diff --git a/arch/riscv/lib/image.c b/arch/riscv/lib/image.c
> index a82f48e9a505..2fd1f6c535ae 100644
> --- a/arch/riscv/lib/image.c
> +++ b/arch/riscv/lib/image.c
> @@ -33,7 +33,7 @@ struct linux_image_h {
>   };
>
>   int booti_setup(ulong image, ulong *relocated_addr, ulong *size,
> -		bool force_reloc)
> +		ulong entry, bool force_reloc)
>   {
>   	struct linux_image_h *lhdr;
>
> @@ -56,6 +56,8 @@ int booti_setup(ulong image, ulong *relocated_addr, ulong *size,
>   		*relocated_addr = image;
>   	}
>
> +	*entry = *relocated_addr;
> +
>   	unmap_sysmem(lhdr);
>
>   	return 0;
> diff --git a/arch/sandbox/lib/bootm.c b/arch/sandbox/lib/bootm.c
> index 44ba8b52e139..4ef34c81d6d2 100644
> --- a/arch/sandbox/lib/bootm.c
> +++ b/arch/sandbox/lib/bootm.c
> @@ -83,7 +83,7 @@ int do_bootm_linux(int flag, struct bootm_info *bmi)
>
>   /* used for testing 'booti' command */
>   int booti_setup(ulong image, ulong *relocated_addr, ulong *size,
> -		bool force_reloc)
> +		ulong entry, bool force_reloc)
>   {
>   	log_err("Booting is not supported on the sandbox.\n");
>
> diff --git a/boot/bootm.c b/boot/bootm.c
> index 032f5a4a1605..770300132891 100644
> --- a/boot/bootm.c
> +++ b/boot/bootm.c
> @@ -693,9 +693,10 @@ static int bootm_load_os(struct bootm_headers *images, int boot_progress)
>   	    images->os.os == IH_OS_LINUX) {
>   		ulong relocated_addr;
>   		ulong image_size;
> +		ulong entry;
>   		int ret;
>
> -		ret = booti_setup(load, &relocated_addr, &image_size, false);
> +		ret = booti_setup(load, &relocated_addr, &image_size, &entry, false);
>   		if (ret) {
>   			printf("Failed to prep arm64 kernel (err=%d)\n", ret);
>   			return BOOTM_ERR_RESET;
> @@ -709,7 +710,7 @@ static int bootm_load_os(struct bootm_headers *images, int boot_progress)
>   			memmove((void *)relocated_addr, load_buf, image_size);
>   		}
>
> -		images->ep = relocated_addr;
> +		images->ep = entry;
>   		images->os.start = relocated_addr;
>   		images->os.end = relocated_addr + image_size;
>   	}
> diff --git a/cmd/booti.c b/cmd/booti.c
> index b9637b3ec3d8..9586a4c58ac1 100644
> --- a/cmd/booti.c
> +++ b/cmd/booti.c
> @@ -27,6 +27,7 @@ static int booti_start(struct bootm_info *bmi)
>   	ulong ld;
>   	ulong relocated_addr;
>   	ulong image_size;
> +	ulong entry;
>   	uint8_t *temp;
>   	ulong dest;
>   	ulong dest_end;
> @@ -73,7 +74,7 @@ static int booti_start(struct bootm_info *bmi)
>   	}
>   	unmap_sysmem((void *)ld);
>
> -	ret = booti_setup(ld, &relocated_addr, &image_size, false);
> +	ret = booti_setup(ld, &relocated_addr, &image_size, &entry, false);
>   	if (ret)
>   		return 1;
>
> @@ -84,7 +85,7 @@ static int booti_start(struct bootm_info *bmi)
>   		memmove((void *)relocated_addr, (void *)ld, image_size);
>   	}
>
> -	images->ep = relocated_addr;
> +	images->ep = entry;
>   	images->os.start = relocated_addr;
>   	images->os.end = relocated_addr + image_size;
>
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index e06bc75d36b2..52a4bee13728 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -113,7 +113,8 @@ int __weak bootz_setup(ulong image, ulong *start, ulong *end)
>   	 return 1;
>   }
>
> -int __weak booti_setup(ulong image, ulong *relocated_addr, ulong *size, bool force_reloc)
> +int __weak booti_setup(ulong image, ulong *relocated_addr, ulong *size,
> +		       ulong *entry, bool force_reloc)
>   {
>   	 return 1;
>   }
> @@ -324,13 +325,13 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
>
>   #if CONFIG_IS_ENABLED(OS_BOOT)
>   #if defined(CMD_BOOTI)
> -		ulong start, size;
> +		ulong start, size, entry;
>
> -		if (!booti_setup((ulong)header, &start, &size, 0)) {
> +		if (!booti_setup((ulong)header, &start, &size, &entry, 0)) {
>   			spl_image->name = "Linux";
>   			spl_image->os = IH_OS_LINUX;
>   			spl_image->load_addr = start;
> -			spl_image->entry_point = start;
> +			spl_image->entry_point = entry;
>   			spl_image->size = size;
>   			debug(SPL_TPL_PROMPT
>   			      "payload Image, load addr: 0x%lx size: %d\n",
> diff --git a/include/image.h b/include/image.h
> index acffd17e0dfd..a2bfc7bb19a3 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -1061,11 +1061,12 @@ int bootz_setup(ulong image, ulong *start, ulong *end);
>    * @image: Address of image
>    * @start: Returns start address of image
>    * @size : Returns size image
> + * @entry: Returns entry point of image
>    * @force_reloc: Ignore image->ep field, always place image to RAM start
>    * Return: 0 if OK, 1 if the image was not recognised
>    */
>   int booti_setup(ulong image, ulong *relocated_addr, ulong *size,
> -		bool force_reloc);
> +		ulong *entry, bool force_reloc);
>
>   /*******************************************************************/
>   /* New uImage format specific code (prefixed with fit_) */
>
Jiaxun Yang June 11, 2024, 1:29 p.m. UTC | #2
在2024年6月11日六月 下午2:02,Heinrich Schuchardt写道:
> On 22.05.24 17:34, Jiaxun Yang wrote:
>> For LoongArch the start of the image is not the entry
>> point to the image.
>
> Looking at arch/loongarch/kernel/head.S there seem to be two cases:
>
> * The kernel has an EFI stub (CONFIG_EFI_STUB=y).
>    The legacy physical entry point is available at offset 0x08 of the
> header.
> * The kernel has no EFI stub.
>    The kernel entry point matches the start of the image.
>
> Where do you differentiate between the cases?

Hi Heinrich,

In case there is no EFI stub LoongArch would use elf format vmlinux, there
is no real "raw" image for us.

bootelf can't be used at the moment as it doesn't setup FDT and other environments
properly.

I'm planning to implement a bootlinuxelf command for this case, as MIPS, LoongArch,
xtensa are all using ELF as default kernel image format.

Thanks
- Jiaxun
>
[...]
Tom Rini June 11, 2024, 1:52 p.m. UTC | #3
On Tue, Jun 11, 2024 at 02:29:38PM +0100, Jiaxun Yang wrote:
> 
> 
> 在2024年6月11日六月 下午2:02,Heinrich Schuchardt写道:
> > On 22.05.24 17:34, Jiaxun Yang wrote:
> >> For LoongArch the start of the image is not the entry
> >> point to the image.
> >
> > Looking at arch/loongarch/kernel/head.S there seem to be two cases:
> >
> > * The kernel has an EFI stub (CONFIG_EFI_STUB=y).
> >    The legacy physical entry point is available at offset 0x08 of the
> > header.
> > * The kernel has no EFI stub.
> >    The kernel entry point matches the start of the image.
> >
> > Where do you differentiate between the cases?
> 
> Hi Heinrich,
> 
> In case there is no EFI stub LoongArch would use elf format vmlinux, there
> is no real "raw" image for us.
> 
> bootelf can't be used at the moment as it doesn't setup FDT and other environments
> properly.
> 
> I'm planning to implement a bootlinuxelf command for this case, as MIPS, LoongArch,
> xtensa are all using ELF as default kernel image format.

Please note that we have CONFIG_CMD_ELF_FDT_SETUP already, so adjusting
things so that bootelf works for this case should be doable without a
new command.
Jiaxun Yang June 11, 2024, 2:01 p.m. UTC | #4
在2024年6月11日六月 下午2:52,Tom Rini写道:
> On Tue, Jun 11, 2024 at 02:29:38PM +0100, Jiaxun Yang wrote:
>> 
>> 
>> 在2024年6月11日六月 下午2:02,Heinrich Schuchardt写道:
>> > On 22.05.24 17:34, Jiaxun Yang wrote:
>> >> For LoongArch the start of the image is not the entry
>> >> point to the image.
>> >
>> > Looking at arch/loongarch/kernel/head.S there seem to be two cases:
>> >
>> > * The kernel has an EFI stub (CONFIG_EFI_STUB=y).
>> >    The legacy physical entry point is available at offset 0x08 of the
>> > header.
>> > * The kernel has no EFI stub.
>> >    The kernel entry point matches the start of the image.
>> >
>> > Where do you differentiate between the cases?
>> 
>> Hi Heinrich,
>> 
>> In case there is no EFI stub LoongArch would use elf format vmlinux, there
>> is no real "raw" image for us.
>> 
>> bootelf can't be used at the moment as it doesn't setup FDT and other environments
>> properly.
>> 
>> I'm planning to implement a bootlinuxelf command for this case, as MIPS, LoongArch,
>> xtensa are all using ELF as default kernel image format.
>
> Please note that we have CONFIG_CMD_ELF_FDT_SETUP already, so adjusting
> things so that bootelf works for this case should be doable without a
> new command.

This is a little bit broken as then you'll need to bring architecture specific
functions to bootelf to setup boot registers, initrd etc. It's fine for Arm64 which
you just need to throw fdt into a location in memory but for MIPS and LoongArch 
you have to setup a good deal of other stuff.

Ideally those setups should be done with bootm_run_states as what we've done at
booti, but that would be a semantic to bootelf command. I know many existing
MIPS bare-metal applications rely on argc/argv style bootelf and I don't want to
break them, thus I think a new command is necessary.

Thanks
>
> -- 
> Tom
>
> 附件:
> * signature.asc
Tom Rini June 11, 2024, 2:09 p.m. UTC | #5
On Tue, Jun 11, 2024 at 03:01:00PM +0100, Jiaxun Yang wrote:
> 
> 
> 在2024年6月11日六月 下午2:52,Tom Rini写道:
> > On Tue, Jun 11, 2024 at 02:29:38PM +0100, Jiaxun Yang wrote:
> >> 
> >> 
> >> 在2024年6月11日六月 下午2:02,Heinrich Schuchardt写道:
> >> > On 22.05.24 17:34, Jiaxun Yang wrote:
> >> >> For LoongArch the start of the image is not the entry
> >> >> point to the image.
> >> >
> >> > Looking at arch/loongarch/kernel/head.S there seem to be two cases:
> >> >
> >> > * The kernel has an EFI stub (CONFIG_EFI_STUB=y).
> >> >    The legacy physical entry point is available at offset 0x08 of the
> >> > header.
> >> > * The kernel has no EFI stub.
> >> >    The kernel entry point matches the start of the image.
> >> >
> >> > Where do you differentiate between the cases?
> >> 
> >> Hi Heinrich,
> >> 
> >> In case there is no EFI stub LoongArch would use elf format vmlinux, there
> >> is no real "raw" image for us.
> >> 
> >> bootelf can't be used at the moment as it doesn't setup FDT and other environments
> >> properly.
> >> 
> >> I'm planning to implement a bootlinuxelf command for this case, as MIPS, LoongArch,
> >> xtensa are all using ELF as default kernel image format.
> >
> > Please note that we have CONFIG_CMD_ELF_FDT_SETUP already, so adjusting
> > things so that bootelf works for this case should be doable without a
> > new command.
> 
> This is a little bit broken as then you'll need to bring architecture specific
> functions to bootelf to setup boot registers, initrd etc. It's fine for Arm64 which
> you just need to throw fdt into a location in memory but for MIPS and LoongArch 
> you have to setup a good deal of other stuff.
> 
> Ideally those setups should be done with bootm_run_states as what we've done at
> booti, but that would be a semantic to bootelf command. I know many existing
> MIPS bare-metal applications rely on argc/argv style bootelf and I don't want to
> break them, thus I think a new command is necessary.

We should discuss this in its own thread, yeah. It's been a long while
since I had to bootelf something non-trivial and one of the issues is
that yes, a bunch of quiesce the system functionality wasn't done. Maybe
it's just a matter of adding a flag to bootelf to say we're booting an
OS rather than simple app and so call the normal boot os prep function
chain.
diff mbox series

Patch

diff --git a/arch/arm/lib/image.c b/arch/arm/lib/image.c
index e394c1ad9093..024b6adc75e7 100644
--- a/arch/arm/lib/image.c
+++ b/arch/arm/lib/image.c
@@ -30,7 +30,7 @@  struct Image_header {
 };
 
 int booti_setup(ulong image, ulong *relocated_addr, ulong *size,
-		bool force_reloc)
+		ulong *entry, bool force_reloc)
 {
 	struct Image_header *ih;
 	uint64_t dst;
@@ -73,6 +73,7 @@  int booti_setup(ulong image, ulong *relocated_addr, ulong *size,
 		dst = gd->bd->bi_dram[0].start;
 
 	*relocated_addr = ALIGN(dst, SZ_2M) + text_offset;
+	*entry = *relocated_addr;
 
 	unmap_sysmem(ih);
 
diff --git a/arch/riscv/lib/image.c b/arch/riscv/lib/image.c
index a82f48e9a505..2fd1f6c535ae 100644
--- a/arch/riscv/lib/image.c
+++ b/arch/riscv/lib/image.c
@@ -33,7 +33,7 @@  struct linux_image_h {
 };
 
 int booti_setup(ulong image, ulong *relocated_addr, ulong *size,
-		bool force_reloc)
+		ulong entry, bool force_reloc)
 {
 	struct linux_image_h *lhdr;
 
@@ -56,6 +56,8 @@  int booti_setup(ulong image, ulong *relocated_addr, ulong *size,
 		*relocated_addr = image;
 	}
 
+	*entry = *relocated_addr;
+
 	unmap_sysmem(lhdr);
 
 	return 0;
diff --git a/arch/sandbox/lib/bootm.c b/arch/sandbox/lib/bootm.c
index 44ba8b52e139..4ef34c81d6d2 100644
--- a/arch/sandbox/lib/bootm.c
+++ b/arch/sandbox/lib/bootm.c
@@ -83,7 +83,7 @@  int do_bootm_linux(int flag, struct bootm_info *bmi)
 
 /* used for testing 'booti' command */
 int booti_setup(ulong image, ulong *relocated_addr, ulong *size,
-		bool force_reloc)
+		ulong entry, bool force_reloc)
 {
 	log_err("Booting is not supported on the sandbox.\n");
 
diff --git a/boot/bootm.c b/boot/bootm.c
index 032f5a4a1605..770300132891 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -693,9 +693,10 @@  static int bootm_load_os(struct bootm_headers *images, int boot_progress)
 	    images->os.os == IH_OS_LINUX) {
 		ulong relocated_addr;
 		ulong image_size;
+		ulong entry;
 		int ret;
 
-		ret = booti_setup(load, &relocated_addr, &image_size, false);
+		ret = booti_setup(load, &relocated_addr, &image_size, &entry, false);
 		if (ret) {
 			printf("Failed to prep arm64 kernel (err=%d)\n", ret);
 			return BOOTM_ERR_RESET;
@@ -709,7 +710,7 @@  static int bootm_load_os(struct bootm_headers *images, int boot_progress)
 			memmove((void *)relocated_addr, load_buf, image_size);
 		}
 
-		images->ep = relocated_addr;
+		images->ep = entry;
 		images->os.start = relocated_addr;
 		images->os.end = relocated_addr + image_size;
 	}
diff --git a/cmd/booti.c b/cmd/booti.c
index b9637b3ec3d8..9586a4c58ac1 100644
--- a/cmd/booti.c
+++ b/cmd/booti.c
@@ -27,6 +27,7 @@  static int booti_start(struct bootm_info *bmi)
 	ulong ld;
 	ulong relocated_addr;
 	ulong image_size;
+	ulong entry;
 	uint8_t *temp;
 	ulong dest;
 	ulong dest_end;
@@ -73,7 +74,7 @@  static int booti_start(struct bootm_info *bmi)
 	}
 	unmap_sysmem((void *)ld);
 
-	ret = booti_setup(ld, &relocated_addr, &image_size, false);
+	ret = booti_setup(ld, &relocated_addr, &image_size, &entry, false);
 	if (ret)
 		return 1;
 
@@ -84,7 +85,7 @@  static int booti_start(struct bootm_info *bmi)
 		memmove((void *)relocated_addr, (void *)ld, image_size);
 	}
 
-	images->ep = relocated_addr;
+	images->ep = entry;
 	images->os.start = relocated_addr;
 	images->os.end = relocated_addr + image_size;
 
diff --git a/common/spl/spl.c b/common/spl/spl.c
index e06bc75d36b2..52a4bee13728 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -113,7 +113,8 @@  int __weak bootz_setup(ulong image, ulong *start, ulong *end)
 	 return 1;
 }
 
-int __weak booti_setup(ulong image, ulong *relocated_addr, ulong *size, bool force_reloc)
+int __weak booti_setup(ulong image, ulong *relocated_addr, ulong *size,
+		       ulong *entry, bool force_reloc)
 {
 	 return 1;
 }
@@ -324,13 +325,13 @@  int spl_parse_image_header(struct spl_image_info *spl_image,
 
 #if CONFIG_IS_ENABLED(OS_BOOT)
 #if defined(CMD_BOOTI)
-		ulong start, size;
+		ulong start, size, entry;
 
-		if (!booti_setup((ulong)header, &start, &size, 0)) {
+		if (!booti_setup((ulong)header, &start, &size, &entry, 0)) {
 			spl_image->name = "Linux";
 			spl_image->os = IH_OS_LINUX;
 			spl_image->load_addr = start;
-			spl_image->entry_point = start;
+			spl_image->entry_point = entry;
 			spl_image->size = size;
 			debug(SPL_TPL_PROMPT
 			      "payload Image, load addr: 0x%lx size: %d\n",
diff --git a/include/image.h b/include/image.h
index acffd17e0dfd..a2bfc7bb19a3 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1061,11 +1061,12 @@  int bootz_setup(ulong image, ulong *start, ulong *end);
  * @image: Address of image
  * @start: Returns start address of image
  * @size : Returns size image
+ * @entry: Returns entry point of image
  * @force_reloc: Ignore image->ep field, always place image to RAM start
  * Return: 0 if OK, 1 if the image was not recognised
  */
 int booti_setup(ulong image, ulong *relocated_addr, ulong *size,
-		bool force_reloc);
+		ulong *entry, bool force_reloc);
 
 /*******************************************************************/
 /* New uImage format specific code (prefixed with fit_) */