Re: [Minios-devel] [UNIKRAFT PATCH v2] plat/xen: Grant table support as menu option

[Sharan Santhanam <sharan.santhanam@xxxxxxxxx>]

Hello Simon,

Please find the comment inline.

Thanks & Regards
Sharan

On 2/11/19 3:48 PM, Simon Kuenzer wrote:
> Introduces a menu option to enable or disable grant table support.
> The reason is that our grant table implementation depends on
> libuklock, libuksched, and libukalloc. A minimal requires these
> libraries selected, too. This may be an obstacle when running driver
> implementation in higher-level languages. Additionally, some of
> these libraries are currenlty not supported on Arm. It is
> wiser to give an option for disabling grant table support.
>
> Signed-off-by: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> ---
>   plat/xen/Config.uk               | 20 +++++++++++++++-----
>   plat/xen/Makefile.uk             |  5 ++++-
>   plat/xen/include/common/gnttab.h |  6 ++++++
>   plat/xen/memory.c                |  2 ++
>   4 files changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/plat/xen/Config.uk b/plat/xen/Config.uk
> index d0143e95..9cfe90a2 100644
> --- a/plat/xen/Config.uk
> +++ b/plat/xen/Config.uk
> @@ -3,8 +3,6 @@ menuconfig PLAT_XEN
>          default n
>          depends on (ARCH_X86_32 || ARCH_X86_64 || ARCH_ARM_32)
>          select LIBUKDEBUG
> -       select LIBUKLOCK
> -       select LIBUKLOCK_SEMAPHORE
>          select LIBNOLIBC if !HAVE_LIBC
>          select LIBFDT if ARCH_ARM_32
>          select XEN_DBGEMERGENCY if ARCH_ARM_32
> @@ -21,11 +19,23 @@ if (PLAT_XEN)
>                 option is enabled the hypervisor console is used
>                 for kernel messages only.
>   
> +config XEN_GNTTAB
> +       bool "Grant table support"
> +       default y
> +       depends on (ARCH_X86_64)
> +       select LIBUKALLOC
> +       select LIBUKLOCK
> +       select LIBUKLOCK_SEMAPHORE
> +       select LIBUKSCHED
> +       help
> +               Support grant table operations
> +
>   menuconfig XEN_XENBUS
> -       bool "Xenbus Driver"
> +       bool "Xenbus/XenStore driver"
>         default n
> -       depends on (ARCH_X86_64)
> +       depends on XEN_GNTTAB
Why do we introduce the dependency on the Grant table. The xenbus seems 
to have a dependency on the LIBUKALLOC and LIBUKSCHED.
>         select LIBUKBUS
>         help
> -               Register a Xenbus driver as uk_bus
> +               Registers Xenbus as bus driver to libukbus and provides a
> +               XenStore communication API for Xen drivers
>   endif
> diff --git a/plat/xen/Makefile.uk b/plat/xen/Makefile.uk
> index 38b510ad..3439a0d9 100644
> --- a/plat/xen/Makefile.uk
> +++ b/plat/xen/Makefile.uk
> @@ -45,7 +45,6 @@ LIBXENPLAT_SRCS-$(CONFIG_ARCH_X86_64) += 
> $(LIBXENPLAT_BASE)/x86/entry64.S
>   LIBXENPLAT_SRCS-$(CONFIG_ARCH_X86_64) += $(LIBXENPLAT_BASE)/x86/mm.c
>   LIBXENPLAT_SRCS-$(CONFIG_ARCH_X86_64) += $(LIBXENPLAT_BASE)/x86/arch_events.c
>   LIBXENPLAT_SRCS-$(CONFIG_ARCH_X86_64) += $(LIBXENPLAT_BASE)/x86/arch_time.c
> -LIBXENPLAT_SRCS-$(CONFIG_ARCH_X86_64) += $(LIBXENPLAT_BASE)/x86/gnttab.c|x86
>   
>   ifneq ($(XEN_HVMLITE),y)
>   LIBXENPLAT_ASFLAGS-y           += -DCONFIG_PARAVIRT
> @@ -73,7 +72,11 @@ LIBXENPLAT_SRCS-y              += $(LIBXENPLAT_BASE)/lcpu.c
>   LIBXENPLAT_SRCS-y              += $(LIBXENPLAT_BASE)/console.c
>   LIBXENPLAT_SRCS-y              += $(LIBXENPLAT_BASE)/shutdown.c
>   LIBXENPLAT_SRCS-y              += $(LIBXENPLAT_BASE)/events.c
> +
> +ifeq ($(CONFIG_XEN_GNTTAB),y)
>   LIBXENPLAT_SRCS-y              += $(LIBXENPLAT_BASE)/gnttab.c
> +LIBXENPLAT_SRCS-$(CONFIG_ARCH_X86_64) += $(LIBXENPLAT_BASE)/x86/gnttab.c|x86
> +endif
>   
>   ifeq ($(CONFIG_XEN_XENBUS),y)
>   LIBXENBUS_EXPORTS               = $(LIBXENPLAT_BASE)/xenbus/exportsyms.uk
> diff --git a/plat/xen/include/common/gnttab.h b/plat/xen/include/common/gnttab.h
> index d4bb4172..f5cde5c5 100644
> --- a/plat/xen/include/common/gnttab.h
> +++ b/plat/xen/include/common/gnttab.h
> @@ -26,6 +26,10 @@
>   #ifndef __GNTTAB_H__
>   #define __GNTTAB_H__
>   
> +#include <uk/config.h>
> +
> +#ifdef CONFIG_XEN_GNTTAB
> +
>   #include <uk/alloc.h>
>   #include <xen/grant_table.h>
>   
> @@ -45,4 +49,6 @@ const char *gnttabop_error(__s16 status);
>   
>   grant_entry_v1_t *gnttab_arch_init(int nr_grant_frames);
>   
> +#endif /* CONFIG_XEN_GNTTAB */
> +
>   #endif /* !__GNTTAB_H__ */
> diff --git a/plat/xen/memory.c b/plat/xen/memory.c
> index f84dca71..1f55887e 100644
> --- a/plat/xen/memory.c
> +++ b/plat/xen/memory.c
> @@ -123,6 +123,8 @@ int ukplat_memregion_get(int i, struct 
> ukplat_memregion_desc *m)
>   
>   int _ukplat_mem_mappings_init(void)
>   {
> +#ifdef CONFIG_XEN_GNTTAB
>         gnttab_init();
> +#endif
>         return 0;
>   }
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel