Re: [Xen-devel] [PATCH 2/4] tools/dombuilder: Remove PV-only, mandatory hooks

[Julien Grall <julien@xen.org>]

Hi Andrew,

On 17/12/2019 21:15, Andrew Cooper wrote:
> Currently, the setup_pgtable() hook is optional, but alloc_pgtable() hook is
> not.  Both are specific to x86 PV guests, and stubbed in various ways by the
> dombuilders for translated guests (x86 HVM, ARM).
> 
> Make alloc_pgtables() optional, and drop all the stubs for translated guest
> types.
> 
> No change in the constructed guests.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Julien Grall <julien@xen.org>

Cheers,

> ---
> CC: Ian Jackson <Ian.Jackson@citrix.com>
> CC: Wei Liu <wl@xen.org>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien@xen.org>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
> CC: Varad Gautam <vrd@amazon.de>
> ---
>   tools/libxc/include/xc_dom.h |  3 ++-
>   tools/libxc/xc_dom_arm.c     | 21 ---------------------
>   tools/libxc/xc_dom_boot.c    |  6 +++---
>   tools/libxc/xc_dom_core.c    |  3 ++-
>   tools/libxc/xc_dom_x86.c     |  7 -------
>   5 files changed, 7 insertions(+), 33 deletions(-)
> 
> diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
> index 5900bbe8fa..9ff1cb8b07 100644
> --- a/tools/libxc/include/xc_dom.h
> +++ b/tools/libxc/include/xc_dom.h
> @@ -253,8 +253,9 @@ void xc_dom_register_loader(struct xc_dom_loader *loader);
>   /* --- arch specific hooks ----------------------------------------- */
>   
>   struct xc_dom_arch {
> -    /* pagetable setup */
>       int (*alloc_magic_pages) (struct xc_dom_image * dom);
> +
> +    /* pagetable setup - x86 PV only */
>       int (*alloc_pgtables) (struct xc_dom_image * dom);
>       int (*alloc_p2m_list) (struct xc_dom_image * dom);
>       int (*setup_pgtables) (struct xc_dom_image * dom);
> diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
> index 5b9eca6087..7e0fb9169f 100644
> --- a/tools/libxc/xc_dom_arm.c
> +++ b/tools/libxc/xc_dom_arm.c
> @@ -47,23 +47,6 @@ const char *xc_domain_get_native_protocol(xc_interface *xch,
>   }
>   
>   /* ------------------------------------------------------------------------ */
> -/*
> - * arm guests are hybrid and start off with paging disabled, therefore no
> - * pagetables and nothing to do here.
> - */
> -static int alloc_pgtables_arm(struct xc_dom_image *dom)
> -{
> -    DOMPRINTF_CALLED(dom->xch);
> -    return 0;
> -}
> -
> -static int setup_pgtables_arm(struct xc_dom_image *dom)
> -{
> -    DOMPRINTF_CALLED(dom->xch);
> -    return 0;
> -}
> -
> -/* ------------------------------------------------------------------------ */
>   
>   static int alloc_magic_pages(struct xc_dom_image *dom)
>   {
> @@ -539,8 +522,6 @@ static struct xc_dom_arch xc_dom_32 = {
>       .page_shift = PAGE_SHIFT_ARM,
>       .sizeof_pfn = 8,
>       .alloc_magic_pages = alloc_magic_pages,
> -    .alloc_pgtables = alloc_pgtables_arm,
> -    .setup_pgtables = setup_pgtables_arm,
>       .start_info = start_info_arm,
>       .shared_info = shared_info_arm,
>       .vcpu = vcpu_arm32,
> @@ -555,8 +536,6 @@ static struct xc_dom_arch xc_dom_64 = {
>       .page_shift = PAGE_SHIFT_ARM,
>       .sizeof_pfn = 8,
>       .alloc_magic_pages = alloc_magic_pages,
> -    .alloc_pgtables = alloc_pgtables_arm,
> -    .setup_pgtables = setup_pgtables_arm,
>       .start_info = start_info_arm,
>       .shared_info = shared_info_arm,
>       .vcpu = vcpu_arm64,
> diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c
> index 918ee4d045..79dbbf6571 100644
> --- a/tools/libxc/xc_dom_boot.c
> +++ b/tools/libxc/xc_dom_boot.c
> @@ -199,9 +199,9 @@ int xc_dom_boot_image(struct xc_dom_image *dom)
>       /* initial mm setup */
>       if ( (rc = xc_dom_update_guest_p2m(dom)) != 0 )
>           return rc;
> -    if ( dom->arch_hooks->setup_pgtables )
> -        if ( (rc = dom->arch_hooks->setup_pgtables(dom)) != 0 )
> -            return rc;
> +    if ( dom->arch_hooks->setup_pgtables &&
> +         (rc = dom->arch_hooks->setup_pgtables(dom)) != 0 )
> +        return rc;
>   
>       /* start info page */
>       if ( dom->arch_hooks->start_info )
> diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c
> index 9bd04cb2d5..fc77804a7e 100644
> --- a/tools/libxc/xc_dom_core.c
> +++ b/tools/libxc/xc_dom_core.c
> @@ -1247,7 +1247,8 @@ int xc_dom_build_image(struct xc_dom_image *dom)
>           goto err;
>       if ( dom->arch_hooks->alloc_magic_pages(dom) != 0 )
>           goto err;
> -    if ( dom->arch_hooks->alloc_pgtables(dom) != 0 )
> +    if ( dom->arch_hooks->alloc_pgtables &&
> +         dom->arch_hooks->alloc_pgtables(dom) != 0 )
>           goto err;
>       if ( dom->alloc_bootstack )
>       {
> diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
> index 1ce3c798ef..d2acff1061 100644
> --- a/tools/libxc/xc_dom_x86.c
> +++ b/tools/libxc/xc_dom_x86.c
> @@ -1690,12 +1690,6 @@ static int bootlate_pv(struct xc_dom_image *dom)
>       return 0;
>   }
>   
> -static int alloc_pgtables_hvm(struct xc_dom_image *dom)
> -{
> -    DOMPRINTF("%s: doing nothing", __func__);
> -    return 0;
> -}
> -
>   /*
>    * The memory layout of the start_info page and the modules, and where the
>    * addresses are stored:
> @@ -1906,7 +1900,6 @@ static struct xc_dom_arch xc_hvm_32 = {
>       .page_shift = PAGE_SHIFT_X86,
>       .sizeof_pfn = 4,
>       .alloc_magic_pages = alloc_magic_pages_hvm,
> -    .alloc_pgtables = alloc_pgtables_hvm,
>       .vcpu = vcpu_hvm,
>       .meminit = meminit_hvm,
>       .bootearly = bootearly,
> 

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel