[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v6 06/11] RISC-V: Add Linux load logic
From: |
Daniel Kiper |
Subject: |
Re: [PATCH v6 06/11] RISC-V: Add Linux load logic |
Date: |
Mon, 18 Feb 2019 20:28:09 +0100 |
User-agent: |
NeoMutt/20170113 (1.7.2) |
On Tue, Feb 12, 2019 at 11:31:03AM +0100, Alexander Graf wrote:
> We currently only support to run grub on RISC-V as UEFI payload. Ideally,
> we also only want to support running Linux underneath as UEFI payload.
>
> Prepare that with some Linux boot stub code. Once the arm64 target is
> generalized, we can hook into that one and gain boot functionality.
>
> Signed-off-by: Alexander Graf <address@hidden>
Reviewed-by: Daniel Kiper <address@hidden>
But two nitpicks below...
[...]
> diff --git a/include/grub/riscv32/linux.h b/include/grub/riscv32/linux.h
> new file mode 100644
> index 000000000..b8ed39407
> --- /dev/null
> +++ b/include/grub/riscv32/linux.h
> @@ -0,0 +1,41 @@
> +/*
> + * GRUB -- GRand Unified Bootloader
> + * Copyright (C) 2018 Free Software Foundation, Inc.
> + *
> + * GRUB is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * GRUB is distributed in the hope that 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 GRUB. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef GRUB_RISCV32_LINUX_HEADER
> +#define GRUB_RISCV32_LINUX_HEADER 1
> +
> +#define GRUB_LINUX_RISCV_MAGIC_SIGNATURE 0x52534356 /* 'RSCV' */
> +
> +/* From linux/Documentation/riscv/booting.txt */
> +struct linux_riscv_kernel_header
> +{
> + grub_uint32_t code0; /* Executable code */
> + grub_uint32_t code1; /* Executable code */
> + grub_uint64_t text_offset; /* Image load offset */
> + grub_uint64_t res0; /* reserved */
> + grub_uint64_t res1; /* reserved */
> + grub_uint64_t res2; /* reserved */
> + grub_uint64_t res3; /* reserved */
> + grub_uint64_t res4; /* reserved */
> + grub_uint32_t magic; /* Magic number, little endian, "RSCV"
> */
> + grub_uint32_t hdr_offset; /* Offset of PE/COFF header */
> +};
> +
> +# define linux_arch_kernel_header linux_riscv_kernel_header
I do not think that we need a space between "#" and "define".
> +#endif /* ! GRUB_RISCV32_LINUX_HEADER */
> diff --git a/include/grub/riscv64/linux.h b/include/grub/riscv64/linux.h
> new file mode 100644
> index 000000000..29140e45e
> --- /dev/null
> +++ b/include/grub/riscv64/linux.h
> @@ -0,0 +1,43 @@
> +/*
> + * GRUB -- GRand Unified Bootloader
> + * Copyright (C) 2018 Free Software Foundation, Inc.
> + *
> + * GRUB is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * GRUB is distributed in the hope that 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 GRUB. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef GRUB_RISCV64_LINUX_HEADER
> +#define GRUB_RISCV64_LINUX_HEADER 1
> +
> +#define GRUB_LINUX_RISCV_MAGIC_SIGNATURE 0x52534356 /* 'RSCV' */
> +
> +#define GRUB_EFI_PE_MAGIC 0x5A4D
> +
> +/* From linux/Documentation/riscv/booting.txt */
> +struct linux_riscv_kernel_header
> +{
> + grub_uint32_t code0; /* Executable code */
> + grub_uint32_t code1; /* Executable code */
> + grub_uint64_t text_offset; /* Image load offset */
> + grub_uint64_t res0; /* reserved */
> + grub_uint64_t res1; /* reserved */
> + grub_uint64_t res2; /* reserved */
> + grub_uint64_t res3; /* reserved */
> + grub_uint64_t res4; /* reserved */
> + grub_uint32_t magic; /* Magic number, little endian, "RSCV"
> */
> + grub_uint32_t hdr_offset; /* Offset of PE/COFF header */
> +};
> +
> +# define linux_arch_kernel_header linux_riscv_kernel_header
Ditto. I can fix it before committing this patch. Are you OK with that?
Daniel
[PATCH v6 07/11] RISC-V: Add awareness for RISC-V reloations, Alexander Graf, 2019/02/12