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

[Andrew Jones]

On Tue, Jun 18, 2019 at 02:50:15PM +0200, Andrew Jones wrote:
> On Tue, Jun 18, 2019 at 12:58:30PM +0100, Peter Maydell wrote:
> > 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?
> 
> On it's way.
>

Argh, just fired it off by forgot the '-v2' on my format-patch so
the subject tag won't be right. Sorry about that.

drew