[Top] [All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT PATCH v2 3/9] plat/{kvm, xen}: Clean up Makefile.uk conditional build rules

Florian Schmidt <Florian.Schmidt@xxxxxxxxx> writes:

> Hi Yuri,
>
> On 12/17/18 2:51 PM, Yuri Volchkov wrote:
>> Hi,
>> 
>> please see my notes inline
>> 
>> - Yuri.
>> 
>> Florian Schmidt <florian.schmidt@xxxxxxxxx> writes:
>> 
>>> 1) Those ifeqs aren't necessary because the $(CONFIG_ARCH...) part
>>>     already deals with the conditions under which to build those files.
>> If you replace "necessary" with "needed" the understandably will
>> increase significantly
>
> OK, if that helps, I'm happy to change that.
>> 
>>> 2) Add $(LIBKVMPLAT_BASE)/include as include directory for libkvmpci and
>>>     libkvmpcivirtio.
>
> I just noticed myself that "and libkvmpcivirtio" isn't true any more. It 
> was true back when I did the v1, but the virtio patches in between 
> restructured this. I properly removed those parts during rebasing, but 
> forgot to change the commit message accordingly. I'll do that for the v3.
>
>>>
>>> Signed-off-by: Florian Schmidt <florian.schmidt@xxxxxxxxx>
>>> ---
>>>   plat/kvm/Makefile.uk |  6 ++----
>>>   plat/xen/Makefile.uk | 21 +++++++++++----------
>>>   2 files changed, 13 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/plat/kvm/Makefile.uk b/plat/kvm/Makefile.uk
>>> index 1f9c5dc0..b04a9868 100644
>>> --- a/plat/kvm/Makefile.uk
>>> +++ b/plat/kvm/Makefile.uk
>>> @@ -21,7 +21,6 @@ LIBKVMPLAT_CINCLUDES-y         += 
>>> -I$(UK_PLAT_COMMON_BASE)/include
>>>   ##
>>>   ## Architecture library definitions for x86_64
>>>   ##
>>> -ifeq ($(CONFIG_ARCH_X86_64),y)
>>>   LIBKVMPLAT_SRCS-$(CONFIG_ARCH_X86_64) += 
>>> $(UK_PLAT_COMMON_BASE)/x86/trace.c|common
>>>   LIBKVMPLAT_SRCS-$(CONFIG_ARCH_X86_64) += 
>>> $(UK_PLAT_COMMON_BASE)/x86/traps.c|common
>>>   LIBKVMPLAT_SRCS-$(CONFIG_ARCH_X86_64) += 
>>> $(UK_PLAT_COMMON_BASE)/x86/cpu_native.c|common
>>> @@ -45,12 +44,10 @@ endif
>>>   ifeq ($(findstring y,$(CONFIG_KVM_KERNEL_SERIAL_CONSOLE) 
>>> $(CONFIG_KVM_DEBUG_SERIAL_CONSOLE)),y)
>>>   LIBKVMPLAT_SRCS-$(CONFIG_ARCH_X86_64) += 
>>> $(LIBKVMPLAT_BASE)/x86/serial_console.c
>>>   endif
>>> -endif
>>>   
>>>   ##
>>>   ## Architecture library definitions for arm64
>>>   ##
>>> -ifeq ($(CONFIG_ARCH_ARM_64),y)
>>>   ifeq ($(findstring y,$(CONFIG_KVM_KERNEL_SERIAL_CONSOLE) 
>>> $(CONFIG_KVM_DEBUG_SERIAL_CONSOLE)),y)
>>>   LIBKVMPLAT_SRCS-$(CONFIG_ARCH_ARM_64) += 
>>> $(UK_PLAT_COMMON_BASE)/arm/pl011.c|common
>>>   endif
>>> @@ -65,7 +62,6 @@ LIBKVMPLAT_SRCS-$(CONFIG_ARCH_ARM_64) += 
>>> $(LIBKVMPLAT_BASE)/arm/pagetable.S
>>>   LIBKVMPLAT_SRCS-$(CONFIG_ARCH_ARM_64) += $(LIBKVMPLAT_BASE)/arm/setup.c
>>>   LIBKVMPLAT_SRCS-$(CONFIG_ARCH_ARM_64) += $(LIBKVMPLAT_BASE)/arm/lcpu.c
>>>   LIBKVMPLAT_SRCS-$(CONFIG_ARCH_ARM_64) += $(LIBKVMPLAT_BASE)/arm/intctrl.c
>>> -endif
>>>   
>>>   LIBKVMPLAT_SRCS-y              += $(LIBKVMPLAT_BASE)/shutdown.c
>>>   LIBKVMPLAT_SRCS-y              += $(LIBKVMPLAT_BASE)/memory.c
>>> @@ -77,7 +73,9 @@ LIBKVMPLAT_SRCS-y              += 
>>> $(UK_PLAT_COMMON_BASE)/memory.c|common
>>>   ##
>>>   ## PCI library definitions
>>>   ##
>>> +LIBKVMPCI_ASINCLUDES-$(CONFIG_ARCH_X86_64)  += -I$(LIBKVMPLAT_BASE)/include
>>>   LIBKVMPCI_ASINCLUDES-$(CONFIG_ARCH_X86_64)  += 
>>> -I$(UK_PLAT_COMMON_BASE)/include
>>> +LIBKVMPCI_CINCLUDES-$(CONFIG_ARCH_X86_64)   += -I$(LIBKVMPLAT_BASE)/include
>>>   LIBKVMPCI_CINCLUDES-$(CONFIG_ARCH_X86_64)   += 
>>> -I$(UK_PLAT_COMMON_BASE)/include
>>>   LIBKVMPCI_SRCS-$(CONFIG_ARCH_X86_64)        += 
>>> $(UK_PLAT_COMMON_BASE)/pci_bus.c|common
>>>   
>>> diff --git a/plat/xen/Makefile.uk b/plat/xen/Makefile.uk
>>> index 5d777b23..7e8f114c 100644
>>> --- a/plat/xen/Makefile.uk
>>> +++ b/plat/xen/Makefile.uk
>>> @@ -31,7 +31,6 @@ LIBXENPLAT_SRCS-y              += $(LIBXENPLAT_BASE)/io.c
>>>   LIBXENPLAT_SRCS-y              += $(UK_PLAT_COMMON_BASE)/lcpu.c|common
>>>   LIBXENPLAT_SRCS-y              += $(UK_PLAT_COMMON_BASE)/memory.c|common
>>>   
>>> -ifneq (,$(filter x86_32 x86_64,$(CONFIG_UK_ARCH)))
>>>   LIBXENPLAT_SRCS-$(CONFIG_ARCH_X86_64) += 
>>> $(UK_PLAT_COMMON_BASE)/x86/trace.c|common
>>>   LIBXENPLAT_SRCS-$(CONFIG_ARCH_X86_64) += 
>>> $(UK_PLAT_COMMON_BASE)/x86/traps.c|common
>>>   ifeq ($(CONFIG_HAVE_SCHED),y)
>> Here follows a portion of sources included unconditionally
>> LIBXENPLAT_SRCS-y              += $(LIBXENPLAT_BASE)/x86/setup.c
>> LIBXENPLAT_SRCS-y              += $(LIBXENPLAT_BASE)/x86/traps.c
>> ...
>> 
>> Did you forget to add $(CONFIG_ARCH_X86_64)?
>
> Hmmm... That's interesting. That code was always set up to 
> unconditionally include those files. I guess that's back from when we 
> had x86_32 and x86_64, but no Arm? In any case, you are right, these 
> should all have the $(CONFIG_ARCH_X86_64) conditional. Should I roll 
> that into this patch for v3, or should we make a separate patch out of this?
I think including this into current patch is fine.

>
>> 
>>> @@ -55,18 +54,20 @@ LIBXENPLAT_SRCS-y              += 
>>> $(LIBXENPLAT_BASE)/x86/cpu_pv.c
>>>   else
>>>   LIBXENPLAT_SRCS-y              += $(UK_PLAT_COMMON_BASE)/x86/cpu_native.c
>>>   endif
>>> -endif
>>>   
>>> -ifneq (,$(filter arm arm_64,$(CONFIG_UK_ARCH)))
>>> -LIBXENPLAT_SRCS-y              += $(LIBXENPLAT_BASE)/arm/setup.c
>>> -LIBXENPLAT_SRCS-y              += $(LIBXENPLAT_BASE)/arm/traps.c
>>> +LIBXENPLAT_SRCS-$(CONFIG_ARCH_ARM_32) += $(LIBXENPLAT_BASE)/arm/setup.c
>>> +LIBXENPLAT_SRCS-$(CONFIG_ARCH_ARM_32) += $(LIBXENPLAT_BASE)/arm/traps.c
>>>   LIBXENPLAT_SRCS-$(CONFIG_ARCH_ARM_32) += $(LIBXENPLAT_BASE)/arm/entry32.S
>>> -LIBXENPLAT_SRCS-$(ARCH_ARM_64) += $(LIBXENPLAT_BASE)/arm/entry64.S
>>> -LIBXENPLAT_SRCS-y              += $(LIBXENPLAT_BASE)/arm/arch_events.c
>>> -LIBXENPLAT_SRCS-y              += $(LIBXENPLAT_BASE)/arm/arch_time.c
>>> +LIBXENPLAT_SRCS-$(CONFIG_ARCH_ARM_32) += 
>>> $(LIBXENPLAT_BASE)/arm/arch_events.c
>>> +LIBXENPLAT_SRCS-$(CONFIG_ARCH_ARM_32) += $(LIBXENPLAT_BASE)/arm/arch_time.c
>>>   LIBXENPLAT_SRCS-$(CONFIG_ARCH_ARM_32) += 
>>> $(LIBXENPLAT_BASE)/arm/hypercalls32.S
>>> -LIBXENPLAT_SRCS-$(ARCH_ARM_64) += $(LIBXENPLAT_BASE)/arm/hypercalls64.S
>>> -endif
>>> +
>>> +LIBXENPLAT_SRCS-$(CONFIG_ARCH_ARM_64) += $(LIBXENPLAT_BASE)/arm/setup.c
>>> +LIBXENPLAT_SRCS-$(CONFIG_ARCH_ARM_64) += $(LIBXENPLAT_BASE)/arm/traps.c
>>> +LIBXENPLAT_SRCS-$(CONFIG_ARCH_ARM_64) += $(LIBXENPLAT_BASE)/arm/entry64.S
>>> +LIBXENPLAT_SRCS-$(CONFIG_ARCH_ARM)64) += 
>>> $(LIBXENPLAT_BASE)/arm/arch_events.c
>> A typo with a parenthesis
>
> Good catch!
>
>> 
>>> +LIBXENPLAT_SRCS-$(CONFIG_ARCH_ARM_64) += $(LIBXENPLAT_BASE)/arm/arch_time.c
>>> +LIBXENPLAT_SRCS-$(CONFIG_ARCH_ARM_64) += 
>>> $(LIBXENPLAT_BASE)/arm/hypercalls64.S
>>>   
>>>   LIBXENPLAT_SRCS-y              += $(LIBXENPLAT_BASE)/lcpu.c
>>>   LIBXENPLAT_SRCS-y              += $(LIBXENPLAT_BASE)/console.c
>>> -- 
>>> 2.19.2
>>>
>> 
>
> -- 
> Dr. Florian Schmidt
> フローリアン・シュミット
> Research Scientist,
> Systems and Machine Learning Group
> NEC Laboratories Europe
> Kurfürsten-Anlage 36, D-69115 Heidelberg
> Tel.     +49 (0)6221 4342-265
> Fax:     +49 (0)6221 4342-155
> e-mail:  florian.schmidt@xxxxxxxxx
> ============================================================
> Registered at Amtsgericht Mannheim, Germany, HRB728558

-- 
Yuri Volchkov
Software Specialist

NEC Europe Ltd
Kurfürsten-Anlage 36
D-69115 Heidelberg

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel

 

©2013 Xen Project, A Linux Foundation Collaborative Project. All Rights Reserved.
Linux Foundation is a registered trademark of The Linux Foundation.
Xen Project is a trademark of The Linux Foundation.

Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.