Re: [PATCH v4 08/19] xen: add basic hooks for PVH in current code

[Daniel Kiper]

On Fri, Nov 02, 2018 at 01:37:27PM +0100, Juergen Gross wrote:
> Add the hooks to current code needed for Xen PVH. They will be filled
> with code later when the related functionality is being added.
>
> Signed-off-by: Juergen Gross <address@hidden>
> ---
> V3: xenpvh->xen_pvh (Daniel Kiper)
>     adjust copyright date (Roger Pau Monné)
> ---
>  grub-core/kern/i386/xen/pvh.c         | 36 
> +++++++++++++++++++++++++++++++++++
>  grub-core/kern/i386/xen/startup_pvh.S | 29 ++++++++++++++++++++++++++++
>  grub-core/kern/xen/init.c             |  6 ++++++
>  grub-core/loader/i386/linux.c         |  1 +
>  include/grub/i386/xen_pvh/kernel.h    | 30 +++++++++++++++++++++++++++++
>  include/grub/xen.h                    |  6 ++++++
>  6 files changed, 108 insertions(+)
>  create mode 100644 grub-core/kern/i386/xen/pvh.c
>  create mode 100644 grub-core/kern/i386/xen/startup_pvh.S
>  create mode 100644 include/grub/i386/xen_pvh/kernel.h
>
> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> new file mode 100644
> index 000000000..ac6181f4e
> --- /dev/null
> +++ b/grub-core/kern/i386/xen/pvh.c
> @@ -0,0 +1,36 @@
> +/*
> + *  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/>.
> + */
> +
> +#include <grub/kernel.h>
> +#include <grub/misc.h>
> +#include <grub/memory.h>
> +#include <grub/mm.h>
> +#include <grub/xen.h>
> +#include <grub/machine/kernel.h>
> +
> +grub_uint64_t grub_rsdp_addr;
> +
> +void
> +grub_xen_setup_pvh (void)
> +{
> +}
> +
> +grub_err_t
> +grub_machine_mmap_iterate (grub_memory_hook_t hook, void *hook_data)
> +{
> +}
> diff --git a/grub-core/kern/i386/xen/startup_pvh.S 
> b/grub-core/kern/i386/xen/startup_pvh.S
> new file mode 100644
> index 000000000..69b8fdcca
> --- /dev/null
> +++ b/grub-core/kern/i386/xen/startup_pvh.S
> @@ -0,0 +1,29 @@
> +/* startup.S - bootstrap GRUB itself */
> +/*
> + *  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/>.
> + */
> +
> +#include <config.h>
> +#include <grub/symbol.h>
> +
> +     .file   "startup_pvh.S"
> +     .text
> +
> +/* Saved pointer to start info structure. */
> +     .globl  pvh_start_info
> +pvh_start_info:
> +     .long   0
> diff --git a/grub-core/kern/xen/init.c b/grub-core/kern/xen/init.c
> index 10007b411..782ca7295 100644
> --- a/grub-core/kern/xen/init.c
> +++ b/grub-core/kern/xen/init.c
> @@ -45,6 +45,8 @@ grub_xen_ptr2mfn (void *ptr)
>    grub_xen_mfn_t *mfn_list =
>      (grub_xen_mfn_t *) grub_xen_start_page_addr->mfn_list;
>    return mfn_list[(grub_addr_t) ptr >> GRUB_XEN_LOG_PAGE_SIZE];
> +#else
> +  return (grub_addr_t) ptr >> GRUB_XEN_LOG_PAGE_SIZE;

It seems to me that this change does not belong to this patch.

>  #endif
>  }
>
> @@ -562,6 +564,10 @@ grub_machine_init (void)
>                          + GRUB_KERNEL_MACHINE_MOD_GAP,
>                          GRUB_KERNEL_MACHINE_MOD_ALIGN);
>
> +#ifdef GRUB_MACHINE_XEN_PVH
> +  grub_xen_setup_pvh ();
> +#endif
> +
>    grub_xen_setup_gnttab ();
>
>  #ifdef GRUB_MACHINE_XEN
> diff --git a/grub-core/loader/i386/linux.c b/grub-core/loader/i386/linux.c
> index 51920896e..f96309476 100644
> --- a/grub-core/loader/i386/linux.c
> +++ b/grub-core/loader/i386/linux.c
> @@ -35,6 +35,7 @@
>  #include <grub/i18n.h>
>  #include <grub/lib/cmdline.h>
>  #include <grub/linux.h>
> +#include <grub/machine/kernel.h>

Please say in the commit message why you include grub/machine/kernel.h here.

>  GRUB_MOD_LICENSE ("GPLv3+");
>
> diff --git a/include/grub/i386/xen_pvh/kernel.h 
> b/include/grub/i386/xen_pvh/kernel.h
> new file mode 100644
> index 000000000..2b7b8a129
> --- /dev/null
> +++ b/include/grub/i386/xen_pvh/kernel.h
> @@ -0,0 +1,30 @@
> +/*
> + *  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_KERNEL_MACHINE_HEADER
> +#define GRUB_KERNEL_MACHINE_HEADER   1
> +
> +#ifndef ASM_FILE
> +
> +#define GRUB_KERNEL_USE_RSDP_ADDR    1
> +
> +extern grub_uint64_t EXPORT_VAR(grub_rsdp_addr);
> +
> +#endif /* ! ASM_FILE */
> +
> +#endif /* GRUB_KERNEL_MACHINE_HEADER */
> diff --git a/include/grub/xen.h b/include/grub/xen.h
> index c31cc10c7..4cb2c8f43 100644
> --- a/include/grub/xen.h
> +++ b/include/grub/xen.h
> @@ -43,6 +43,7 @@ typedef grub_uint64_t uint64_t;
>
>  #include <xen/sched.h>
>  #include <xen/grant_table.h>
> +#include <xen/hvm/start_info.h>

Hmmm... Why?

Daniel