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

["Justin He (Arm Technology China)" <Justin.He@xxxxxxx>]

> -----Original Message-----
> From: Sharan Santhanam <sharan.santhanam@xxxxxxxxx>
> Sent: 2019年8月20日 23:29
> To: Justin He (Arm Technology China) <Justin.He@xxxxxxx>; minios-
> devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>; Felipe Huici
> <felipe.huici@xxxxxxxxx>; Kaly Xin (Arm Technology China)
> <Kaly.Xin@xxxxxxx>
> Subject: Re: [UNIKRAFT PATCHv2 1/3] build: Split gic-v2 from kvm plat to
> independent library
>
> Hello Justin,
>
> please find the review comment inline.
>
> Thanks & Regards
>
> Sharan
>
> On 8/15/19 9:16 AM, 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   |  9 ++++++++-
> >   plat/kvm/Makefile.uk | 12 +++++++++++-
> >   2 files changed, 19 insertions(+), 2 deletions(-)
> >
> > diff --git a/plat/kvm/Config.uk b/plat/kvm/Config.uk
> > index 5a6dfc1..39da143 100644
> > --- a/plat/kvm/Config.uk
> > +++ b/plat/kvm/Config.uk
> > @@ -6,7 +6,7 @@ menuconfig PLAT_KVM
> >          select LIBUKALLOC
> >          select LIBUKTIMECONV
> >          select LIBNOLIBC if !HAVE_LIBC
> > -       select LIBFDT if ARCH_ARM_64
>
> Unfortunately this is still necessary as pl011 needs it.
>
> /plat/common/arm/pl011.c:21:20: fatal error: libfdt.h: No such file or
> directory
>
> We can split the pl011 similar to gic as another patch series and remove
> it from the platform. What do you think?
Ok, let's do it in the future 😉

>
> > +       select LIBGICV2 if ARCH_ARM_64
> >          help
> >                   Create a Unikraft image that runs as a KVM guest
> >
> > @@ -80,4 +80,11 @@ config VIRTIO_NET
> >          help
> >                 Virtual network driver.
> >   endmenu
> > +
> > +config LIBGICV2
> > +       bool "Arm GIC (generic interrupt controller) v2 library support"
> > +       default n
> > +       select LIBOFW
> LIBOFW is still not available. We should add it once we introduce the
> library.

Yes, indeed.

--
Cheers,
Justin (Jia He)


> > +       depends on ARCH_ARM_64
> > +
> >   endif
> > diff --git a/plat/kvm/Makefile.uk b/plat/kvm/Makefile.uk
> > index 7d0c696..b1fd50b 100644
> > --- a/plat/kvm/Makefile.uk
> > +++ b/plat/kvm/Makefile.uk
> > @@ -10,6 +10,7 @@ $(eval $(call addplatlib,kvm,libkvmplat))
> >   $(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,libkvmgicv2,$(CONFIG_LIBGICV2)))
> >
> >   ##
> >   ## Platform library definitions
> > @@ -67,7 +68,6 @@ LIBKVMPLAT_SRCS-$(CONFIG_ARCH_ARM_64) +=
> $(UK_PLAT_COMMON_BASE)/arm/psci_arm64.S
> >   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)/ofw/fdt.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
> > @@ -118,3 +118,13 @@ LIBKVMVIRTIONET_ASINCLUDES-y   += -
> I$(UK_PLAT_DRIVERS_BASE)/include
> >   LIBKVMVIRTIONET_CINCLUDES-y    += -
> I$(UK_PLAT_DRIVERS_BASE)/include
> >   LIBKVMVIRTIONET_SRCS-y +=\
> >                     $(UK_PLAT_DRIVERS_BASE)/virtio/virtio_net.c
> > +
> > +##
> > +## GICv2 library definitions
> > +##
> > +LIBKVMGICV2_CINCLUDES-y         += -I$(LIBKVMPLAT_BASE)/include
> > +LIBKVMGICV2_CINCLUDES-y         += -
> I$(UK_PLAT_COMMON_BASE)/include
> > +LIBKVMGICV2_CINCLUDES-y         += -
> I$(UK_PLAT_DRIVERS_BASE)/include
> > +
> > +LIBKVMGICV2_SRCS-$(CONFIG_LIBGICV2) += \
> Is having CONFIG_LIBGICV2 necessary as we are adding for this library?
> > +                                   $(UK_PLAT_DRIVERS_BASE)/gic/gic-
> v2.c
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel