qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-arm] [Qemu-devel] [PATCH] hw/arm/boot: fix direct kernel boot


From: Philippe Mathieu-Daudé
Subject: Re: [Qemu-arm] [Qemu-devel] [PATCH] hw/arm/boot: fix direct kernel boot setup
Date: Tue, 18 Jun 2019 14:08:13 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1

On 6/18/19 1:55 PM, Andrew Jones wrote:
> On Tue, Jun 18, 2019 at 12:02:37PM +0100, Peter Maydell wrote:
>> On Tue, 18 Jun 2019 at 09:34, Andrew Jones <address@hidden> wrote:
>>>
>>> We need to check ram_end, not ram_size.
>>>
>>> Fixes: 852dc64d665f ("hw/arm/boot: Diagnose layouts that put initrd or
>>> DTB off the end of RAM")
>>> Signed-off-by: Andrew Jones <address@hidden>
>>> ---
>>>  hw/arm/boot.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
>>> index b2f93f6beff6..8a280ab3ed49 100644
>>> --- a/hw/arm/boot.c
>>> +++ b/hw/arm/boot.c
>>> @@ -1109,7 +1109,7 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
>>>                               info->initrd_filename);
>>>                  exit(1);
>>>              }
>>> -            if (info->initrd_start + initrd_size > info->ram_size) {
>>> +            if (info->initrd_start + initrd_size > ram_end) {
>>>                  error_report("could not load initrd '%s': "
>>>                               "too big to fit into RAM after the kernel",
>>>                               info->initrd_filename);
>>> --
>>> 2.20.1
>>
>> Reviewed-by: Peter Maydell <address@hidden>
>>
>> I think I missed this because my test case doesn't have an
>> initrd -- direct kernel boot works fine if all you're
>> passing to QEMU is the kernel... I think we could clarify
>> the commit message a little:
> 
> I found it using kvm-unit-tests which uses a small initrd to
> pass environment variables to the guest. All the tests started
> to report FAIL.
> 
>>
>> hw/arm/boot: fix direct kernel boot with initrd
>>
>> Fix the condition used to check whether the initrd fits
>> into RAM; this meant we were spuriously refusing to do
>> a direct boot of a kernel in some cases if an initrd
>> was also passed on the command line.
> 
> Actually I think we need another fix for this error too. We
> weren't actually refusing do direct boot the kernel, but we
> should have been. We're missing the 'exit(1)' after the error
> message.
> 
>>
>> ?
>>
>> (if you agree I can just fix up the commit message when I apply it.)
> 
> I agree with the improved commit message if we also add the
> 'exit(1)', otherwise "refusing to boot" isn't the right thing
> to say.

Indeed. So for this patch + Peter comment + exit():

Fixes: 852dc64d665
Reviewed-by: Philippe Mathieu-Daudé <address@hidden>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]