[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
Hello Justin,
Seems good
Thanks & Regards
Sharan
On 7/3/19 6:14 PM, Justin He (Arm Technology China) wrote:
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
|
