Re: [Minios-devel] [UNIKRAFT PATCHv7 2/8] drivers/ofw: Enable build support for internal fdt interfaces

[Sharan Santhanam <sharan.santhanam@xxxxxxxxx>]

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.
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.
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/
> > > --
> > > Cheers,
> > > Justin (Jia He)
> > >
> > >
> > > > >     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
> > > > >
> > > 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.
> > >
> 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