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

[Yuri Volchkov <yuri.volchkov@xxxxxxxxx>]

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

> 2) Add $(LIBKVMPLAT_BASE)/include as include directory for libkvmpci and
>    libkvmpcivirtio.
>
> 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)?

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

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

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