Re: [Qemu-devel] [RFC v1 1/5] hw/riscv: Split out the boot functions

[Alistair Francis]

On Wed, Jun 19, 2019 at 8:16 AM Bin Meng <address@hidden> wrote:
>
> On Wed, Jun 19, 2019 at 8:51 AM Alistair Francis
> <address@hidden> wrote:
> >
> > Split the common RISC-V boot functions into a seperate file. This allows
> > us to share the common code.
> >
> > Signed-off-by: Alistair Francis <address@hidden>
> > ---
> >  hw/riscv/Makefile.objs  |  1 +
> >  hw/riscv/boot.c         | 69 +++++++++++++++++++++++++++++++++++++++++
> >  hw/riscv/sifive_e.c     | 17 ++--------
> >  hw/riscv/sifive_u.c     | 17 ++--------
> >  hw/riscv/spike.c        | 21 +++----------
> >  hw/riscv/virt.c         | 51 +++---------------------------
> >  include/hw/riscv/boot.h | 27 ++++++++++++++++
> >  7 files changed, 110 insertions(+), 93 deletions(-)
> >  create mode 100644 hw/riscv/boot.c
> >  create mode 100644 include/hw/riscv/boot.h
> >
> > diff --git a/hw/riscv/Makefile.objs b/hw/riscv/Makefile.objs
> > index a65027304a..eb9d4f9ffc 100644
> > --- a/hw/riscv/Makefile.objs
> > +++ b/hw/riscv/Makefile.objs
> > @@ -1,3 +1,4 @@
> > +obj-y += boot.o
> >  obj-$(CONFIG_SPIKE) += riscv_htif.o
> >  obj-$(CONFIG_HART) += riscv_hart.o
> >  obj-$(CONFIG_SIFIVE_E) += sifive_e.o
> > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> > new file mode 100644
> > index 0000000000..62f94aaf8a
> > --- /dev/null
> > +++ b/hw/riscv/boot.c
> > @@ -0,0 +1,69 @@
> > +/*
> > + * QEMU RISC-V Boot Helper
> > + *
> > + * Copyright (c) 2017 SiFive, Inc.
> > + * Copyright (c) 2019 Alistair Francis <address@hidden>
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License 
> > for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along 
> > with
> > + * this program.  If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu/units.h"
> > +#include "qemu/error-report.h"
> > +#include "exec/cpu-defs.h"
> > +#include "hw/loader.h"
> > +#include "hw/riscv/boot.h"
> > +#include "elf.h"
> > +
> > +target_ulong riscv_load_kernel(const char *kernel_filename)
> > +{
> > +    uint64_t kernel_entry, kernel_high;
> > +
> > +    if (load_elf(kernel_filename, NULL, NULL, NULL,
> > +                 &kernel_entry, NULL, &kernel_high,
> > +                 0, EM_RISCV, 1, 0) < 0) {
> > +        error_report("could not load kernel '%s'", kernel_filename);
> > +        exit(1);
> > +    }
> > +
> > +    return kernel_entry;
> > +}
> > +
> > +hwaddr riscv_load_initrd(const char *filename, uint64_t mem_size,
> > +                         uint64_t kernel_entry, hwaddr *start)
> > +{
> > +    int size;
> > +
> > +    /* We want to put the initrd far enough into RAM that when the
>
> nits: /* should be on a separate line

Fixed

>
> > +     * kernel is uncompressed it will not clobber the initrd. However
> > +     * on boards without much RAM we must ensure that we still leave
> > +     * enough room for a decent sized initrd, and on boards with large
> > +     * amounts of RAM we must avoid the initrd being so far up in RAM
> > +     * that it is outside lowmem and inaccessible to the kernel.
> > +     * So for boards with less  than 256MB of RAM we put the initrd
> > +     * halfway into RAM, and for boards with 256MB of RAM or more we put
> > +     * the initrd at 128MB.
> > +     */
>
> [snip]
>
> Reviewed-by: Bin Meng <address@hidden>
> Tested-by: Bin Meng <address@hidden>

Thanks!

Alistair