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

[Peter Maydell]

On Tue, 18 Jun 2019 at 12:56, Andrew Jones <address@hidden> 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.

Hmm, so we are. Do you want to send a v2 then, which fixes
that too and fixes up the commit message?

thanks
-- PMM