Re: [Minios-devel] [UNIKRAFT PATCH 2/3] build: Split gic-v2 from kvm plat to independent library

[Jia He <jiakernel2@xxxxxxxxx>]

Hi Sharan

On 2019/8/13 22:02, Sharan Santhanam wrote:
> Hello Justin,
>
> Please find the comments inline
>
> Thanks & Regards
>
> Sharan
>
> On 7/29/19 1:26 PM, Jia He wrote:
> > This patch splits the arm gic-v2 interrupt controller as a separate library
> > from the kvm platform library. This is similar to what we do for virtio or
> > pci drivers.
> >
> > Signed-off-by: Jia He <justin.he@xxxxxxx>
> > Suggested-by: Sharan Santhanam <sharan.santhanam@xxxxxxxxx>
> > ---
> >   plat/kvm/Config.uk   |  7 +++++++
> >   plat/kvm/Makefile.uk | 14 +++++++++++++-
> >   2 files changed, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/plat/kvm/Config.uk b/plat/kvm/Config.uk
> > index 096beb1..145887b 100644
> > --- a/plat/kvm/Config.uk
> > +++ b/plat/kvm/Config.uk
> > @@ -8,6 +8,7 @@ menuconfig PLAT_KVM
> >          select LIBNOLIBC if !HAVE_LIBC
> >          select LIBFDT if ARCH_ARM_64
> >          select LIBOFW if ARCH_ARM_64
> > +       select LIBGICV2 if ARCH_ARM_64
> Do we need this select? Can't we select gic when the library/application 
> needs it.
Yes, we need it

Because intctrl.c is included in libkvmplat unconditionally, and intctrl.c 
depends on
some interface which are defined in gic v2 lib.

So maybe I still need the "select" here.


> >          help
> >                   Create a Unikraft image that runs as a KVM guest
> >   @@ -86,4 +87,10 @@ config LIBOFW
> >       bool "Open Firmware library support"
> >       default n
> >       select LIBFDT
> > +
> > +config LIBGICV2
> > +    bool "Arm GIC (generic interrupt controller) v2 library support"
> > +    default n
> > +    select LIBFDT
>  Do we need to select LIBFDT here?
No need, since LIBOFW will select LIBFDT

Thanks for the pointing

> > +    select LIBOFW
> >   endif
> > diff --git a/plat/kvm/Makefile.uk b/plat/kvm/Makefile.uk
> > index c2ec45b..8b9bcd7 100644
> > --- a/plat/kvm/Makefile.uk
> > +++ b/plat/kvm/Makefile.uk
> > @@ -11,6 +11,7 @@ $(eval $(call addplatlib_s,kvm,libkvmpci,$(CONFIG_KVM_PCI)))
> >   $(eval $(call addplatlib_s,kvm,libkvmvirtio,$(CONFIG_VIRTIO_BUS)))
> >   $(eval $(call addplatlib_s,kvm,libkvmvirtionet,$(CONFIG_VIRTIO_NET)))
> >   $(eval $(call addplatlib_s,kvm,libkvmofw,$(CONFIG_LIBOFW)))
> > +$(eval $(call addplatlib_s,kvm,libkvmgicv2,$(CONFIG_LIBGICV2)))
> >     ##
> >   ## Platform library definitions
> > @@ -67,7 +68,6 @@ LIBKVMPLAT_SRCS-$(CONFIG_ARCH_ARM_64) += 
> > $(UK_PLAT_COMMON_BASE)/arm/cache64.S|co
> >   LIBKVMPLAT_SRCS-$(CONFIG_ARCH_ARM_64) += 
> > $(UK_PLAT_COMMON_BASE)/arm/psci_arm64.S|common
> >   LIBKVMPLAT_SRCS-$(CONFIG_ARCH_ARM_64) += 
> > $(UK_PLAT_COMMON_BASE)/arm/time.c|common
> >   LIBKVMPLAT_SRCS-$(CONFIG_ARCH_ARM_64) += 
> > $(UK_PLAT_COMMON_BASE)/arm/traps.c|common
> > -LIBKVMPLAT_SRCS-$(CONFIG_ARCH_ARM_64) += 
> > $(UK_PLAT_DRIVERS_BASE)/gic/gic-v2.c|common
> >   LIBKVMPLAT_SRCS-$(CONFIG_ARCH_ARM_64) += $(LIBKVMPLAT_BASE)/arm/entry64.S
> >   LIBKVMPLAT_SRCS-$(CONFIG_ARCH_ARM_64) += $(LIBKVMPLAT_BASE)/arm/exceptions.S
> >   LIBKVMPLAT_SRCS-$(CONFIG_ARCH_ARM_64) += $(LIBKVMPLAT_BASE)/arm/pagetable64.S
> > @@ -95,6 +95,18 @@ LIBKVMOFW_CINCLUDES-y         += 
> > -I$(UK_PLAT_DRIVERS_BASE)/include
> >   LIBKVMOFW_SRCS-$(CONFIG_ARCH_ARM_64) += \
> > $(UK_PLAT_DRIVERS_BASE)/ofw/fdt.c
> >   +##
> > +## GICv2 library definitions
> > +##
> > +LIBKVMGICV2_ASINCLUDES-y        += -I$(LIBKVMPLAT_BASE)/include
> > +LIBKVMGICV2_ASINCLUDES-y        += -I$(UK_PLAT_COMMON_BASE)/include
> Do we need common as include path?
No need, thanks

---
Cheers,
Justin (Jia He)


_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel