* [Qemu-devel] [PATCH] hw/arm/boot: take Linux/arm64 TEXT_OFFSET header field into account
@ 2017-03-09 17:09 Ard Biesheuvel
2017-03-09 17:13 ` no-reply
2017-03-13 11:22 ` Peter Maydell
0 siblings, 2 replies; 4+ messages in thread
From: Ard Biesheuvel @ 2017-03-09 17:09 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: Ard Biesheuvel
The arm64 boot protocol stipulates that the kernel must be loaded
TEXT_OFFSET bytes beyond a 2 MB aligned base address, where TEXT_OFFSET
could be any 4 KB multiple between 0 and 2 MB, and whose value can be
found in the header of the Image file.
So after attempts to load the kernel image as an ELF file or as a
U-Boot image (both of which have their own way of specifying the load
offset) have failed, try to determine the TEXT_OFFSET from the image
before proceeding with loading it as a gzipped or raw image.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
hw/arm/boot.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 56 insertions(+)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index ff621e4b6a4b..0824f74c697f 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -31,6 +31,11 @@
#define KERNEL_LOAD_ADDR 0x00010000
#define KERNEL64_LOAD_ADDR 0x00080000
+#define ARM64_IMAGE_HEADER_SIZE 64
+#define ARM64_TEXT_OFFSET_OFFSET 8
+#define ARM64_IMAGE_SIZE_OFFSET 16
+#define ARM64_MAGIC_OFFSET 56
+
typedef enum {
FIXUP_NONE = 0, /* do nothing */
FIXUP_TERMINATOR, /* end of insns */
@@ -768,6 +773,51 @@ static uint64_t arm_load_elf(struct arm_boot_info *info, uint64_t *pentry,
return ret;
}
+static void aarch64_get_text_offset(struct arm_boot_info *info,
+ hwaddr *text_offset)
+{
+ uint8_t *buffer;
+ uint64_t headerval;
+ int size;
+
+ size = load_image_gzipped_buffer(info->kernel_filename,
+ LOAD_IMAGE_MAX_GUNZIP_BYTES,
+ &buffer);
+
+ if (size < 0) {
+ int fd, bytes;
+
+ fd = open(info->kernel_filename, O_RDONLY | O_BINARY);
+ if (fd < 0)
+ return;
+
+ buffer = g_malloc(ARM64_IMAGE_HEADER_SIZE);
+ bytes = read(fd, buffer, ARM64_IMAGE_HEADER_SIZE);
+ close(fd);
+
+ if (bytes < ARM64_IMAGE_HEADER_SIZE)
+ goto free_buffer;
+ }
+
+ /* check the arm64 magic header value */
+ if (memcmp(buffer + ARM64_MAGIC_OFFSET, "ARM\x64", 4) != 0)
+ goto free_buffer;
+
+ /* The arm64 Image header has text_offset and image_size fields at 8 and
+ * 16 bytes into the Image header, respectively. The text_offset field is
+ * only valid if the image_size is non-zero.
+ */
+ memcpy(&headerval, buffer + ARM64_IMAGE_SIZE_OFFSET, sizeof(headerval));
+ if (headerval == 0)
+ goto free_buffer;
+
+ memcpy(&headerval, buffer + ARM64_TEXT_OFFSET_OFFSET, sizeof(headerval));
+ *text_offset = le64_to_cpu(headerval);
+
+free_buffer:
+ g_free(buffer);
+}
+
static void arm_load_kernel_notify(Notifier *notifier, void *data)
{
CPUState *cs;
@@ -900,6 +950,12 @@ static void arm_load_kernel_notify(Notifier *notifier, void *data)
kernel_size = load_uimage(info->kernel_filename, &entry, NULL,
&is_linux, NULL, NULL);
}
+
+ /* A bare Linux/arm64 kernel carries the load offset in the Image header */
+ if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64) && kernel_size < 0) {
+ aarch64_get_text_offset(info, &kernel_load_offset);
+ }
+
/* On aarch64, it's the bootloader's job to uncompress the kernel. */
if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64) && kernel_size < 0) {
entry = info->loader_start + kernel_load_offset;
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/arm/boot: take Linux/arm64 TEXT_OFFSET header field into account
2017-03-09 17:09 [Qemu-devel] [PATCH] hw/arm/boot: take Linux/arm64 TEXT_OFFSET header field into account Ard Biesheuvel
@ 2017-03-09 17:13 ` no-reply
2017-03-13 11:22 ` Peter Maydell
1 sibling, 0 replies; 4+ messages in thread
From: no-reply @ 2017-03-09 17:13 UTC (permalink / raw)
To: ard.biesheuvel; +Cc: famz, qemu-devel, peter.maydell
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Subject: [Qemu-devel] [PATCH] hw/arm/boot: take Linux/arm64 TEXT_OFFSET header field into account
Message-id: 1489079383-11162-1-git-send-email-ard.biesheuvel@linaro.org
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/1489079383-11162-1-git-send-email-ard.biesheuvel@linaro.org -> patchew/1489079383-11162-1-git-send-email-ard.biesheuvel@linaro.org
- [tag update] patchew/20170309132216.23482-1-dgilbert@redhat.com -> patchew/20170309132216.23482-1-dgilbert@redhat.com
Switched to a new branch 'test'
55ab14d hw/arm/boot: take Linux/arm64 TEXT_OFFSET header field into account
=== OUTPUT BEGIN ===
Checking PATCH 1/1: hw/arm/boot: take Linux/arm64 TEXT_OFFSET header field into account...
ERROR: braces {} are necessary for all arms of this statement
#55: FILE: hw/arm/boot.c:791:
+ if (fd < 0)
[...]
ERROR: braces {} are necessary for all arms of this statement
#62: FILE: hw/arm/boot.c:798:
+ if (bytes < ARM64_IMAGE_HEADER_SIZE)
[...]
ERROR: braces {} are necessary for all arms of this statement
#67: FILE: hw/arm/boot.c:803:
+ if (memcmp(buffer + ARM64_MAGIC_OFFSET, "ARM\x64", 4) != 0)
[...]
ERROR: braces {} are necessary for all arms of this statement
#75: FILE: hw/arm/boot.c:811:
+ if (headerval == 0)
[...]
total: 4 errors, 0 warnings, 74 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/arm/boot: take Linux/arm64 TEXT_OFFSET header field into account
2017-03-09 17:09 [Qemu-devel] [PATCH] hw/arm/boot: take Linux/arm64 TEXT_OFFSET header field into account Ard Biesheuvel
2017-03-09 17:13 ` no-reply
@ 2017-03-13 11:22 ` Peter Maydell
2017-03-13 11:25 ` Ard Biesheuvel
1 sibling, 1 reply; 4+ messages in thread
From: Peter Maydell @ 2017-03-13 11:22 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: QEMU Developers
On 9 March 2017 at 18:09, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> The arm64 boot protocol stipulates that the kernel must be loaded
> TEXT_OFFSET bytes beyond a 2 MB aligned base address, where TEXT_OFFSET
> could be any 4 KB multiple between 0 and 2 MB, and whose value can be
> found in the header of the Image file.
>
> So after attempts to load the kernel image as an ELF file or as a
> U-Boot image (both of which have their own way of specifying the load
> offset) have failed, try to determine the TEXT_OFFSET from the image
> before proceeding with loading it as a gzipped or raw image.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> hw/arm/boot.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 56 insertions(+)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index ff621e4b6a4b..0824f74c697f 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -31,6 +31,11 @@
> #define KERNEL_LOAD_ADDR 0x00010000
> #define KERNEL64_LOAD_ADDR 0x00080000
>
> +#define ARM64_IMAGE_HEADER_SIZE 64
> +#define ARM64_TEXT_OFFSET_OFFSET 8
> +#define ARM64_IMAGE_SIZE_OFFSET 16
> +#define ARM64_MAGIC_OFFSET 56
> +
> typedef enum {
> FIXUP_NONE = 0, /* do nothing */
> FIXUP_TERMINATOR, /* end of insns */
> @@ -768,6 +773,51 @@ static uint64_t arm_load_elf(struct arm_boot_info *info, uint64_t *pentry,
> return ret;
> }
>
> +static void aarch64_get_text_offset(struct arm_boot_info *info,
> + hwaddr *text_offset)
> +{
Hmm, this isn't quite what I had in mind when we talked about this.
I think what we want is a function like
static uint64_t load_aarch64_image(const char *filename, hwaddr *entry,
other stuff you need)
which loads the image if it's the right format and writes
the entry point into *entry, and returns a negative value
if the image isn't the right format.
ie whose API is basically parallel to the load_uimage(),
load_image_gzipped(), etc functions. You can make it handle
both gzipped and non-gzipped images (and then drop the code
we currently have to call load_image_gzipped()).
PS: you'll want to use g_file_get_contents() for the load
in the non-compressed image case, and as the patchew
robot points out our coding style preferences mandate braces.
thanks
-- PMM
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/arm/boot: take Linux/arm64 TEXT_OFFSET header field into account
2017-03-13 11:22 ` Peter Maydell
@ 2017-03-13 11:25 ` Ard Biesheuvel
0 siblings, 0 replies; 4+ messages in thread
From: Ard Biesheuvel @ 2017-03-13 11:25 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers
On 13 March 2017 at 11:22, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 9 March 2017 at 18:09, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> The arm64 boot protocol stipulates that the kernel must be loaded
>> TEXT_OFFSET bytes beyond a 2 MB aligned base address, where TEXT_OFFSET
>> could be any 4 KB multiple between 0 and 2 MB, and whose value can be
>> found in the header of the Image file.
>>
>> So after attempts to load the kernel image as an ELF file or as a
>> U-Boot image (both of which have their own way of specifying the load
>> offset) have failed, try to determine the TEXT_OFFSET from the image
>> before proceeding with loading it as a gzipped or raw image.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> hw/arm/boot.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 56 insertions(+)
>>
>> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
>> index ff621e4b6a4b..0824f74c697f 100644
>> --- a/hw/arm/boot.c
>> +++ b/hw/arm/boot.c
>> @@ -31,6 +31,11 @@
>> #define KERNEL_LOAD_ADDR 0x00010000
>> #define KERNEL64_LOAD_ADDR 0x00080000
>>
>> +#define ARM64_IMAGE_HEADER_SIZE 64
>> +#define ARM64_TEXT_OFFSET_OFFSET 8
>> +#define ARM64_IMAGE_SIZE_OFFSET 16
>> +#define ARM64_MAGIC_OFFSET 56
>> +
>> typedef enum {
>> FIXUP_NONE = 0, /* do nothing */
>> FIXUP_TERMINATOR, /* end of insns */
>> @@ -768,6 +773,51 @@ static uint64_t arm_load_elf(struct arm_boot_info *info, uint64_t *pentry,
>> return ret;
>> }
>>
>> +static void aarch64_get_text_offset(struct arm_boot_info *info,
>> + hwaddr *text_offset)
>> +{
>
> Hmm, this isn't quite what I had in mind when we talked about this.
> I think what we want is a function like
> static uint64_t load_aarch64_image(const char *filename, hwaddr *entry,
> other stuff you need)
> which loads the image if it's the right format and writes
> the entry point into *entry, and returns a negative value
> if the image isn't the right format.
>
> ie whose API is basically parallel to the load_uimage(),
> load_image_gzipped(), etc functions. You can make it handle
> both gzipped and non-gzipped images (and then drop the code
> we currently have to call load_image_gzipped()).
>
OK, that sounds reasonable.
> PS: you'll want to use g_file_get_contents() for the load
> in the non-compressed image case, and as the patchew
> robot points out our coding style preferences mandate braces.
>
Yes. TBH I contribute to so many different projects, it's hard to keep
track of such things, and so I simply looked elsewhere in the file for
examples.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-03-13 11:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-09 17:09 [Qemu-devel] [PATCH] hw/arm/boot: take Linux/arm64 TEXT_OFFSET header field into account Ard Biesheuvel
2017-03-09 17:13 ` no-reply
2017-03-13 11:22 ` Peter Maydell
2017-03-13 11:25 ` Ard Biesheuvel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.