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

[Sharan Santhanam <sharan.santhanam@xxxxxxxxx>]

On 8/28/19 8:03 AM, Justin He (Arm Technology China) wrote:
> > -----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 😉

I agree. We should do it in another patch series.

> > > +       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