diff mbox

[v2] Make -kernel flag optional on ARM.

Message ID 1373444183-11557-1-git-send-email-grant.likely@linaro.org
State New
Headers show

Commit Message

Grant Likely July 10, 2013, 8:16 a.m. UTC
Sometimes we want to boot the system via firmware instead of loading a
kernel into ram with the -kernel parameter. This patch makes the -kernel
parameter optional so that a bios image provided by the -pflash flag
will be executed.

For example:
qemu-system-arm -M vexpress-a15 -pflash <filename>

Note: Currently the file must be at least the size of the emulated flash
device (ie 64M for VExpress) otherwise QEMU will silently not use the
data. This will be fixed in a separate patch

v2: just return if the kernel filename isn't provided

Signed-off-by: Grant Likely <grant.likely@linaro.org>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-devel@nongnu.org
---
 hw/arm/boot.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Comments

Grant Likely Aug. 30, 2013, 10:58 a.m. UTC | #1
Hi Peter,

What's the status on this patch? Is it able to be merged?

g.

On Wed, Jul 10, 2013 at 9:16 AM, Grant Likely <grant.likely@linaro.org> wrote:
> Sometimes we want to boot the system via firmware instead of loading a
> kernel into ram with the -kernel parameter. This patch makes the -kernel
> parameter optional so that a bios image provided by the -pflash flag
> will be executed.
>
> For example:
> qemu-system-arm -M vexpress-a15 -pflash <filename>
>
> Note: Currently the file must be at least the size of the emulated flash
> device (ie 64M for VExpress) otherwise QEMU will silently not use the
> data. This will be fixed in a separate patch
>
> v2: just return if the kernel filename isn't provided
>
> Signed-off-by: Grant Likely <grant.likely@linaro.org>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: qemu-devel@nongnu.org
> ---
>  hw/arm/boot.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 7c0090f..e702fd7 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -361,11 +361,8 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
>      int big_endian;
>      QemuOpts *machine_opts;
>
> -    /* Load the kernel.  */
> -    if (!info->kernel_filename) {
> -        fprintf(stderr, "Kernel image must be specified\n");
> -        exit(1);
> -    }
> +    if (!info->kernel_filename)
> +        return;
>
>      machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
>      if (machine_opts) {
> --
> 1.8.1.2
>
Andreas Färber Aug. 30, 2013, 1:12 p.m. UTC | #2
Hi,

Am 30.08.2013 12:58, schrieb Grant Likely:
> Hi Peter,
> 
> What's the status on this patch? Is it able to be merged?

I had posted a slightly different patch earlier that just returned
immediately when qtest_enabled(). If we go with yours, we might be able
to drop mine, have you checked on that?

As for your patch I wonder, isn't there anything else to do in the
kernel_filename==NULL case?

Andreas

> On Wed, Jul 10, 2013 at 9:16 AM, Grant Likely <grant.likely@linaro.org> wrote:
>> Sometimes we want to boot the system via firmware instead of loading a
>> kernel into ram with the -kernel parameter. This patch makes the -kernel
>> parameter optional so that a bios image provided by the -pflash flag
>> will be executed.
>>
>> For example:
>> qemu-system-arm -M vexpress-a15 -pflash <filename>
>>
>> Note: Currently the file must be at least the size of the emulated flash
>> device (ie 64M for VExpress) otherwise QEMU will silently not use the
>> data. This will be fixed in a separate patch
>>
>> v2: just return if the kernel filename isn't provided
>>
>> Signed-off-by: Grant Likely <grant.likely@linaro.org>
>> Cc: Peter Maydell <peter.maydell@linaro.org>
>> Cc: qemu-devel@nongnu.org
>> ---
>>  hw/arm/boot.c | 7 ++-----
>>  1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
>> index 7c0090f..e702fd7 100644
>> --- a/hw/arm/boot.c
>> +++ b/hw/arm/boot.c
>> @@ -361,11 +361,8 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
>>      int big_endian;
>>      QemuOpts *machine_opts;
>>
>> -    /* Load the kernel.  */
>> -    if (!info->kernel_filename) {
>> -        fprintf(stderr, "Kernel image must be specified\n");
>> -        exit(1);
>> -    }
>> +    if (!info->kernel_filename)
>> +        return;
>>
>>      machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
>>      if (machine_opts) {
>> --
>> 1.8.1.2
Grant Likely Aug. 30, 2013, 1:24 p.m. UTC | #3
On Fri, Aug 30, 2013 at 2:12 PM, Andreas Färber <afaerber@suse.de> wrote:
> Hi,
>
> Am 30.08.2013 12:58, schrieb Grant Likely:
>> Hi Peter,
>>
>> What's the status on this patch? Is it able to be merged?
>
> I had posted a slightly different patch earlier that just returned
> immediately when qtest_enabled(). If we go with yours, we might be able
> to drop mine, have you checked on that?
>
> As for your patch I wonder, isn't there anything else to do in the
> kernel_filename==NULL case?

Not that I've been able to find, although it would be nice to have a
warning if neither a kernel or NOR image is provided.

g.

>
> Andreas
>
>> On Wed, Jul 10, 2013 at 9:16 AM, Grant Likely <grant.likely@linaro.org> wrote:
>>> Sometimes we want to boot the system via firmware instead of loading a
>>> kernel into ram with the -kernel parameter. This patch makes the -kernel
>>> parameter optional so that a bios image provided by the -pflash flag
>>> will be executed.
>>>
>>> For example:
>>> qemu-system-arm -M vexpress-a15 -pflash <filename>
>>>
>>> Note: Currently the file must be at least the size of the emulated flash
>>> device (ie 64M for VExpress) otherwise QEMU will silently not use the
>>> data. This will be fixed in a separate patch
>>>
>>> v2: just return if the kernel filename isn't provided
>>>
>>> Signed-off-by: Grant Likely <grant.likely@linaro.org>
>>> Cc: Peter Maydell <peter.maydell@linaro.org>
>>> Cc: qemu-devel@nongnu.org
>>> ---
>>>  hw/arm/boot.c | 7 ++-----
>>>  1 file changed, 2 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
>>> index 7c0090f..e702fd7 100644
>>> --- a/hw/arm/boot.c
>>> +++ b/hw/arm/boot.c
>>> @@ -361,11 +361,8 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
>>>      int big_endian;
>>>      QemuOpts *machine_opts;
>>>
>>> -    /* Load the kernel.  */
>>> -    if (!info->kernel_filename) {
>>> -        fprintf(stderr, "Kernel image must be specified\n");
>>> -        exit(1);
>>> -    }
>>> +    if (!info->kernel_filename)
>>> +        return;
>>>
>>>      machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
>>>      if (machine_opts) {
>>> --
>>> 1.8.1.2
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Peter Maydell Aug. 30, 2013, 1:29 p.m. UTC | #4
On 30 August 2013 14:12, Andreas Färber <afaerber@suse.de> wrote:
> Hi,
>
> Am 30.08.2013 12:58, schrieb Grant Likely:
>> Hi Peter,
>>
>> What's the status on this patch? Is it able to be merged?

I had a variant on it I'd already written, plus a second patch
that cleans up some places that no longer need to make calls
to arm_load_kernel() be conditional on kernel_filename not
being NULL, but I'm afraid I didn't get to sending it out before
1.6 freeze and I forgot about it in the interim.

Also I couldn't decide what I thought about the way that boards
end up with not warning if the user doesn't specify -kernel or
provide a boot rom image somehow. I'd like to be user friendly
but it doesn't seem right that every board init function ends up
having to have similar boilerplate either.

> I had posted a slightly different patch earlier that just returned
> immediately when qtest_enabled(). If we go with yours, we might be able
> to drop mine, have you checked on that?
>
> As for your patch I wonder, isn't there anything else to do in the
> kernel_filename==NULL case?

Nope, that's it. In that case we'll just do what the hardware
does, ie all CPUs start executing at the reset vector address
(zero). The assumption is the boot firmware deals with that,
as it has to for h/w.

-- PMM
diff mbox

Patch

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 7c0090f..e702fd7 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -361,11 +361,8 @@  void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
     int big_endian;
     QemuOpts *machine_opts;
 
-    /* Load the kernel.  */
-    if (!info->kernel_filename) {
-        fprintf(stderr, "Kernel image must be specified\n");
-        exit(1);
-    }
+    if (!info->kernel_filename)
+        return;
 
     machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
     if (machine_opts) {