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

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

On 12.02.19, 15:23, "Sharan Santhanam" <Sharan.Santhanam@xxxxxxxxx> wrote:

    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.

Yes you are right. I assumed such a dependency but in fact there isn't one. I 
will fix this with a v2.

    >           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