[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Minios-devel] [UNIKRAFT PATCHv7 2/8] drivers/ofw: Enable build support for internal fdt interfaces
Hi Sharan > -----Original Message----- > From: Sharan Santhanam <sharan.santhanam@xxxxxxxxx> > Sent: 2019年7月3日 23:42 > To: Justin He (Arm Technology China) <Justin.He@xxxxxxx>; minios- > devel@xxxxxxxxxxxxxxxxxxxx; Simon Kuenzer <simon.kuenzer@xxxxxxxxx> > Cc: Florian Schmidt <florian.schmidt@xxxxxxxxx>; Felipe Huici > <felipe.huici@xxxxxxxxx>; Julien Grall <Julien.Grall@xxxxxxx>; > yuri.volchkov@xxxxxxxxx; Kaly Xin (Arm Technology China) > <Kaly.Xin@xxxxxxx> > Subject: Re: [UNIKRAFT PATCHv7 2/8] drivers/ofw: Enable build support for > internal fdt interfaces > > Hello Jia He, > > On 7/3/19 5:09 PM, Justin He (Arm Technology China) wrote: > > Hi Sharan > > > >> -----Original Message----- > >> From: Sharan Santhanam <sharan.santhanam@xxxxxxxxx> > >> Sent: 2019年7月3日 17:25 > >> To: Justin He (Arm Technology China) <Justin.He@xxxxxxx>; minios- > >> devel@xxxxxxxxxxxxxxxxxxxx; Simon Kuenzer <simon.kuenzer@xxxxxxxxx> > >> Cc: Florian Schmidt <florian.schmidt@xxxxxxxxx>; Felipe Huici > >> <felipe.huici@xxxxxxxxx>; Julien Grall <Julien.Grall@xxxxxxx>; > >> yuri.volchkov@xxxxxxxxx; Kaly Xin (Arm Technology China) > >> <Kaly.Xin@xxxxxxx> > >> Subject: Re: [UNIKRAFT PATCHv7 2/8] drivers/ofw: Enable build support > for > >> internal fdt interfaces > >> > >> Hello Justin, > >> > >> One more suggestion, I would prefer if we split the device tree driver > >> as a separate library from the kvm platform library. This is similar to > >> what we do virtio or pci drivers in > >> 'plat/kvm/Makefile.uk'. > > Ok with me to split the libofw (is this lib name ok with you?) > > Since the libofw is not dedicated to kvm plat, do you think it would be > better move > > below directories to ./lib/ofw and ./lib/ofw/include ? > > ./plat/drivers/include/ofw > > ./plat/drivers/ofw > > > I would still keep it within the plat/drivers and follow the naming > convention we have adopted so far which in this case is "libkvmofw". The > prefixing the "kvm" is to make sure these libraries platform specific > and each platform enable the driver it needs. Ok. > > Of course as a next step we can make it independent, but then we might > have to extend the build system further to process the driver Makefiles > separately. So I avoid it for this patch set. Ok, I tried it just now, not so much changes as follows, I can make it in the future (not his patch series if it is ok with you): diff --git a/plat/kvm/Config.uk b/plat/kvm/Config.uk index 5a6dfc1..096beb1 100644 --- a/plat/kvm/Config.uk +++ b/plat/kvm/Config.uk @@ -7,6 +7,7 @@ menuconfig PLAT_KVM select LIBUKTIMECONV select LIBNOLIBC if !HAVE_LIBC select LIBFDT if ARCH_ARM_64 + select LIBOFW if ARCH_ARM_64 help Create a Unikraft image that runs as a KVM guest @@ -80,4 +81,9 @@ config VIRTIO_NET help Virtual network driver. endmenu + +config LIBOFW + bool "Open Firmware library support" + default n + select LIBFDT endif diff --git a/plat/kvm/Makefile.uk b/plat/kvm/Makefile.uk index 3c3c006..3aeab2c 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,libkvmofw,$(CONFIG_LIBOFW))) ## ## Platform library definitions @@ -66,7 +67,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)/ofw/fdt.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 @@ -82,6 +82,18 @@ LIBKVMPLAT_SRCS-y += $(UK_PLAT_COMMON_BASE)/lcpu.c|common LIBKVMPLAT_SRCS-y += $(UK_PLAT_COMMON_BASE)/memory.c|common LIBKVMPLAT_SRCS-y += $(KVM_LDSCRIPT_SRC-y) +## +## OFW library definitions +## +LIBKVMOFW_ASINCLUDES-y += -I$(LIBKVMPLAT_BASE)/include +LIBKVMOFW_ASINCLUDES-y += -I$(UK_PLAT_COMMON_BASE)/include +LIBKVMOFW_CINCLUDES-y += -I$(LIBKVMPLAT_BASE)/include +LIBKVMOFW_CINCLUDES-y += -I$(UK_PLAT_COMMON_BASE)/include +LIBKVMOFW_CINCLUDES-y += -I$(UK_PLAT_DRIVERS_BASE)/include + +LIBKVMOFW_SRCS-$(CONFIG_ARCH_ARM_64) += \ + $(UK_PLAT_DRIVERS_BASE)/ofw/fdt.c + ## ## PCI library definitions ## > > Thanks & Regards > Sharan > > > Thanks for the clarification > > > > -- > > Cheers, > > Justin (Jia He) > > > > > >> > >> We can do it as a part of this series or take it up as a subsequent > >> patch. This would help us with maintaining the modularity of the library. > >> > >> Please find the other comment inline. > >> > >> Thanks & Regards > >> Sharan Santhanam > >> > >> On 7/2/19 3:35 AM, Justin He (Arm Technology China) wrote: > >>> Hi Sharan > >>> > >>>> -----Original Message----- > >>>> From: Sharan Santhanam <sharan.santhanam@xxxxxxxxx> > >>>> Sent: 2019年7月1日 22:54 > >>>> To: Justin He (Arm Technology China) <Justin.He@xxxxxxx>; minios- > >>>> devel@xxxxxxxxxxxxxxxxxxxx; Simon Kuenzer <simon.kuenzer@xxxxxxxxx> > >>>> Cc: Florian Schmidt <florian.schmidt@xxxxxxxxx>; Felipe Huici > >>>> <felipe.huici@xxxxxxxxx>; Julien Grall <Julien.Grall@xxxxxxx>; > >>>> yuri.volchkov@xxxxxxxxx; Kaly Xin (Arm Technology China) > >> <Kaly.Xin@xxxxxxx> > >>>> Subject: Re: [UNIKRAFT PATCHv7 2/8] drivers/ofw: Enable build support > >> for > >>>> internal fdt interfaces > >>>> > >>>> Hello Jia He, > >>>> > >>>> This patch seems fine. > >>>> > >>>> My suggestion would be in the next version of the patch series we can > >>>> combine this patch with the previous one where we include the > function. > >>>> > >>> > >>> Thanks, ok I will merge it into previous one > >>> > >>>> Thanks & Regards > >>>> Sharan Santhanam > >>>> > >>>> On 6/27/19 10:55 AM, Jia He wrote: > >>>>> This enable the build support for unikraft internal fdt interfaces > >>>>> > >>>>> Signed-off-by: Jia He <justin.he@xxxxxxx> > >>>>> --- > >>>>> plat/kvm/Makefile.uk | 1 + > >>>>> 1 file changed, 1 insertion(+) > >>>>> > >>>>> diff --git a/plat/kvm/Makefile.uk b/plat/kvm/Makefile.uk > >>>>> index 71c4c41..18eaca4 100644 > >>>>> --- a/plat/kvm/Makefile.uk > >>>>> +++ b/plat/kvm/Makefile.uk > >>>>> @@ -65,6 +65,7 @@ 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)/ofw/fdt.c|common > >>>> Unfortunately, adding this to files causes compilation error. This was > >>>> related to the previous patch where we include print.h instead of > >>>> assert.h . > >>>> > >>> I will check it > >> > >> We are also missing the include path for the ofw/fdt.h header/ I have fixed it in patch v8 [1] [1] https://lists.xenproject.org/archives/html/minios-devel/2019-07/msg00035.html -- Cheers, Justin (Jia He) 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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |