[PATCH 0/6] arm: more kconfig configurability and small default configs

[PATCH 0/6] arm: more kconfig configurability and small default configs

[Stefano Stabellini]

Hi all,

This patch series is the first step toward building a small certifiable
Xen hypervisor for ARM boards.

First, the series makes a few changes to allow disabling more kconfig
options: most of them already exist but cannot be disabled.

Then, it introduces a reference kconfig for Renesas RCar (due to popular
demand, candidate for certifications) and for QEMU aarch64 (not for
certifications, but useful for debugging).

The last patch in the series adds a convenient cloc target to count the
total lines of code of the source files built.


There a couple of open questions which need to be addressed in regards
to which options to enable/disable in the reference kconfig. The primary
one is which schedulers to enable.

In this series, I enabled NULL and credit, but none of the others. The
choice is somewhat arbitrary but driven by the idea that NULL is
required for best interrupt latency results, and credit is the default.
They can be paired using multiple cpupools.

Aside from the schedulers, most other kconfig options seem pretty
obvious.

I am interested in hearing other opinions about this.

Cheers,

Stefano



Stefano Stabellini (6):
      arm: make it possible to disable more kconfig options
      arm: make it possible to enable/disable UART drivers
      arm: make it possible to disable the SMMU driver
      arm: add a small kconfig for Renesas RCar H3
      arm: add a small kconfig for qemu-system-aarch64
      xen: add cloc target

 xen/Makefile                         | 14 ++++++-
 xen/Rules.mk                         |  2 +
 xen/arch/arm/Kconfig                 | 15 +++++--
 xen/arch/arm/configs/qemu.config     | 81 ++++++++++++++++++++++++++++++++++++
 xen/arch/arm/configs/renesas.config  | 80 +++++++++++++++++++++++++++++++++++
 xen/drivers/char/Kconfig             | 16 +++----
 xen/drivers/passthrough/Kconfig      |  2 +
 xen/drivers/passthrough/arm/Kconfig  |  7 ++++
 xen/drivers/passthrough/arm/Makefile |  2 +-
 xen/drivers/video/Kconfig            |  8 +++-
 10 files changed, 211 insertions(+), 16 deletions(-)
 create mode 100644 xen/arch/arm/configs/qemu.config
 create mode 100644 xen/arch/arm/configs/renesas.config
 create mode 100644 xen/drivers/passthrough/arm/Kconfig

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 1/6] arm: make it possible to disable more kconfig options

[Stefano Stabellini]

Make it possible to disable the following existing kconfig options:
  HAS_GICV3
  HAS_ARM_HDLCD
  HAS_MEM_ACCESS

Today they are silent option. This patch adds one line descriptions and
make them de/selectable.

Also, do not select VIDEO: make HAS_ARM_HDLCD select VIDEO instead. In
fact, VIDEO is only needed by HAS_ARM_HDLCD.

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
---
 xen/arch/arm/Kconfig      | 15 +++++++++++----
 xen/drivers/video/Kconfig |  8 +++++++-
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 8174c0c..83963c8 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -12,17 +12,13 @@ config ARM_32
 config ARM_64
 	def_bool y
 	depends on 64BIT
-	select HAS_GICV3
 
 config ARM
 	def_bool y
 	select HAS_ALTERNATIVE
-	select HAS_ARM_HDLCD
 	select HAS_DEVICE_TREE
-	select HAS_MEM_ACCESS
 	select HAS_PASSTHROUGH
 	select HAS_PDX
-	select VIDEO
 
 config ARCH_DEFCONFIG
 	string
@@ -44,6 +40,17 @@ config ACPI
 
 config HAS_GICV3
 	bool
+	prompt "GICv3 driver"
+	default y
+
+config HAS_MEM_ACCESS
+	bool
+	prompt "Memory Access and VM events"
+	default y
+	---help---
+
+	  Framework to configure memory access types for guests and receive
+	  related events in userspace.
 
 config HAS_ITS
         bool
diff --git a/xen/drivers/video/Kconfig b/xen/drivers/video/Kconfig
index 52e8ce6..26daf9a 100644
--- a/xen/drivers/video/Kconfig
+++ b/xen/drivers/video/Kconfig
@@ -13,4 +13,10 @@ config VGA
 	  If unsure, say Y.
 
 config HAS_ARM_HDLCD
-	bool
+	bool "ARM HDLCD driver"
+	default n
+	select VIDEO
+	---help---
+	  Enable Xen video output to ARM HDLCD graphic controllers.
+
+	  if unsure, say N.
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 2/6] arm: make it possible to enable/disable UART drivers

[Stefano Stabellini]

All the UART drivers are silent options. Add one line descriptions so
that can be de/selected via menuconfig.

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
---
 xen/drivers/char/Kconfig | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/xen/drivers/char/Kconfig b/xen/drivers/char/Kconfig
index cc78ec3..b6fcefd 100644
--- a/xen/drivers/char/Kconfig
+++ b/xen/drivers/char/Kconfig
@@ -1,11 +1,11 @@
 config HAS_NS16550
-	bool
+	bool "NS16550 UART driver"
 	default y
 	help
 	  This selects the 16550-series UART support. For most systems, say Y.
 
 config HAS_CADENCE_UART
-	bool
+	bool "Xilinx Cadence UART driver"
 	default y
 	depends on ARM_64
 	help
@@ -13,7 +13,7 @@ config HAS_CADENCE_UART
 	  based board, say Y.
 
 config HAS_MVEBU
-	bool
+	bool "Marvell MVEBU UART driver"
 	default y
 	depends on ARM_64
 	help
@@ -21,7 +21,7 @@ config HAS_MVEBU
 	  based board, say Y.
 
 config HAS_PL011
-	bool
+	bool "ARM PL011 UART driver"
 	default y
 	depends on ARM
 	help
@@ -29,7 +29,7 @@ config HAS_PL011
 	  an Integrator/PP2, Integrator/CP or Versatile platform, say Y.
 
 config HAS_EXYNOS4210
-	bool
+	bool "Samsung Exynos 4210 UART driver"
 	default y
 	depends on ARM_32
 	help
@@ -37,7 +37,7 @@ config HAS_EXYNOS4210
 	  Exynos based board, say Y.
 
 config HAS_OMAP
-	bool
+	bool "Texas Instruments OMAP UART driver"
 	default y
 	depends on ARM_32
 	help
@@ -45,7 +45,7 @@ config HAS_OMAP
 	  Instruments based CPU, say Y.
 
 config HAS_SCIF
-	bool
+	bool "SuperH SCI(F) UART driver"
 	default y
 	depends on ARM
 	help
@@ -53,7 +53,7 @@ config HAS_SCIF
 	  or Renesas R-Car Gen 2/3 based board say Y.
 
 config HAS_EHCI
-	bool
+	bool "EHCI UART driver"
 	help
 	  This selects the USB based EHCI debug port to be used as a UART. If
 	  you have an x86 based system with USB, say Y.
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 3/6] arm: make it possible to disable the SMMU driver

[Stefano Stabellini]

Introduce a Kconfig option for the ARM SMMUv2 driver.

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
CC: jbeulich@suse.com
---
 xen/drivers/passthrough/Kconfig      | 2 ++
 xen/drivers/passthrough/arm/Kconfig  | 7 +++++++
 xen/drivers/passthrough/arm/Makefile | 2 +-
 3 files changed, 10 insertions(+), 1 deletion(-)
 create mode 100644 xen/drivers/passthrough/arm/Kconfig

diff --git a/xen/drivers/passthrough/Kconfig b/xen/drivers/passthrough/Kconfig
index 8d90b67..2fe94fa 100644
--- a/xen/drivers/passthrough/Kconfig
+++ b/xen/drivers/passthrough/Kconfig
@@ -1,3 +1,5 @@
 
 config HAS_PASSTHROUGH
 	bool
+
+source "drivers/passthrough/arm/Kconfig"
diff --git a/xen/drivers/passthrough/arm/Kconfig b/xen/drivers/passthrough/arm/Kconfig
new file mode 100644
index 0000000..98318d9
--- /dev/null
+++ b/xen/drivers/passthrough/arm/Kconfig
@@ -0,0 +1,7 @@
+
+config HAS_SMMUv2
+	bool "ARM SMMUv2 driver"
+	default y
+	depends on ARM
+	---help---
+	  Driver for the ARM SMMU version 2, a popular IOMMU by ARM.
diff --git a/xen/drivers/passthrough/arm/Makefile b/xen/drivers/passthrough/arm/Makefile
index f4cd26e..4db273a 100644
--- a/xen/drivers/passthrough/arm/Makefile
+++ b/xen/drivers/passthrough/arm/Makefile
@@ -1,2 +1,2 @@
 obj-y += iommu.o
-obj-y += smmu.o
+obj-$(HAS_SMMUv2) += smmu.o
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 4/6] arm: add a small kconfig for Renesas RCar H3

[Stefano Stabellini]

This is a reference tiny kconfig for Renesas RCar.  In terms of
schedulers, it selects credit and NULL only.  It enables all the ARM64
errata.

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
CC: artem_mygaiev@epam.com
CC: volodymyr_babchuk@epam.com

---

This patch is untested on Renesas RCar, please test!
Also, I am not sure whether some of the errata workarounds can be
disabled on the RCar.
---
 xen/arch/arm/configs/renesas.config | 80 +++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)
 create mode 100644 xen/arch/arm/configs/renesas.config

diff --git a/xen/arch/arm/configs/renesas.config b/xen/arch/arm/configs/renesas.config
new file mode 100644
index 0000000..7ad3f1c
--- /dev/null
+++ b/xen/arch/arm/configs/renesas.config
@@ -0,0 +1,80 @@
+#
+# Automatically generated file; DO NOT EDIT.
+# Xen/arm 4.11-unstable Configuration
+#
+CONFIG_64BIT=y
+CONFIG_ARM_64=y
+CONFIG_ARM=y
+CONFIG_ARCH_DEFCONFIG="arch/arm/configs/arm64_defconfig"
+
+#
+# Architecture Features
+#
+CONFIG_NR_CPUS=8
+# CONFIG_ACPI is not set
+# CONFIG_HAS_GICV3 is not set
+# CONFIG_HAS_MEM_ACCESS is not set
+# CONFIG_NEW_VGIC is not set
+# CONFIG_SBSA_VUART_CONSOLE is not set
+
+#
+# ARM errata workaround via the alternative framework
+#
+CONFIG_ARM64_ERRATUM_827319=y
+CONFIG_ARM64_ERRATUM_824069=y
+CONFIG_ARM64_ERRATUM_819472=y
+CONFIG_ARM64_ERRATUM_832075=y
+CONFIG_ARM64_ERRATUM_834220=y
+CONFIG_HARDEN_BRANCH_PREDICTOR=y
+CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR=y
+
+#
+# Common Features
+#
+CONFIG_HAS_ALTERNATIVE=y
+CONFIG_HAS_DEVICE_TREE=y
+CONFIG_HAS_PDX=y
+# CONFIG_TMEM is not set
+# CONFIG_XSM is not set
+
+#
+# Schedulers
+#
+CONFIG_SCHED_CREDIT=y
+# CONFIG_SCHED_CREDIT2 is not set
+# CONFIG_SCHED_RTDS is not set
+# CONFIG_SCHED_ARINC653 is not set
+CONFIG_SCHED_NULL=y
+# CONFIG_SCHED_CREDIT_DEFAULT is not set
+CONFIG_SCHED_NULL_DEFAULT=y
+CONFIG_SCHED_DEFAULT="null"
+# CONFIG_LIVEPATCH is not set
+# CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS is not set
+CONFIG_CMDLINE=""
+
+#
+# Device Drivers
+#
+# CONFIG_HAS_NS16550 is not set
+# CONFIG_HAS_CADENCE_UART is not set
+# CONFIG_HAS_MVEBU is not set
+# CONFIG_HAS_PL011 is not set
+CONFIG_HAS_SCIF=y
+# CONFIG_HAS_EHCI is not set
+CONFIG_HAS_PASSTHROUGH=y
+CONFIG_HAS_SMMUv2=y
+# CONFIG_VIDEO is not set
+# CONFIG_HAS_ARM_HDLCD is not set
+CONFIG_DEFCONFIG_LIST="$ARCH_DEFCONFIG"
+
+#
+# Debugging Options
+#
+# CONFIG_DEBUG is not set
+# CONFIG_FRAME_POINTER is not set
+# CONFIG_COVERAGE is not set
+# CONFIG_LOCK_PROFILE is not set
+# CONFIG_PERF_COUNTERS is not set
+# CONFIG_VERBOSE_DEBUG is not set
+# CONFIG_DEVICE_TREE_DEBUG is not set
+# CONFIG_SCRUB_DEBUG is not set
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 5/6] arm: add a small kconfig for qemu-system-aarch64

[Stefano Stabellini]

Disable erratas because they don't apply to QEMU's software emulated
CPUs.

Arbitrarily choose a limit of 4 CPUs.

Select the credit and NULL schedulers only.

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
---
 xen/arch/arm/configs/qemu.config | 81 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)
 create mode 100644 xen/arch/arm/configs/qemu.config

diff --git a/xen/arch/arm/configs/qemu.config b/xen/arch/arm/configs/qemu.config
new file mode 100644
index 0000000..6351cfd
--- /dev/null
+++ b/xen/arch/arm/configs/qemu.config
@@ -0,0 +1,81 @@
+#
+# Automatically generated file; DO NOT EDIT.
+# Xen/arm 4.11-unstable Configuration
+#
+CONFIG_64BIT=y
+CONFIG_ARM_64=y
+CONFIG_ARM=y
+CONFIG_ARCH_DEFCONFIG="arch/arm/configs/arm64_defconfig"
+
+#
+# Architecture Features
+#
+CONFIG_NR_CPUS=4
+# CONFIG_ACPI is not set
+CONFIG_HAS_GICV3=y
+# CONFIG_HAS_MEM_ACCESS is not set
+# CONFIG_HAS_ITS is not set
+# CONFIG_NEW_VGIC is not set
+# CONFIG_SBSA_VUART_CONSOLE is not set
+
+#
+# ARM errata workaround via the alternative framework
+#
+# CONFIG_ARM64_ERRATUM_827319 is not set
+# CONFIG_ARM64_ERRATUM_824069 is not set
+# CONFIG_ARM64_ERRATUM_819472 is not set
+# CONFIG_ARM64_ERRATUM_832075 is not set
+# CONFIG_ARM64_ERRATUM_834220 is not set
+CONFIG_HARDEN_BRANCH_PREDICTOR=y
+CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR=y
+
+#
+# Common Features
+#
+CONFIG_HAS_ALTERNATIVE=y
+CONFIG_HAS_DEVICE_TREE=y
+CONFIG_HAS_PDX=y
+# CONFIG_TMEM is not set
+# CONFIG_XSM is not set
+
+#
+# Schedulers
+#
+CONFIG_SCHED_CREDIT=y
+# CONFIG_SCHED_CREDIT2 is not set
+# CONFIG_SCHED_RTDS is not set
+# CONFIG_SCHED_ARINC653 is not set
+CONFIG_SCHED_NULL=y
+CONFIG_SCHED_CREDIT_DEFAULT=y
+# CONFIG_SCHED_NULL_DEFAULT is not set
+CONFIG_SCHED_DEFAULT="credit"
+# CONFIG_LIVEPATCH is not set
+# CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS is not set
+CONFIG_CMDLINE=""
+
+#
+# Device Drivers
+#
+# CONFIG_HAS_NS16550 is not set
+# CONFIG_HAS_CADENCE_UART is not set
+# CONFIG_HAS_MVEBU is not set
+CONFIG_HAS_PL011=y
+# CONFIG_HAS_SCIF is not set
+# CONFIG_HAS_EHCI is not set
+CONFIG_HAS_PASSTHROUGH=y
+# CONFIG_HAS_SMMUv2 is not set
+# CONFIG_VIDEO is not set
+# CONFIG_HAS_ARM_HDLCD is not set
+CONFIG_DEFCONFIG_LIST="$ARCH_DEFCONFIG"
+
+#
+# Debugging Options
+#
+# CONFIG_DEBUG is not set
+# CONFIG_FRAME_POINTER is not set
+# CONFIG_COVERAGE is not set
+# CONFIG_LOCK_PROFILE is not set
+# CONFIG_PERF_COUNTERS is not set
+# CONFIG_VERBOSE_DEBUG is not set
+# CONFIG_DEVICE_TREE_DEBUG is not set
+# CONFIG_SCRUB_DEBUG is not set
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 6/6] xen: add cloc target

[Stefano Stabellini]

Add a Xen build target to count the lines of code of the source files
built. Uses `cloc' to do the job.

Generate the list of source files from the %.o targets, append output
to "sourcelist".

Remove sourcelist on clean, and also at the beginning of the build
target to avoid appending to sourcelist on consequence builds. Otherwise
one could imagine sourcelist could become large if the user builds Xen
repeatedly without calling clean.

For the cloc target, first clean, then build to make sure all files are
properly accounted (no partial builds).

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
CC: jbeulich@suse.com
CC: andrew.cooper3@citrix.com
---
 xen/Makefile | 14 ++++++++++++--
 xen/Rules.mk |  2 ++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/xen/Makefile b/xen/Makefile
index 62d479c..f53fd22 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -48,7 +48,7 @@ else
 endif
 
 .PHONY: _build
-_build: $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
+_build: clean-sourcelist $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
 
 .PHONY: _install
 _install: D=$(DESTDIR)
@@ -108,7 +108,7 @@ _debug:
 	$(OBJDUMP) -D -S $(TARGET)-syms > $(TARGET).s
 
 .PHONY: _clean
-_clean: delete-unfresh-files
+_clean: delete-unfresh-files clean-sourcelist
 	$(MAKE) -C tools clean
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C include clean
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C common clean
@@ -267,3 +267,13 @@ $(KCONFIG_CONFIG):
 include/config/auto.conf.cmd: ;
 
 -include $(BASEDIR)/include/config/auto.conf.cmd
+
+.PHONY: cloc
+cloc: $(BASEDIR)/sourcelist
+	cloc --list-file=$(BASEDIR)/sourcelist
+
+$(BASEDIR)/sourcelist: clean build
+
+.PHONY: clean-sourcelist
+clean-sourcelist:
+	rm -f $(BASEDIR)/sourcelist
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 5337e20..cb48aa7 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -190,9 +190,11 @@ _clean_%/: FORCE
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C $* clean
 
 %.o: %.c Makefile
+	echo `pwd`/$< >> $(BASEDIR)/sourcelist
 	$(CC) $(CFLAGS) -c $< -o $@
 
 %.o: %.S Makefile
+	echo `pwd`/$< >> $(BASEDIR)/sourcelist
 	$(CC) $(AFLAGS) -c $< -o $@
 
 SPECIAL_DATA_SECTIONS := rodata $(foreach a,1 2 4 8 16, \
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 3/6] arm: make it possible to disable the SMMU driver

[Jan Beulich]

>>> On 19.04.18 at 00:15, <sstabellini@kernel.org> wrote:
> --- /dev/null
> +++ b/xen/drivers/passthrough/arm/Kconfig
> @@ -0,0 +1,7 @@
> +
> +config HAS_SMMUv2
> +	bool "ARM SMMUv2 driver"
> +	default y
> +	depends on ARM
> +	---help---
> +	  Driver for the ARM SMMU version 2, a popular IOMMU by ARM.

This being user-visible but not dependent upon EXPERT - what's the
support status of a hypervisor built without that option? Please recall
that as a rule of thumb we try to limit the number of different
configurations people can build without setting EXPERT, such that we
don't have to go long ways to figure out what configuration was used
in case someone reports a problem. People enabling EXPERT are left
on their own anyway.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 6/6] xen: add cloc target

[Jan Beulich]

>>> On 19.04.18 at 00:15, <sstabellini@kernel.org> wrote:
> Add a Xen build target to count the lines of code of the source files
> built. Uses `cloc' to do the job.
> 
> Generate the list of source files from the %.o targets, append output
> to "sourcelist".
> 
> Remove sourcelist on clean, and also at the beginning of the build
> target to avoid appending to sourcelist on consequence builds. Otherwise
> one could imagine sourcelist could become large if the user builds Xen
> repeatedly without calling clean.
> 
> For the cloc target, first clean, then build to make sure all files are
> properly accounted (no partial builds).
> 
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>

All fine, but what I'm missing is why we want something like this in the
first place.

> ---
>  xen/Makefile | 14 ++++++++++++--
>  xen/Rules.mk |  2 ++
>  2 files changed, 14 insertions(+), 2 deletions(-)

.gitignore ?

> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -48,7 +48,7 @@ else
>  endif
>  
>  .PHONY: _build
> -_build: $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
> +_build: clean-sourcelist $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)

Both here and ...

> @@ -267,3 +267,13 @@ $(KCONFIG_CONFIG):
>  include/config/auto.conf.cmd: ;
>  
>  -include $(BASEDIR)/include/config/auto.conf.cmd
> +
> +.PHONY: cloc
> +cloc: $(BASEDIR)/sourcelist
> +	cloc --list-file=$(BASEDIR)/sourcelist
> +
> +$(BASEDIR)/sourcelist: clean build

... here I'm afraid the dependencies aren't right: All dependencies can
be handled in parallel by make, i.e. there's no ordering implication from
the ordering you provide here.

> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -190,9 +190,11 @@ _clean_%/: FORCE
>  	$(MAKE) -f $(BASEDIR)/Rules.mk -C $* clean
>  
>  %.o: %.c Makefile
> +	echo `pwd`/$< >> $(BASEDIR)/sourcelist
>  	$(CC) $(CFLAGS) -c $< -o $@
>  
>  %.o: %.S Makefile
> +	echo `pwd`/$< >> $(BASEDIR)/sourcelist
>  	$(CC) $(AFLAGS) -c $< -o $@

For one I'd prefer if this file was written only when actually processing
the "cloc" target you add. And then - is echo guaranteed to produce all
its output with a single atomic write? Otherwise you risk producing a
complete mess in sourcelist if someone hands -j to make.

Furthermore - are e.g. header files not counting at all?

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 4/6] arm: add a small kconfig for Renesas RCar H3

[Andrew Cooper]

On 18/04/2018 23:15, Stefano Stabellini wrote:
>  xen/arch/arm/configs/renesas.config | 80 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
>  create mode 100644 xen/arch/arm/configs/renesas.config
>
> diff --git a/xen/arch/arm/configs/renesas.config b/xen/arch/arm/configs/renesas.config
> new file mode 100644
> index 0000000..7ad3f1c
> --- /dev/null
> +++ b/xen/arch/arm/configs/renesas.config
> @@ -0,0 +1,80 @@
> +#
> +# Automatically generated file; DO NOT EDIT.
> +# Xen/arm 4.11-unstable Configuration
> +#

This is now the second Kconfig file we've got added into the tree.  As
with the PV-shim config, keeping it up to date is going to be a little
tricky.

I think we either need a script to keep all of the embedded configs up
to date, or switch to a model (similar to the travis randconfig target)
where it becomes a `make defaultconfig` with certain specific options
forced one way or another (similar to `make {tiny,kvm,xen}config` in
Linux).  The latter means that we only store the specific delta
applicable for purpose, and it will probably change less frequently.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 4/6] arm: add a small kconfig for Renesas RCar H3

[Juergen Gross]

On 19/04/18 10:06, Andrew Cooper wrote:
> On 18/04/2018 23:15, Stefano Stabellini wrote:
>>  xen/arch/arm/configs/renesas.config | 80 +++++++++++++++++++++++++++++++++++++
>>  1 file changed, 80 insertions(+)
>>  create mode 100644 xen/arch/arm/configs/renesas.config
>>
>> diff --git a/xen/arch/arm/configs/renesas.config b/xen/arch/arm/configs/renesas.config
>> new file mode 100644
>> index 0000000..7ad3f1c
>> --- /dev/null
>> +++ b/xen/arch/arm/configs/renesas.config
>> @@ -0,0 +1,80 @@
>> +#
>> +# Automatically generated file; DO NOT EDIT.
>> +# Xen/arm 4.11-unstable Configuration
>> +#
> 
> This is now the second Kconfig file we've got added into the tree.  As
> with the PV-shim config, keeping it up to date is going to be a little
> tricky.
> 
> I think we either need a script to keep all of the embedded configs up
> to date, or switch to a model (similar to the travis randconfig target)
> where it becomes a `make defaultconfig` with certain specific options
> forced one way or another (similar to `make {tiny,kvm,xen}config` in
> Linux).  The latter means that we only store the specific delta
> applicable for purpose, and it will probably change less frequently.

I think the easiest way would be to have a config file with only the
required non-default options being specified and then run
"make olddefconfig" against that (or better: a copy of that in order
to avoid modifying a source from git).

I'm just writing a patch to do that for the shim config as it is nasty
to remove shim.config from my patches in case it has been modified by
the build (again) and STGit has picked it up.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 0/6] arm: more kconfig configurability and small default configs

[Julien Grall]

On 18/04/18 23:15, Stefano Stabellini wrote:
> Hi all,

Hi,

> This patch series is the first step toward building a small certifiable
> Xen hypervisor for ARM boards.
> 
> First, the series makes a few changes to allow disabling more kconfig
> options: most of them already exist but cannot be disabled.
> 
> Then, it introduces a reference kconfig for Renesas RCar (due to popular
> demand, candidate for certifications) and for QEMU aarch64 (not for
> certifications, but useful for debugging).

While I could be tempt by more option to be configurable in Kconfig with 
some caveats (see below), I quite dislike the idea of providing a config 
per board. A few reasons:
	- Where do we draw the line on config we accept for a given board?
	- Do we have to test them every release?
	- What if we add a new option?
	- What does it mean in term of security support?

Regarding the Kconfig, allowing more option to be selected by the user 
means that it will increasingly be more difficult for the community to 
help user in debug. More that we today don't provide a way to embed the 
.config in Xen binary. So if you happen to clean you repo, then you are 
screw to reproduce it.

Furthermore, if we decide to impose CONFIG_XEN_EXPERT=y when using those 
.config, then you happen to drop one by mistake you end up rewrite you 
.config which is not really nice.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/6] arm: make it possible to disable more kconfig options

[Julien Grall]

Hi Stefano,

On 18/04/18 23:15, Stefano Stabellini wrote:
> Make it possible to disable the following existing kconfig options:
>    HAS_GICV3
>    HAS_ARM_HDLCD
>    HAS_MEM_ACCESS
> 
> Today they are silent option. This patch adds one line descriptions and
> make them de/selectable.
> 
> Also, do not select VIDEO: make HAS_ARM_HDLCD select VIDEO instead. In
> fact, VIDEO is only needed by HAS_ARM_HDLCD.
> 
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
>   xen/arch/arm/Kconfig      | 15 +++++++++++----
>   xen/drivers/video/Kconfig |  8 +++++++-
>   2 files changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 8174c0c..83963c8 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -12,17 +12,13 @@ config ARM_32
>   config ARM_64
>   	def_bool y
>   	depends on 64BIT
> -	select HAS_GICV3
>   
>   config ARM
>   	def_bool y
>   	select HAS_ALTERNATIVE
> -	select HAS_ARM_HDLCD
>   	select HAS_DEVICE_TREE
> -	select HAS_MEM_ACCESS
>   	select HAS_PASSTHROUGH
>   	select HAS_PDX
> -	select VIDEO
>   
>   config ARCH_DEFCONFIG
>   	string
> @@ -44,6 +40,17 @@ config ACPI
>   
>   config HAS_GICV3
>   	bool
> +	prompt "GICv3 driver"
> +	default y

That's quite different for the existing config. Now you select GICv3 for 
arm32 which we know does not work.

Also, to bike-shed a bit, I feel HAS_* is more to say "we support" over 
this is selectable by the user. So probably some renaming is required here.

> +
> +config HAS_MEM_ACCESS
> +	bool
> +	prompt "Memory Access and VM events"
> +	default y

Most of the memaccess code is not protected by HAS_MEM_ACCESS.  So you 
are going to drop just a couple of hundreds line. Not sure if it is 
worth it in the actual state.

> +	---help---
> +
> +	  Framework to configure memory access types for guests and receive
> +	  related events in userspace.
>   
>   config HAS_ITS
>           bool
> diff --git a/xen/drivers/video/Kconfig b/xen/drivers/video/Kconfig
> index 52e8ce6..26daf9a 100644
> --- a/xen/drivers/video/Kconfig
> +++ b/xen/drivers/video/Kconfig
> @@ -13,4 +13,10 @@ config VGA
>   	  If unsure, say Y.
>   
>   config HAS_ARM_HDLCD

To be honest I would just rip off the driver. I doubt anybody has been 
using it for the past 5 years and only targets vexpress.

> -	bool
> +	bool "ARM HDLCD driver"
> +	default n
> +	select VIDEO
> +	---help---
> +	  Enable Xen video output to ARM HDLCD graphic controllers.
> +
> +	  if unsure, say N.
> 

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/6] arm: make it possible to enable/disable UART drivers

[Julien Grall]

Hi Stefano,

Please CC "THE REST" maintainers here.

On 18/04/18 23:15, Stefano Stabellini wrote:
> All the UART drivers are silent options. Add one line descriptions so
> that can be de/selected via menuconfig.
> 
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
>   xen/drivers/char/Kconfig | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/drivers/char/Kconfig b/xen/drivers/char/Kconfig
> index cc78ec3..b6fcefd 100644
> --- a/xen/drivers/char/Kconfig
> +++ b/xen/drivers/char/Kconfig
> @@ -1,11 +1,11 @@
>   config HAS_NS16550
> -	bool
> +	bool "NS16550 UART driver"

It looks like to me that x86 will require this unconditionally. So 
that's going to break randconfig.

>   	default y
>   	help
>   	  This selects the 16550-series UART support. For most systems, say Y.
>   
>   config HAS_CADENCE_UART
> -	bool
> +	bool "Xilinx Cadence UART driver"
>   	default y
>   	depends on ARM_64
>   	help
> @@ -13,7 +13,7 @@ config HAS_CADENCE_UART
>   	  based board, say Y.
>   
>   config HAS_MVEBU
> -	bool am not entirely sure whether x86 require it by default.
> +	bool "Marvell MVEBU UART driver"
>   	default y
>   	depends on ARM_64
>   	help
> @@ -21,7 +21,7 @@ config HAS_MVEBU
>   	  based board, say Y.
>   
>   config HAS_PL011
> -	bool
> +	bool "ARM PL011 UART driver"
>   	default y
>   	depends on ARM
>   	help
> @@ -29,7 +29,7 @@ config HAS_PL011
>   	  an Integrator/PP2, Integrator/CP or Versatile platform, say Y.
>   
>   config HAS_EXYNOS4210
> -	bool
> +	bool "Samsung Exynos 4210 UART driver"
>   	default y
>   	depends on ARM_32
>   	help
> @@ -37,7 +37,7 @@ config HAS_EXYNOS4210
>   	  Exynos based board, say Y.
>   
>   config HAS_OMAP
> -	bool
> +	bool "Texas Instruments OMAP UART driver"
>   	default y
>   	depends on ARM_32
>   	help
> @@ -45,7 +45,7 @@ config HAS_OMAP
>   	  Instruments based CPU, say Y.
>   
>   config HAS_SCIF
> -	bool
> +	bool "SuperH SCI(F) UART driver"
>   	default y
>   	depends on ARM
>   	help
> @@ -53,7 +53,7 @@ config HAS_SCIF
>   	  or Renesas R-Car Gen 2/3 based board say Y.
>   
>   config HAS_EHCI
> -	bool
> +	bool "EHCI UART driver"

Well, you are going to break at least Arm because this driver is not 
meant to be working there.

Furthermore, it looks like to me that x86 will require this unconditionally.

>   	help
>   	  This selects the USB based EHCI debug port to be used as a UART. If
>   	  you have an x86 based system with USB, say Y.
> 

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 3/6] arm: make it possible to disable the SMMU driver

[Julien Grall]

Hi,

On 18/04/18 23:15, Stefano Stabellini wrote:
> Introduce a Kconfig option for the ARM SMMUv2 driver.

The driver is handling both SMMUv1 and SMMUv2.

> 
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> CC: jbeulich@suse.com
> ---
>   xen/drivers/passthrough/Kconfig      | 2 ++
>   xen/drivers/passthrough/arm/Kconfig  | 7 +++++++
>   xen/drivers/passthrough/arm/Makefile | 2 +-
>   3 files changed, 10 insertions(+), 1 deletion(-)
>   create mode 100644 xen/drivers/passthrough/arm/Kconfig
> 
> diff --git a/xen/drivers/passthrough/Kconfig b/xen/drivers/passthrough/Kconfig
> index 8d90b67..2fe94fa 100644
> --- a/xen/drivers/passthrough/Kconfig
> +++ b/xen/drivers/passthrough/Kconfig
> @@ -1,3 +1,5 @@
>   
>   config HAS_PASSTHROUGH
>   	bool
> +
> +source "drivers/passthrough/arm/Kconfig"

Can't we load arm/Kconfig only when arm is selected? This would avoid 
weird option on x86.

> diff --git a/xen/drivers/passthrough/arm/Kconfig b/xen/drivers/passthrough/arm/Kconfig
> new file mode 100644
> index 0000000..98318d9
> --- /dev/null
> +++ b/xen/drivers/passthrough/arm/Kconfig
> @@ -0,0 +1,7 @@
> +
> +config HAS_SMMUv2
> +	bool "ARM SMMUv2 driver"
> +	default y
> +	depends on ARM
> +	---help---
> +	  Driver for the ARM SMMU version 2, a popular IOMMU by ARM.
> diff --git a/xen/drivers/passthrough/arm/Makefile b/xen/drivers/passthrough/arm/Makefile
> index f4cd26e..4db273a 100644
> --- a/xen/drivers/passthrough/arm/Makefile
> +++ b/xen/drivers/passthrough/arm/Makefile
> @@ -1,2 +1,2 @@
>   obj-y += iommu.o
> -obj-y += smmu.o
> +obj-$(HAS_SMMUv2) += smmu.o
> 

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 3/6] arm: make it possible to disable the SMMU driver

[Julien Grall]

On 19/04/18 16:44, Julien Grall wrote:
> Hi,
> 
> On 18/04/18 23:15, Stefano Stabellini wrote:
>> Introduce a Kconfig option for the ARM SMMUv2 driver.
> 
> The driver is handling both SMMUv1 and SMMUv2.

I forgot to mention that by disabling the driver, you are going to 
end-up giving the SMMU to Dom0. I don't think we want that to happen.

So you need to find a way to blacklist all the SMMUs.

> 
>>
>> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
>> CC: jbeulich@suse.com
>> ---
>>   xen/drivers/passthrough/Kconfig      | 2 ++
>>   xen/drivers/passthrough/arm/Kconfig  | 7 +++++++
>>   xen/drivers/passthrough/arm/Makefile | 2 +-
>>   3 files changed, 10 insertions(+), 1 deletion(-)
>>   create mode 100644 xen/drivers/passthrough/arm/Kconfig
>>
>> diff --git a/xen/drivers/passthrough/Kconfig 
>> b/xen/drivers/passthrough/Kconfig
>> index 8d90b67..2fe94fa 100644
>> --- a/xen/drivers/passthrough/Kconfig
>> +++ b/xen/drivers/passthrough/Kconfig
>> @@ -1,3 +1,5 @@
>>   config HAS_PASSTHROUGH
>>       bool
>> +
>> +source "drivers/passthrough/arm/Kconfig"
> 
> Can't we load arm/Kconfig only when arm is selected? This would avoid 
> weird option on x86.
> 
>> diff --git a/xen/drivers/passthrough/arm/Kconfig 
>> b/xen/drivers/passthrough/arm/Kconfig
>> new file mode 100644
>> index 0000000..98318d9
>> --- /dev/null
>> +++ b/xen/drivers/passthrough/arm/Kconfig
>> @@ -0,0 +1,7 @@
>> +
>> +config HAS_SMMUv2
>> +    bool "ARM SMMUv2 driver"
>> +    default y
>> +    depends on ARM
>> +    ---help---
>> +      Driver for the ARM SMMU version 2, a popular IOMMU by ARM.
>> diff --git a/xen/drivers/passthrough/arm/Makefile 
>> b/xen/drivers/passthrough/arm/Makefile
>> index f4cd26e..4db273a 100644
>> --- a/xen/drivers/passthrough/arm/Makefile
>> +++ b/xen/drivers/passthrough/arm/Makefile
>> @@ -1,2 +1,2 @@
>>   obj-y += iommu.o
>> -obj-y += smmu.o
>> +obj-$(HAS_SMMUv2) += smmu.o
>>
> 
> Cheers,
> 

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 0/6] arm: more kconfig configurability and small default configs

[Lars Kurth]

On 19/04/2018, 17:32, "Julien Grall" <julien.grall@arm.com> wrote:

    
    
    On 18/04/18 23:15, Stefano Stabellini wrote:
    > Hi all,
    
    Hi,
    
    > This patch series is the first step toward building a small certifiable
    > Xen hypervisor for ARM boards.
    > 
    > First, the series makes a few changes to allow disabling more kconfig
    > options: most of them already exist but cannot be disabled.
    > 
    > Then, it introduces a reference kconfig for Renesas RCar (due to popular
    > demand, candidate for certifications) and for QEMU aarch64 (not for
    > certifications, but useful for debugging).
    
    While I could be tempt by more option to be configurable in Kconfig with 
    some caveats (see below), I quite dislike the idea of providing a config 
    per board. A few reasons:
    	- Where do we draw the line on config we accept for a given board?
    	- Do we have to test them every release?
    	- What if we add a new option?
    	- What does it mean in term of security support?
    
    Regarding the Kconfig, allowing more option to be selected by the user 
    means that it will increasingly be more difficult for the community to 
    help user in debug. More that we today don't provide a way to embed the 
    .config in Xen binary. So if you happen to clean you repo, then you are 
    screw to reproduce it.
    
    Furthermore, if we decide to impose CONFIG_XEN_EXPERT=y when using those 
    .config, then you happen to drop one by mistake you end up rewrite you 
    .config which is not really nice.
    
Maybe the easiest way to solve this is to emit the config into the serial output. That should be easy enough to implement while not requiring any ABI/interface changes
Lars
    

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 3/6] arm: make it possible to disable the SMMU driver

[Stefano Stabellini]

On Thu, 19 Apr 2018, Jan Beulich wrote:
> >>> On 19.04.18 at 00:15, <sstabellini@kernel.org> wrote:
> > --- /dev/null
> > +++ b/xen/drivers/passthrough/arm/Kconfig
> > @@ -0,0 +1,7 @@
> > +
> > +config HAS_SMMUv2
> > +	bool "ARM SMMUv2 driver"
> > +	default y
> > +	depends on ARM
> > +	---help---
> > +	  Driver for the ARM SMMU version 2, a popular IOMMU by ARM.
> 
> This being user-visible but not dependent upon EXPERT - what's the
> support status of a hypervisor built without that option?

Let me take a step back to give you a bit of context to this work, which
will help us decide the best way forward in terms of security support. I
am CC'ing the REST maintainers as well.

With Xen on ARM taking off in embedded, IoT, and automotive, we are
seeing more and more uses of Xen in constrained environments. Users and
system integrators want the smallest Xen and Dom0 configurations. Some
of these deployments require certifications, where you definitely want
the smallest lines of code count. I provided a patch to give us the
lines of code count exactly for that purpose. For reference, one of the
selling points of ACRN is the small code size [0].

I expect kconfig options to proliferate and, generally, all-purpose
kconfigs becoming less attractive going forward.  Having kconfig options
for drivers that should be security supported otherwise is inevitable.


Let's take this example: SMMUv2. The driver is decently quality, and
definitely some boards will require it. It makes sense to security
support it. However, some boards don't come with an SMMU at all, such as
Pine64, or have a different SMMU, such as the Renesas RCar. Certainly,
the smallest kconfig for Pine64/Renesas Rcar shouldn't have the SMMUv2
driver [1]. I think it makes sense to allow the SMMUv2 driver to be
disabled by users. The same argument goes for all of the UART drivers:
each board has only one type of UART, it doesn't make sense to enable
more than one in a kconfig [2].

I think it is OK *not* to security support any option that depends on
EXPERT. But I also think we should allow for options that are
user-visible *and* security supported.

What do you guys think?


[0] https://projectacrn.org/
[1] I realize I enabled the SMMUv2 in the Renesas Rcar kconfig by
    mistake, I'll fix in v2.
[2] See patch #2


> Please recall that as a rule of thumb we try to limit the number of
> different configurations people can build without setting EXPERT, such
> that we don't have to go long ways to figure out what configuration
> was used in case someone reports a problem. People enabling EXPERT are
> left on their own anyway.

To address this specific concern I would like to embed the .config into
the Xen binary, so that we can go back and ask for the kconfig used to
generated Xen easily, see:

https://xenproject.atlassian.net/browse/XEN-38

I intend to come up with a patch and add it to this series.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 0/6] arm: more kconfig configurability and small default configs

[Stefano Stabellini]

On Thu, 19 Apr 2018, Lars Kurth wrote:
> 19/04/2018, 17:32, "Julien Grall" <julien.grall@arm.com> wrote:
> 
>     
>     
>     On 18/04/18 23:15, Stefano Stabellini wrote:
>     > Hi all,
>     
>     Hi,
>     
>     > This patch series is the first step toward building a small certifiable
>     > Xen hypervisor for ARM boards.
>     > 
>     > First, the series makes a few changes to allow disabling more kconfig
>     > options: most of them already exist but cannot be disabled.
>     > 
>     > Then, it introduces a reference kconfig for Renesas RCar (due to popular
>     > demand, candidate for certifications) and for QEMU aarch64 (not for
>     > certifications, but useful for debugging).
>     
>     While I could be tempt by more option to be configurable in Kconfig with 
>     some caveats (see below), I quite dislike the idea of providing a config 
>     per board. A few reasons:
>     	- Where do we draw the line on config we accept for a given board?
>     	- Do we have to test them every release?
>     	- What if we add a new option?

I discussed this topic with Julien over a call this morning. It seems to
us that the best way forward is to follow this simple rule:

* Only accept and keep kconfig that are regularly tested for each release *

We already maintain a table of supported boards for Xen on ARM. We ask
members of the community to test Xen on these boards during the code
freeze. We mark as supported only the ones for which we get positive
responses.

In this case, if nobody steps up to test Xen with a specific kconfig
during the code freeze, the kconfig will be dropped.


>     	- What does it mean in term of security support?

I replied to this question here:

https://marc.info/?l=xen-devel&m=152417791426130


>     Regarding the Kconfig, allowing more option to be selected by the user 
>     means that it will increasingly be more difficult for the community to 
>     help user in debug. More that we today don't provide a way to embed the 
>     .config in Xen binary. So if you happen to clean you repo, then you are 
>     screw to reproduce it.
>     
>     Furthermore, if we decide to impose CONFIG_XEN_EXPERT=y when using those 
>     .config, then you happen to drop one by mistake you end up rewrite you 
>     .config which is not really nice.
>     
> Maybe the easiest way to solve this is to emit the config into the serial output. That should be easy enough to implement while not requiring any ABI/interface changes

We have a better idea: we can embedded the kconfig into the Xen binary.
In fact, we even have a Jira ticket for it already:

https://xenproject.atlassian.net/browse/XEN-38

I'll try to come up with a patch and make it part of this series.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 4/6] arm: add a small kconfig for Renesas RCar H3

[Stefano Stabellini]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2069 bytes --]

On Thu, 19 Apr 2018, Juergen Gross wrote:
> On 19/04/18 10:06, Andrew Cooper wrote:
> > On 18/04/2018 23:15, Stefano Stabellini wrote:
> >>  xen/arch/arm/configs/renesas.config | 80 +++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 80 insertions(+)
> >>  create mode 100644 xen/arch/arm/configs/renesas.config
> >>
> >> diff --git a/xen/arch/arm/configs/renesas.config b/xen/arch/arm/configs/renesas.config
> >> new file mode 100644
> >> index 0000000..7ad3f1c
> >> --- /dev/null
> >> +++ b/xen/arch/arm/configs/renesas.config
> >> @@ -0,0 +1,80 @@
> >> +#
> >> +# Automatically generated file; DO NOT EDIT.
> >> +# Xen/arm 4.11-unstable Configuration
> >> +#
> > 
> > This is now the second Kconfig file we've got added into the tree.  As
> > with the PV-shim config, keeping it up to date is going to be a little
> > tricky.
> > 
> > I think we either need a script to keep all of the embedded configs up
> > to date, or switch to a model (similar to the travis randconfig target)
> > where it becomes a `make defaultconfig` with certain specific options
> > forced one way or another (similar to `make {tiny,kvm,xen}config` in
> > Linux).  The latter means that we only store the specific delta
> > applicable for purpose, and it will probably change less frequently.
>
> I think the easiest way would be to have a config file with only the
> required non-default options being specified and then run
> "make olddefconfig" against that (or better: a copy of that in order
> to avoid modifying a source from git).
> 
> I'm just writing a patch to do that for the shim config as it is nasty
> to remove shim.config from my patches in case it has been modified by
> the build (again) and STGit has picked it up.

Given that the goal of this kconfig is to provide the smallest possible
kconfig for a given board, I think that your suggestion wouldn't end up
improving things much in this case because most options will have to be
specified as "disabled", otherwise "make olddefconfig" would end up
enabling some of them by default, which is not what we want.

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 6/6] xen: add cloc target

[Stefano Stabellini]

On Thu, 19 Apr 2018, Jan Beulich wrote:
> >>> On 19.04.18 at 00:15, <sstabellini@kernel.org> wrote:
> > Add a Xen build target to count the lines of code of the source files
> > built. Uses `cloc' to do the job.
> > 
> > Generate the list of source files from the %.o targets, append output
> > to "sourcelist".
> > 
> > Remove sourcelist on clean, and also at the beginning of the build
> > target to avoid appending to sourcelist on consequence builds. Otherwise
> > one could imagine sourcelist could become large if the user builds Xen
> > repeatedly without calling clean.
> > 
> > For the cloc target, first clean, then build to make sure all files are
> > properly accounted (no partial builds).
> > 
> > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> All fine, but what I'm missing is why we want something like this in the
> first place.

I provided an explanation here:
https://marc.info/?l=xen-devel&m=152417791426130, but I can elaborate
more if you have questions.

Thanks for the sharp review.



> > ---
> >  xen/Makefile | 14 ++++++++++++--
> >  xen/Rules.mk |  2 ++
> >  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> .gitignore ?

Good point, I'll add sourcelist to .gitignore


> > --- a/xen/Makefile
> > +++ b/xen/Makefile
> > @@ -48,7 +48,7 @@ else
> >  endif
> >  
> >  .PHONY: _build
> > -_build: $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
> > +_build: clean-sourcelist $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
> 
> Both here and ...
> 
> > @@ -267,3 +267,13 @@ $(KCONFIG_CONFIG):
> >  include/config/auto.conf.cmd: ;
> >  
> >  -include $(BASEDIR)/include/config/auto.conf.cmd
> > +
> > +.PHONY: cloc
> > +cloc: $(BASEDIR)/sourcelist
> > +	cloc --list-file=$(BASEDIR)/sourcelist
> > +
> > +$(BASEDIR)/sourcelist: clean build
> 
> ... here I'm afraid the dependencies aren't right: All dependencies can
> be handled in parallel by make, i.e. there's no ordering implication from
> the ordering you provide here.

I see what you mean. Nasty. Do you have a suggestion on how to better
handle this kind of thing?


> > --- a/xen/Rules.mk
> > +++ b/xen/Rules.mk
> > @@ -190,9 +190,11 @@ _clean_%/: FORCE
> >  	$(MAKE) -f $(BASEDIR)/Rules.mk -C $* clean
> >  
> >  %.o: %.c Makefile
> > +	echo `pwd`/$< >> $(BASEDIR)/sourcelist
> >  	$(CC) $(CFLAGS) -c $< -o $@
> >  
> >  %.o: %.S Makefile
> > +	echo `pwd`/$< >> $(BASEDIR)/sourcelist
> >  	$(CC) $(AFLAGS) -c $< -o $@
> 
> For one I'd prefer if this file was written only when actually processing
> the "cloc" target you add. 

I can make the echo command conditional on the cloc target using a
global flag.


> And then - is echo guaranteed to produce all
> its output with a single atomic write? Otherwise you risk producing a
> complete mess in sourcelist if someone hands -j to make.

I haven't seen this issue in my tests so far. POSIX guarantees that
write requests of PIPE_BUF bytes or less shall not be interleaved.
PIPE_BUF is 4K on Linux and is always greater than 512, which should be
fine here. Therefore it is down to the echo implementation, as you
pointed out.

Honestly, I would prefer to trust the echo implementation to do the
right thing, and risk a corruption in sourcelist, rather than
introducing file locks to solve the problem. What is your take on this?


> Furthermore - are e.g. header files not counting at all?

I have been wondering about that too. I am not sure, but I have seen
other LOC counts in the past ignoring header files. In any case, I
thought that C and ASM files would be a good start. 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 4/6] arm: add a small kconfig for Renesas RCar H3

[Juergen Gross]

On 20/04/18 01:00, Stefano Stabellini wrote:
> On Thu, 19 Apr 2018, Juergen Gross wrote:
>> On 19/04/18 10:06, Andrew Cooper wrote:
>>> On 18/04/2018 23:15, Stefano Stabellini wrote:
>>>>  xen/arch/arm/configs/renesas.config | 80 +++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 80 insertions(+)
>>>>  create mode 100644 xen/arch/arm/configs/renesas.config
>>>>
>>>> diff --git a/xen/arch/arm/configs/renesas.config b/xen/arch/arm/configs/renesas.config
>>>> new file mode 100644
>>>> index 0000000..7ad3f1c
>>>> --- /dev/null
>>>> +++ b/xen/arch/arm/configs/renesas.config
>>>> @@ -0,0 +1,80 @@
>>>> +#
>>>> +# Automatically generated file; DO NOT EDIT.
>>>> +# Xen/arm 4.11-unstable Configuration
>>>> +#
>>>
>>> This is now the second Kconfig file we've got added into the tree.  As
>>> with the PV-shim config, keeping it up to date is going to be a little
>>> tricky.
>>>
>>> I think we either need a script to keep all of the embedded configs up
>>> to date, or switch to a model (similar to the travis randconfig target)
>>> where it becomes a `make defaultconfig` with certain specific options
>>> forced one way or another (similar to `make {tiny,kvm,xen}config` in
>>> Linux).  The latter means that we only store the specific delta
>>> applicable for purpose, and it will probably change less frequently.
>>
>> I think the easiest way would be to have a config file with only the
>> required non-default options being specified and then run
>> "make olddefconfig" against that (or better: a copy of that in order
>> to avoid modifying a source from git).
>>
>> I'm just writing a patch to do that for the shim config as it is nasty
>> to remove shim.config from my patches in case it has been modified by
>> the build (again) and STGit has picked it up.
> 
> Given that the goal of this kconfig is to provide the smallest possible
> kconfig for a given board, I think that your suggestion wouldn't end up
> improving things much in this case because most options will have to be
> specified as "disabled", otherwise "make olddefconfig" would end up
> enabling some of them by default, which is not what we want.
> 

What's the problem with this?

In case you really do want a config with many non-default settings
you'll have to specify them. So just do it and it's fine.

The alternative would be to specify _all_ entries. I don't see why
this would be better.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 3/6] arm: make it possible to disable the SMMU driver

[Jan Beulich]

>>> On 19.04.18 at 17:44, <julien.grall@arm.com> wrote:
> On 18/04/18 23:15, Stefano Stabellini wrote:
>> --- a/xen/drivers/passthrough/Kconfig
>> +++ b/xen/drivers/passthrough/Kconfig
>> @@ -1,3 +1,5 @@
>>   
>>   config HAS_PASSTHROUGH
>>   	bool
>> +
>> +source "drivers/passthrough/arm/Kconfig"
> 
> Can't we load arm/Kconfig only when arm is selected? This would avoid 
> weird option on x86.

How would those options appear for x86, given ...

>> --- /dev/null
>> +++ b/xen/drivers/passthrough/arm/Kconfig
>> @@ -0,0 +1,7 @@
>> +
>> +config HAS_SMMUv2
>> +	bool "ARM SMMUv2 driver"
>> +	default y
>> +	depends on ARM

... this "depends on"? Of course if more options are going to appear here,
wrapping the whole file in "if ARM" would be useful anyway, even if only
to limit redundancy.

Otoh, as voiced before in a different context, I'm not convinced we want
such single-option Kconfig files, i.e. in the case here the option may better
go directly into xen/drivers/passthrough/Kconfig. Splitting files when they
grow too large is an option anyway.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 3/6] arm: make it possible to disable the SMMU driver

[Jan Beulich]

>>> On 20.04.18 at 00:43, <sstabellini@kernel.org> wrote:
> On Thu, 19 Apr 2018, Jan Beulich wrote:
>> >>> On 19.04.18 at 00:15, <sstabellini@kernel.org> wrote:
>> > --- /dev/null
>> > +++ b/xen/drivers/passthrough/arm/Kconfig
>> > @@ -0,0 +1,7 @@
>> > +
>> > +config HAS_SMMUv2
>> > +	bool "ARM SMMUv2 driver"
>> > +	default y
>> > +	depends on ARM
>> > +	---help---
>> > +	  Driver for the ARM SMMU version 2, a popular IOMMU by ARM.
>> 
>> This being user-visible but not dependent upon EXPERT - what's the
>> support status of a hypervisor built without that option?
> 
> Let me take a step back to give you a bit of context to this work, which
> will help us decide the best way forward in terms of security support. I
> am CC'ing the REST maintainers as well.
> 
> With Xen on ARM taking off in embedded, IoT, and automotive, we are
> seeing more and more uses of Xen in constrained environments. Users and
> system integrators want the smallest Xen and Dom0 configurations. Some
> of these deployments require certifications, where you definitely want
> the smallest lines of code count. I provided a patch to give us the
> lines of code count exactly for that purpose. For reference, one of the
> selling points of ACRN is the small code size [0].
> 
> I expect kconfig options to proliferate and, generally, all-purpose
> kconfigs becoming less attractive going forward.  Having kconfig options
> for drivers that should be security supported otherwise is inevitable.
> 
> 
> Let's take this example: SMMUv2. The driver is decently quality, and
> definitely some boards will require it. It makes sense to security
> support it. However, some boards don't come with an SMMU at all, such as
> Pine64, or have a different SMMU, such as the Renesas RCar. Certainly,
> the smallest kconfig for Pine64/Renesas Rcar shouldn't have the SMMUv2
> driver [1]. I think it makes sense to allow the SMMUv2 driver to be
> disabled by users. The same argument goes for all of the UART drivers:
> each board has only one type of UART, it doesn't make sense to enable
> more than one in a kconfig [2].
> 
> I think it is OK *not* to security support any option that depends on
> EXPERT. But I also think we should allow for options that are
> user-visible *and* security supported.
> 
> What do you guys think?

How about a limited set of "canned" configurations which are security
supported?

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 6/6] xen: add cloc target

[Jan Beulich]

>>> On 20.04.18 at 01:22, <sstabellini@kernel.org> wrote:
> On Thu, 19 Apr 2018, Jan Beulich wrote:
>> >>> On 19.04.18 at 00:15, <sstabellini@kernel.org> wrote:
>> > Add a Xen build target to count the lines of code of the source files
>> > built. Uses `cloc' to do the job.
>> > 
>> > Generate the list of source files from the %.o targets, append output
>> > to "sourcelist".
>> > 
>> > Remove sourcelist on clean, and also at the beginning of the build
>> > target to avoid appending to sourcelist on consequence builds. Otherwise
>> > one could imagine sourcelist could become large if the user builds Xen
>> > repeatedly without calling clean.
>> > 
>> > For the cloc target, first clean, then build to make sure all files are
>> > properly accounted (no partial builds).
>> > 
>> > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
>> 
>> All fine, but what I'm missing is why we want something like this in the
>> first place.
> 
> I provided an explanation here:
> https://marc.info/?l=xen-devel&m=152417791426130, but I can elaborate
> more if you have questions.

Yeah with that explanation I can see where you're coming from. Some
of that needs to go into the commit message here though.

>> > --- a/xen/Makefile
>> > +++ b/xen/Makefile
>> > @@ -48,7 +48,7 @@ else
>> >  endif
>> >  
>> >  .PHONY: _build
>> > -_build: $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
>> > +_build: clean-sourcelist $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
>> 
>> Both here and ...
>> 
>> > @@ -267,3 +267,13 @@ $(KCONFIG_CONFIG):
>> >  include/config/auto.conf.cmd: ;
>> >  
>> >  -include $(BASEDIR)/include/config/auto.conf.cmd
>> > +
>> > +.PHONY: cloc
>> > +cloc: $(BASEDIR)/sourcelist
>> > +	cloc --list-file=$(BASEDIR)/sourcelist
>> > +
>> > +$(BASEDIR)/sourcelist: clean build
>> 
>> ... here I'm afraid the dependencies aren't right: All dependencies can
>> be handled in parallel by make, i.e. there's no ordering implication from
>> the ordering you provide here.
> 
> I see what you mean. Nasty. Do you have a suggestion on how to better
> handle this kind of thing?

I think you'll need intermediate (pseudo-)targets, or you need to invoke the
second step as command instead of that being a dependency.

>> > --- a/xen/Rules.mk
>> > +++ b/xen/Rules.mk
>> > @@ -190,9 +190,11 @@ _clean_%/: FORCE
>> >  	$(MAKE) -f $(BASEDIR)/Rules.mk -C $* clean
>> >  
>> >  %.o: %.c Makefile
>> > +	echo `pwd`/$< >> $(BASEDIR)/sourcelist
>> >  	$(CC) $(CFLAGS) -c $< -o $@
>> >  
>> >  %.o: %.S Makefile
>> > +	echo `pwd`/$< >> $(BASEDIR)/sourcelist
>> >  	$(CC) $(AFLAGS) -c $< -o $@
>> 
>> For one I'd prefer if this file was written only when actually processing
>> the "cloc" target you add. 
> 
> I can make the echo command conditional on the cloc target using a
> global flag.

A global flag would mean ifdef-ary here, which I'd like to avoid. Instead I
was hoping for you to define a macro which expands to nothing in the
non-cloc target case.

>> And then - is echo guaranteed to produce all
>> its output with a single atomic write? Otherwise you risk producing a
>> complete mess in sourcelist if someone hands -j to make.
> 
> I haven't seen this issue in my tests so far. POSIX guarantees that
> write requests of PIPE_BUF bytes or less shall not be interleaved.
> PIPE_BUF is 4K on Linux and is always greater than 512, which should be
> fine here. Therefore it is down to the echo implementation, as you
> pointed out.
> 
> Honestly, I would prefer to trust the echo implementation to do the
> right thing, and risk a corruption in sourcelist, rather than
> introducing file locks to solve the problem. What is your take on this?

Can't you use a different approach altogether, e.g. grep-ing the
.*.d files once the whole build is done?

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 4/6] arm: add a small kconfig for Renesas RCar H3

[Stefano Stabellini]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2557 bytes --]

On Fri, 20 Apr 2018, Juergen Gross wrote:
> On 20/04/18 01:00, Stefano Stabellini wrote:
> > On Thu, 19 Apr 2018, Juergen Gross wrote:
> >> On 19/04/18 10:06, Andrew Cooper wrote:
> >>> On 18/04/2018 23:15, Stefano Stabellini wrote:
> >>>>  xen/arch/arm/configs/renesas.config | 80 +++++++++++++++++++++++++++++++++++++
> >>>>  1 file changed, 80 insertions(+)
> >>>>  create mode 100644 xen/arch/arm/configs/renesas.config
> >>>>
> >>>> diff --git a/xen/arch/arm/configs/renesas.config b/xen/arch/arm/configs/renesas.config
> >>>> new file mode 100644
> >>>> index 0000000..7ad3f1c
> >>>> --- /dev/null
> >>>> +++ b/xen/arch/arm/configs/renesas.config
> >>>> @@ -0,0 +1,80 @@
> >>>> +#
> >>>> +# Automatically generated file; DO NOT EDIT.
> >>>> +# Xen/arm 4.11-unstable Configuration
> >>>> +#
> >>>
> >>> This is now the second Kconfig file we've got added into the tree.  As
> >>> with the PV-shim config, keeping it up to date is going to be a little
> >>> tricky.
> >>>
> >>> I think we either need a script to keep all of the embedded configs up
> >>> to date, or switch to a model (similar to the travis randconfig target)
> >>> where it becomes a `make defaultconfig` with certain specific options
> >>> forced one way or another (similar to `make {tiny,kvm,xen}config` in
> >>> Linux).  The latter means that we only store the specific delta
> >>> applicable for purpose, and it will probably change less frequently.
> >>
> >> I think the easiest way would be to have a config file with only the
> >> required non-default options being specified and then run
> >> "make olddefconfig" against that (or better: a copy of that in order
> >> to avoid modifying a source from git).
> >>
> >> I'm just writing a patch to do that for the shim config as it is nasty
> >> to remove shim.config from my patches in case it has been modified by
> >> the build (again) and STGit has picked it up.
> > 
> > Given that the goal of this kconfig is to provide the smallest possible
> > kconfig for a given board, I think that your suggestion wouldn't end up
> > improving things much in this case because most options will have to be
> > specified as "disabled", otherwise "make olddefconfig" would end up
> > enabling some of them by default, which is not what we want.
> > 
> 
> What's the problem with this?
> 
> In case you really do want a config with many non-default settings
> you'll have to specify them. So just do it and it's fine.
> 
> The alternative would be to specify _all_ entries. I don't see why
> this would be better.

Yes, you are right :-)

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 3/6] arm: make it possible to disable the SMMU driver

[Stefano Stabellini]

On Fri, 20 Apr 2018, Jan Beulich wrote:
> >>> On 20.04.18 at 00:43, <sstabellini@kernel.org> wrote:
> > On Thu, 19 Apr 2018, Jan Beulich wrote:
> >> >>> On 19.04.18 at 00:15, <sstabellini@kernel.org> wrote:
> >> > --- /dev/null
> >> > +++ b/xen/drivers/passthrough/arm/Kconfig
> >> > @@ -0,0 +1,7 @@
> >> > +
> >> > +config HAS_SMMUv2
> >> > +	bool "ARM SMMUv2 driver"
> >> > +	default y
> >> > +	depends on ARM
> >> > +	---help---
> >> > +	  Driver for the ARM SMMU version 2, a popular IOMMU by ARM.
> >> 
> >> This being user-visible but not dependent upon EXPERT - what's the
> >> support status of a hypervisor built without that option?
> > 
> > Let me take a step back to give you a bit of context to this work, which
> > will help us decide the best way forward in terms of security support. I
> > am CC'ing the REST maintainers as well.
> > 
> > With Xen on ARM taking off in embedded, IoT, and automotive, we are
> > seeing more and more uses of Xen in constrained environments. Users and
> > system integrators want the smallest Xen and Dom0 configurations. Some
> > of these deployments require certifications, where you definitely want
> > the smallest lines of code count. I provided a patch to give us the
> > lines of code count exactly for that purpose. For reference, one of the
> > selling points of ACRN is the small code size [0].
> > 
> > I expect kconfig options to proliferate and, generally, all-purpose
> > kconfigs becoming less attractive going forward.  Having kconfig options
> > for drivers that should be security supported otherwise is inevitable.
> > 
> > 
> > Let's take this example: SMMUv2. The driver is decently quality, and
> > definitely some boards will require it. It makes sense to security
> > support it. However, some boards don't come with an SMMU at all, such as
> > Pine64, or have a different SMMU, such as the Renesas RCar. Certainly,
> > the smallest kconfig for Pine64/Renesas Rcar shouldn't have the SMMUv2
> > driver [1]. I think it makes sense to allow the SMMUv2 driver to be
> > disabled by users. The same argument goes for all of the UART drivers:
> > each board has only one type of UART, it doesn't make sense to enable
> > more than one in a kconfig [2].
> > 
> > I think it is OK *not* to security support any option that depends on
> > EXPERT. But I also think we should allow for options that are
> > user-visible *and* security supported.
> > 
> > What do you guys think?
> 
> How about a limited set of "canned" configurations which are security
> supported?

Yes, I think that is a good idea. It is a natural evolution of what we
are already doing.


As I mentioned here https://marc.info/?l=xen-devel&m=152417800326142 in
the context of testing, I think we should only keep in-tree kconfigs
that are well tested for each release. On ARM it is not obvious as the
kconfigs are board specific, so we need somebody with the hardware to
run the tests and send back results twice a year.

With this simple rule in place, I think the number of kconfigs will be
drastically reduced to only the ones for boards that are actively used
and supported.


Moreover, specifically regarding security support, most of the ARM
kconfigs will have similar and extremely small set of kconfig options.
The only differences will be in terms of drivers: UART drivers, SMMU
drivers, GIC drivers, and little else.

Today, all these drivers options are already security supported, it is
only the number of combinations that will increase. However, generally,
I don't think that different combinations of drivers (UART, SMMU, etc)
should cause any problems: even today only one driver for each kind of
devices is actually used at runtime.


In other words, provided that we have a way to embed the .config into
the binary, I think that security-supporting a limited set of "canned"
configurations shouldn't pose any additional efforts onto the security
team. Of course, if it turns out that it generates more work than we are
prepared to handle, we can always revisit this decision.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 0/6] arm: more kconfig configurability and small default configs

[Andrii Anisov]

Hello Stefano,

I'm really glad to see this series.

In the list of platform-specific bits you are trying to make selectable, 
you missed platform support code itself. Moreover, as it is said in your 
follow-up emails, set of UART, IOMMU, GIC is defined by SoC family. So 
wouldn't it be more natural to introduce platform configs which would 
couple specific drivers support? With a special platform i.e. "other" 
which enables all drivers and platform code.


-- 

*Andrii Anisov*



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 0/6] arm: more kconfig configurability and small default configs

[Andrii Anisov]

Hello Stefano,

I'm really glad to see this series.

In the list of platform-specific bits you are trying to make selectable, 
you missed platform support code itself. Moreover, as it is said in your 
follow-up emails, set of UART, IOMMU, GIC is defined by SoC family. So 
wouldn't it be more natural to introduce platform configs which would 
couple specific drivers support? With a special platform i.e. "other" 
which enables all drivers and platform code.


-- 

*Andrii Anisov*



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 3/6] arm: make it possible to disable the SMMU driver

[Andrii Anisov]

Hello Stefano,


On 20.04.18 01:43, Stefano Stabellini wrote:
> Let's take this example: SMMUv2. The driver is decently quality, and
> definitely some boards will require it. It makes sense to security
> support it. However, some boards don't come with an SMMU at all, such as
> Pine64
I guess arm passthrough in a whole could be dropped for such platforms.

-- 

*Andrii Anisov*



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 4/6] arm: add a small kconfig for Renesas RCar H3

[Andrii Anisov]

Hello Stefano,


If we stick with platform specific configuration file:


On 19.04.18 01:15, Stefano Stabellini wrote:
> This is a reference tiny kconfig for Renesas RCar.  In terms of
> schedulers, it selects credit and NULL only.  It enables all the ARM64
> errata.
>
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> CC: artem_mygaiev@epam.com
> CC: volodymyr_babchuk@epam.com
>
> ---
>
> This patch is untested on Renesas RCar, please test!
I will test it once we are aligned with it.

> Also, I am not sure whether some of the errata workarounds can be
> disabled on the RCar.
> ---
>   xen/arch/arm/configs/renesas.config | 80 +++++++++++++++++++++++++++++++++++++
>   1 file changed, 80 insertions(+)
>   create mode 100644 xen/arch/arm/configs/renesas.config
>
> diff --git a/xen/arch/arm/configs/renesas.config b/xen/arch/arm/configs/renesas.config
Better name it rcar3.config. XEN still supports Renesas R-Car Gen2 which 
is quite different (at least 32-bit).

> new file mode 100644
> index 0000000..7ad3f1c
> --- /dev/null
> +++ b/xen/arch/arm/configs/renesas.config
> @@ -0,0 +1,80 @@
> +#
> +# Automatically generated file; DO NOT EDIT.
> +# Xen/arm 4.11-unstable Configuration
> +#
> +CONFIG_64BIT=y
> +CONFIG_ARM_64=y
> +CONFIG_ARM=y
> +CONFIG_ARCH_DEFCONFIG="arch/arm/configs/arm64_defconfig"
> +
> +#
> +# Architecture Features
> +#
> +CONFIG_NR_CPUS=8
> +# CONFIG_ACPI is not set
> +# CONFIG_HAS_GICV3 is not set
> +# CONFIG_HAS_MEM_ACCESS is not set
> +# CONFIG_NEW_VGIC is not set
> +# CONFIG_SBSA_VUART_CONSOLE is not set
> +
> +#
> +# ARM errata workaround via the alternative framework
> +#
> +CONFIG_ARM64_ERRATUM_827319=y
> +CONFIG_ARM64_ERRATUM_824069=y
> +CONFIG_ARM64_ERRATUM_819472=y
> +CONFIG_ARM64_ERRATUM_832075=y
> +CONFIG_ARM64_ERRATUM_834220=y
> +CONFIG_HARDEN_BRANCH_PREDICTOR=y
> +CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR=y
> +
> +#
> +# Common Features
> +#
> +CONFIG_HAS_ALTERNATIVE=y
> +CONFIG_HAS_DEVICE_TREE=y
> +CONFIG_HAS_PDX=y
> +# CONFIG_TMEM is not set
> +# CONFIG_XSM is not set
> +
> +#
> +# Schedulers
> +#
> +CONFIG_SCHED_CREDIT=y
> +# CONFIG_SCHED_CREDIT2 is not set
> +# CONFIG_SCHED_RTDS is not set
> +# CONFIG_SCHED_ARINC653 is not set
> +CONFIG_SCHED_NULL=y
> +# CONFIG_SCHED_CREDIT_DEFAULT is not set
> +CONFIG_SCHED_NULL_DEFAULT=y
> +CONFIG_SCHED_DEFAULT="null"
> +# CONFIG_LIVEPATCH is not set
> +# CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS is not set
> +CONFIG_CMDLINE=""
> +
> +#
> +# Device Drivers
> +#
> +# CONFIG_HAS_NS16550 is not set
> +# CONFIG_HAS_CADENCE_UART is not set
> +# CONFIG_HAS_MVEBU is not set
> +# CONFIG_HAS_PL011 is not set
> +CONFIG_HAS_SCIF=y
> +# CONFIG_HAS_EHCI is not set
> +CONFIG_HAS_PASSTHROUGH=y
IMHO, no passthrough support until IPMMU driver upstreamed.

> +CONFIG_HAS_SMMUv2=y
> +# CONFIG_VIDEO is not set
> +# CONFIG_HAS_ARM_HDLCD is not set
> +CONFIG_DEFCONFIG_LIST="$ARCH_DEFCONFIG"
> +
> +#
> +# Debugging Options
> +#
> +# CONFIG_DEBUG is not set
> +# CONFIG_FRAME_POINTER is not set
> +# CONFIG_COVERAGE is not set
> +# CONFIG_LOCK_PROFILE is not set
> +# CONFIG_PERF_COUNTERS is not set
> +# CONFIG_VERBOSE_DEBUG is not set
> +# CONFIG_DEVICE_TREE_DEBUG is not set
> +# CONFIG_SCRUB_DEBUG is not set

But I would rather embed platform support into Kconfigs. Specifying in a 
configuration file something like CONFIG_RCAR3 and software entities 
like schedulers, livepatching, debug, etc.

-- 

*Andrii Anisov*



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 3/6] arm: make it possible to disable the SMMU driver

[Julien Grall]

On 04/20/2018 10:39 AM, Jan Beulich wrote:
>>>> On 19.04.18 at 17:44, <julien.grall@arm.com> wrote:
>> On 18/04/18 23:15, Stefano Stabellini wrote:
>>> --- a/xen/drivers/passthrough/Kconfig
>>> +++ b/xen/drivers/passthrough/Kconfig
>>> @@ -1,3 +1,5 @@
>>>    
>>>    config HAS_PASSTHROUGH
>>>    	bool
>>> +
>>> +source "drivers/passthrough/arm/Kconfig"
>>
>> Can't we load arm/Kconfig only when arm is selected? This would avoid
>> weird option on x86.
> 
> How would those options appear for x86, given ...
> 
>>> --- /dev/null
>>> +++ b/xen/drivers/passthrough/arm/Kconfig
>>> @@ -0,0 +1,7 @@
>>> +
>>> +config HAS_SMMUv2
>>> +	bool "ARM SMMUv2 driver"
>>> +	default y
>>> +	depends on ARM
> 
> ... this "depends on"? Of course if more options are going to appear here,
> wrapping the whole file in "if ARM" would be useful anyway, even if only
> to limit redundancy.

It will appear when you search for it in the menuconfig. This will look 
quite weird for x86.

> 
> Otoh, as voiced before in a different context, I'm not convinced we want
> such single-option Kconfig files, i.e. in the case here the option may better
> go directly into xen/drivers/passthrough/Kconfig. Splitting files when they
> grow too large is an option anyway.

I would be ok with that.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 4/6] arm: add a small kconfig for Renesas RCar H3

[Julien Grall]

Hi,

On 04/23/2018 07:30 PM, Andrii Anisov wrote:
> On 19.04.18 01:15, Stefano Stabellini wrote:
>> +#
>> +# Device Drivers
>> +#
>> +# CONFIG_HAS_NS16550 is not set
>> +# CONFIG_HAS_CADENCE_UART is not set
>> +# CONFIG_HAS_MVEBU is not set
>> +# CONFIG_HAS_PL011 is not set
>> +CONFIG_HAS_SCIF=y
>> +# CONFIG_HAS_EHCI is not set
>> +CONFIG_HAS_PASSTHROUGH=y
> IMHO, no passthrough support until IPMMU driver upstreamed.

You can't unselect HAS_PASSTHROUGH support. Given that you are going to 
have passthrough in the future, I don't much see the point to try to 
allow that option to be disabled.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 4/6] arm: add a small kconfig for Renesas RCar H3

[Andrii Anisov]

Hello Julien,


On 24.04.18 12:01, Julien Grall wrote:
> You can't unselect HAS_PASSTHROUGH support. Given that you are going 
> to have passthrough in the future, I don't much see the point to try 
> to allow that option to be disabled.
If we are speaking of R-Car Gen3, you might be right. We are targeting 
IPMMU support upstreamed. What about IOMMU-less platforms?

-- 

*Andrii Anisov*



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 4/6] arm: add a small kconfig for Renesas RCar H3

[Julien Grall]

On 04/24/2018 10:52 AM, Andrii Anisov wrote:
> Hello Julien,

Hi Andrii,

> On 24.04.18 12:01, Julien Grall wrote:
>> You can't unselect HAS_PASSTHROUGH support. Given that you are going 
>> to have passthrough in the future, I don't much see the point to try 
>> to allow that option to be disabled.
> If we are speaking of R-Car Gen3, you might be right. We are targeting 
> IPMMU support upstreamed. What about IOMMU-less platforms?
For everything you disable, you double the number of configuration 
possible. So we have to weight the benefits of disabling some code.

I believe that passthrough should be kept as core Xen, it is small and 
quite tight to the rest of Xen.

But I would be interested to know what would be the rationale to disable 
that. Do you foresee certification on IOMMU-less platform?

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 4/6] arm: add a small kconfig for Renesas RCar H3

[Andrii Anisov]

Hello Julien,


On 24.04.18 13:06, Julien Grall wrote:
> I believe that passthrough should be kept as core Xen, it is small and 
> quite tight to the rest of Xen.
It might be.

> But I would be interested to know what would be the rationale to 
> disable that. Do you foresee certification on IOMMU-less platform?
I would give TI Jacinto6 as an example. It is an automotive focused SoC, 
has no IOMMU. BTW, I have a strong feeling that some J6 based automotive 
systems with XEN already reached production stage. I'm not really sure 
if they are certified, but anyway.
So, yes, I'm pretty sure IOMMU-less platforms with XEN would require 
certification.

-- 

*Andrii Anisov*



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 4/6] arm: add a small kconfig for Renesas RCar H3

[Julien Grall]

On 24/04/2018 12:04, Andrii Anisov wrote:
> Hello Julien,
> 
> 
> On 24.04.18 13:06, Julien Grall wrote:
>> I believe that passthrough should be kept as core Xen, it is small and 
>> quite tight to the rest of Xen.
> It might be.
> 
>> But I would be interested to know what would be the rationale to 
>> disable that. Do you foresee certification on IOMMU-less platform?
> I would give TI Jacinto6 as an example. It is an automotive focused SoC, 
> has no IOMMU. BTW, I have a strong feeling that some J6 based automotive 
> systems with XEN already reached production stage. I'm not really sure 
> if they are certified, but anyway.
> So, yes, I'm pretty sure IOMMU-less platforms with XEN would require 
> certification.

Can you quantify what would be the cost of keeping that code around for 
IOMMU-less platform?

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 4/6] arm: add a small kconfig for Renesas RCar H3

[Andrii Anisov]

> Can you quantify what would be the cost of keeping that code around 
> for IOMMU-less platform?
I'm not sure I understand your question. Do you mean a number of loc of 
the passthrough feature for arm?

-- 

*Andrii Anisov*



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 4/6] arm: add a small kconfig for Renesas RCar H3

[Julien Grall]

Hi,

On 04/24/2018 03:18 PM, Andrii Anisov wrote:
> 
>> Can you quantify what would be the cost of keeping that code around 
>> for IOMMU-less platform?
> I'm not sure I understand your question. Do you mean a number of loc of 
> the passthrough feature for arm?

I meant that disabling something in Xen will come with a cost. While for 
a driver the maintenance is fairly minimal for anything touching core 
Xen it will require some more work for any change. So I understand that 
it will make Xen slightly smaller (~600 lines), but at what cost?

In other words, I am all for disabling unnecessary driver in Xen with 
some caveats (see my other answers). But I am quite worry on the burden 
for anything else without any real assessment.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 4/6] arm: add a small kconfig for Renesas RCar H3

[Stefano Stabellini]

On Tue, 24 Apr 2018, Julien Grall wrote:
> Hi,
> 
> On 04/24/2018 03:18 PM, Andrii Anisov wrote:
> > 
> > > Can you quantify what would be the cost of keeping that code around for
> > > IOMMU-less platform?
> > I'm not sure I understand your question. Do you mean a number of loc of the
> > passthrough feature for arm?
> 
> I meant that disabling something in Xen will come with a cost. While for a
> driver the maintenance is fairly minimal for anything touching core Xen it
> will require some more work for any change. So I understand that it will make
> Xen slightly smaller (~600 lines), but at what cost?
> 
> In other words, I am all for disabling unnecessary driver in Xen with some
> caveats (see my other answers). But I am quite worry on the burden for
> anything else without any real assessment.

It is very difficult to quantify the cost of adding a new Kconfig, I
don't think anybody would be able to answer that question :-)

But I think you have a point. When adding Kconfig options, it is easy to
forget the trade-offs and start adding one for everything. However, the
less self-contained the code, the higher the maintenance cost of the new
option.

PASSTHROUGH is an example of something very small in terms of lines of
code and not quite self-contained. A great example of something that
is probably not worth making optional.

My idea is to start from the low hanging fruits with high value returns
(lots of lines of code to remove). Once we are done with those, we can
go back and evaluate things like PASSTHROUGH.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/6] arm: make it possible to disable more kconfig options

[Stefano Stabellini]

On Thu, 19 Apr 2018, Julien Grall wrote:
> Hi Stefano,
> 
> On 18/04/18 23:15, Stefano Stabellini wrote:
> > Make it possible to disable the following existing kconfig options:
> >    HAS_GICV3
> >    HAS_ARM_HDLCD
> >    HAS_MEM_ACCESS
> > 
> > Today they are silent option. This patch adds one line descriptions and
> > make them de/selectable.
> > 
> > Also, do not select VIDEO: make HAS_ARM_HDLCD select VIDEO instead. In
> > fact, VIDEO is only needed by HAS_ARM_HDLCD.
> > 
> > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > ---
> >   xen/arch/arm/Kconfig      | 15 +++++++++++----
> >   xen/drivers/video/Kconfig |  8 +++++++-
> >   2 files changed, 18 insertions(+), 5 deletions(-)
> > 
> > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > index 8174c0c..83963c8 100644
> > --- a/xen/arch/arm/Kconfig
> > +++ b/xen/arch/arm/Kconfig
> > @@ -12,17 +12,13 @@ config ARM_32
> >   config ARM_64
> >   	def_bool y
> >   	depends on 64BIT
> > -	select HAS_GICV3
> >     config ARM
> >   	def_bool y
> >   	select HAS_ALTERNATIVE
> > -	select HAS_ARM_HDLCD
> >   	select HAS_DEVICE_TREE
> > -	select HAS_MEM_ACCESS
> >   	select HAS_PASSTHROUGH
> >   	select HAS_PDX
> > -	select VIDEO
> >     config ARCH_DEFCONFIG
> >   	string
> > @@ -44,6 +40,17 @@ config ACPI
> >     config HAS_GICV3
> >   	bool
> > +	prompt "GICv3 driver"
> > +	default y
> 
> That's quite different for the existing config. Now you select GICv3 for arm32
> which we know does not work.

Ah! I'll fix that.


> Also, to bike-shed a bit, I feel HAS_* is more to say "we support" over this
> is selectable by the user. So probably some renaming is required here.

OK, should I just remove the HAS_ prefix (for example HAS_GICV3 ->
GICV3) or do you have a better suggestion? In any case, I'll do in a
separate patch to make it easier to review.


> > +
> > +config HAS_MEM_ACCESS
> > +	bool
> > +	prompt "Memory Access and VM events"
> > +	default y
> 
> Most of the memaccess code is not protected by HAS_MEM_ACCESS.  So you are
> going to drop just a couple of hundreds line. Not sure if it is worth it in
> the actual state.

Yes, the LOC count it is not worth it today, but I would still like to
make it selectable because I don't want Xen to come to rely on having
HAS_MEM_ACCESS enabled all the time. !MEM_ACCESS is a good and valid
configuration. Also, we can go forward in making more stuff protected by
HAS_MEM_ACCESS soon.


> > +	---help---
> > +
> > +	  Framework to configure memory access types for guests and receive
> > +	  related events in userspace.
> >     config HAS_ITS
> >           bool
> > diff --git a/xen/drivers/video/Kconfig b/xen/drivers/video/Kconfig
> > index 52e8ce6..26daf9a 100644
> > --- a/xen/drivers/video/Kconfig
> > +++ b/xen/drivers/video/Kconfig
> > @@ -13,4 +13,10 @@ config VGA
> >   	  If unsure, say Y.
> >     config HAS_ARM_HDLCD
> 
> To be honest I would just rip off the driver. I doubt anybody has been using
> it for the past 5 years and only targets vexpress.

It is true, the only reason for keeping it would be as a reference
example, but I don't know if it is worth having it in the tree just for
that. I'll remove it completely, unless you tell me otherwise.


> > -	bool
> > +	bool "ARM HDLCD driver"
> > +	default n
> > +	select VIDEO
> > +	---help---
> > +	  Enable Xen video output to ARM HDLCD graphic controllers.
> > +
> > +	  if unsure, say N.
> > 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/6] arm: make it possible to enable/disable UART drivers

[Stefano Stabellini]

On Thu, 19 Apr 2018, Julien Grall wrote:
> Hi Stefano,
> 
> Please CC "THE REST" maintainers here.
> 
> On 18/04/18 23:15, Stefano Stabellini wrote:
> > All the UART drivers are silent options. Add one line descriptions so
> > that can be de/selected via menuconfig.
> > 
> > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > ---
> >   xen/drivers/char/Kconfig | 16 ++++++++--------
> >   1 file changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/xen/drivers/char/Kconfig b/xen/drivers/char/Kconfig
> > index cc78ec3..b6fcefd 100644
> > --- a/xen/drivers/char/Kconfig
> > +++ b/xen/drivers/char/Kconfig
> > @@ -1,11 +1,11 @@
> >   config HAS_NS16550
> > -	bool
> > +	bool "NS16550 UART driver"
> 
> It looks like to me that x86 will require this unconditionally. So that's
> going to break randconfig.

Thank you, I'll fix


> >   	default y
> >   	help
> >   	  This selects the 16550-series UART support. For most systems, say Y.
> >     config HAS_CADENCE_UART
> > -	bool
> > +	bool "Xilinx Cadence UART driver"
> >   	default y
> >   	depends on ARM_64
> >   	help
> > @@ -13,7 +13,7 @@ config HAS_CADENCE_UART
> >   	  based board, say Y.
> >     config HAS_MVEBU
> > -	bool am not entirely sure whether x86 require it by default.
> > +	bool "Marvell MVEBU UART driver"
> >   	default y
> >   	depends on ARM_64
> >   	help
> > @@ -21,7 +21,7 @@ config HAS_MVEBU
> >   	  based board, say Y.
> >     config HAS_PL011
> > -	bool
> > +	bool "ARM PL011 UART driver"
> >   	default y
> >   	depends on ARM
> >   	help
> > @@ -29,7 +29,7 @@ config HAS_PL011
> >   	  an Integrator/PP2, Integrator/CP or Versatile platform, say Y.
> >     config HAS_EXYNOS4210
> > -	bool
> > +	bool "Samsung Exynos 4210 UART driver"
> >   	default y
> >   	depends on ARM_32
> >   	help
> > @@ -37,7 +37,7 @@ config HAS_EXYNOS4210
> >   	  Exynos based board, say Y.
> >     config HAS_OMAP
> > -	bool
> > +	bool "Texas Instruments OMAP UART driver"
> >   	default y
> >   	depends on ARM_32
> >   	help
> > @@ -45,7 +45,7 @@ config HAS_OMAP
> >   	  Instruments based CPU, say Y.
> >     config HAS_SCIF
> > -	bool
> > +	bool "SuperH SCI(F) UART driver"
> >   	default y
> >   	depends on ARM
> >   	help
> > @@ -53,7 +53,7 @@ config HAS_SCIF
> >   	  or Renesas R-Car Gen 2/3 based board say Y.
> >     config HAS_EHCI
> > -	bool
> > +	bool "EHCI UART driver"
> 
> Well, you are going to break at least Arm because this driver is not meant to
> be working there.
> 
> Furthermore, it looks like to me that x86 will require this unconditionally.

I'll fix this too


> >   	help
> >   	  This selects the USB based EHCI debug port to be used as a UART. If
> >   	  you have an x86 based system with USB, say Y.
> > 
> 
> Cheers,
> 
> -- 
> Julien Grall
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/6] arm: make it possible to enable/disable UART drivers

[Stefano Stabellini]

On Mon, 21 May 2018, Stefano Stabellini wrote:
> On Thu, 19 Apr 2018, Julien Grall wrote:
> > Hi Stefano,
> > 
> > Please CC "THE REST" maintainers here.
> > 
> > On 18/04/18 23:15, Stefano Stabellini wrote:
> > > All the UART drivers are silent options. Add one line descriptions so
> > > that can be de/selected via menuconfig.
> > > 
> > > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > > ---
> > >   xen/drivers/char/Kconfig | 16 ++++++++--------
> > >   1 file changed, 8 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/xen/drivers/char/Kconfig b/xen/drivers/char/Kconfig
> > > index cc78ec3..b6fcefd 100644
> > > --- a/xen/drivers/char/Kconfig
> > > +++ b/xen/drivers/char/Kconfig
> > > @@ -1,11 +1,11 @@
> > >   config HAS_NS16550
> > > -	bool
> > > +	bool "NS16550 UART driver"
> > 
> > It looks like to me that x86 will require this unconditionally. So that's
> > going to break randconfig.
> 
> Thank you, I'll fix
> 
> 
> > >   	default y
> > >   	help
> > >   	  This selects the 16550-series UART support. For most systems, say Y.
> > >     config HAS_CADENCE_UART
> > > -	bool
> > > +	bool "Xilinx Cadence UART driver"
> > >   	default y
> > >   	depends on ARM_64
> > >   	help
> > > @@ -13,7 +13,7 @@ config HAS_CADENCE_UART
> > >   	  based board, say Y.
> > >     config HAS_MVEBU
> > > -	bool am not entirely sure whether x86 require it by default.
> > > +	bool "Marvell MVEBU UART driver"
> > >   	default y
> > >   	depends on ARM_64
> > >   	help
> > > @@ -21,7 +21,7 @@ config HAS_MVEBU
> > >   	  based board, say Y.
> > >     config HAS_PL011
> > > -	bool
> > > +	bool "ARM PL011 UART driver"
> > >   	default y
> > >   	depends on ARM
> > >   	help
> > > @@ -29,7 +29,7 @@ config HAS_PL011
> > >   	  an Integrator/PP2, Integrator/CP or Versatile platform, say Y.
> > >     config HAS_EXYNOS4210
> > > -	bool
> > > +	bool "Samsung Exynos 4210 UART driver"
> > >   	default y
> > >   	depends on ARM_32
> > >   	help
> > > @@ -37,7 +37,7 @@ config HAS_EXYNOS4210
> > >   	  Exynos based board, say Y.
> > >     config HAS_OMAP
> > > -	bool
> > > +	bool "Texas Instruments OMAP UART driver"
> > >   	default y
> > >   	depends on ARM_32
> > >   	help
> > > @@ -45,7 +45,7 @@ config HAS_OMAP
> > >   	  Instruments based CPU, say Y.
> > >     config HAS_SCIF
> > > -	bool
> > > +	bool "SuperH SCI(F) UART driver"
> > >   	default y
> > >   	depends on ARM
> > >   	help
> > > @@ -53,7 +53,7 @@ config HAS_SCIF
> > >   	  or Renesas R-Car Gen 2/3 based board say Y.
> > >     config HAS_EHCI
> > > -	bool
> > > +	bool "EHCI UART driver"
> > 
> > Well, you are going to break at least Arm because this driver is not meant to
> > be working there.
> > 
> > Furthermore, it looks like to me that x86 will require this unconditionally.
> 
> I'll fix this too

Actually x86 already select both, I just have to make HAS_EHCI depend on
x86.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 3/6] arm: make it possible to disable the SMMU driver

[Stefano Stabellini]

On Tue, 24 Apr 2018, Julien Grall wrote:
> On 04/20/2018 10:39 AM, Jan Beulich wrote:
> > > > > On 19.04.18 at 17:44, <julien.grall@arm.com> wrote:
> > > On 18/04/18 23:15, Stefano Stabellini wrote:
> > > > --- a/xen/drivers/passthrough/Kconfig
> > > > +++ b/xen/drivers/passthrough/Kconfig
> > > > @@ -1,3 +1,5 @@
> > > >       config HAS_PASSTHROUGH
> > > >    	bool
> > > > +
> > > > +source "drivers/passthrough/arm/Kconfig"
> > > 
> > > Can't we load arm/Kconfig only when arm is selected? This would avoid
> > > weird option on x86.
> > 
> > How would those options appear for x86, given ...
> > 
> > > > --- /dev/null
> > > > +++ b/xen/drivers/passthrough/arm/Kconfig
> > > > @@ -0,0 +1,7 @@
> > > > +
> > > > +config HAS_SMMUv2
> > > > +	bool "ARM SMMUv2 driver"
> > > > +	default y
> > > > +	depends on ARM
> > 
> > ... this "depends on"? Of course if more options are going to appear here,
> > wrapping the whole file in "if ARM" would be useful anyway, even if only
> > to limit redundancy.

Sorry, I think I am misunderstanding your suggestion. If you are
suggesting:

#if ARM
  config HAS_SMMUv2
  bool "ARM SMMUv2 driver"
  default y
  depends on ARM
#endif

then, this won't work because we are not running the kconfig files
through the preprocessor.


> It will appear when you search for it in the menuconfig. This will look quite
> weird for x86.

Is this a problem? If so, how do we want to fix it? I am OK with
anything, but if it was up to me I would leave it as is (HAS_SMMUv2
comes up on x86 searches on the menu).


> > Otoh, as voiced before in a different context, I'm not convinced we want
> > such single-option Kconfig files, i.e. in the case here the option may
> > better
> > go directly into xen/drivers/passthrough/Kconfig. Splitting files when they
> > grow too large is an option anyway.
> 
> I would be ok with that.

I'll make the change

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 6/6] xen: add cloc target

[Stefano Stabellini]

On Fri, 20 Apr 2018, Jan Beulich wrote:
> >>> On 20.04.18 at 01:22, <sstabellini@kernel.org> wrote:
> > On Thu, 19 Apr 2018, Jan Beulich wrote:
> >> >>> On 19.04.18 at 00:15, <sstabellini@kernel.org> wrote:
> >> > Add a Xen build target to count the lines of code of the source files
> >> > built. Uses `cloc' to do the job.
> >> > 
> >> > Generate the list of source files from the %.o targets, append output
> >> > to "sourcelist".
> >> > 
> >> > Remove sourcelist on clean, and also at the beginning of the build
> >> > target to avoid appending to sourcelist on consequence builds. Otherwise
> >> > one could imagine sourcelist could become large if the user builds Xen
> >> > repeatedly without calling clean.
> >> > 
> >> > For the cloc target, first clean, then build to make sure all files are
> >> > properly accounted (no partial builds).
> >> > 
> >> > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> >> 
> >> All fine, but what I'm missing is why we want something like this in the
> >> first place.
> > 
> > I provided an explanation here:
> > https://marc.info/?l=xen-devel&m=152417791426130, but I can elaborate
> > more if you have questions.
> 
> Yeah with that explanation I can see where you're coming from. Some
> of that needs to go into the commit message here though.
> 
> >> > --- a/xen/Makefile
> >> > +++ b/xen/Makefile
> >> > @@ -48,7 +48,7 @@ else
> >> >  endif
> >> >  
> >> >  .PHONY: _build
> >> > -_build: $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
> >> > +_build: clean-sourcelist $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
> >> 
> >> Both here and ...
> >> 
> >> > @@ -267,3 +267,13 @@ $(KCONFIG_CONFIG):
> >> >  include/config/auto.conf.cmd: ;
> >> >  
> >> >  -include $(BASEDIR)/include/config/auto.conf.cmd
> >> > +
> >> > +.PHONY: cloc
> >> > +cloc: $(BASEDIR)/sourcelist
> >> > +	cloc --list-file=$(BASEDIR)/sourcelist
> >> > +
> >> > +$(BASEDIR)/sourcelist: clean build
> >> 
> >> ... here I'm afraid the dependencies aren't right: All dependencies can
> >> be handled in parallel by make, i.e. there's no ordering implication from
> >> the ordering you provide here.
> > 
> > I see what you mean. Nasty. Do you have a suggestion on how to better
> > handle this kind of thing?
> 
> I think you'll need intermediate (pseudo-)targets, or you need to invoke the
> second step as command instead of that being a dependency.
> 
> >> > --- a/xen/Rules.mk
> >> > +++ b/xen/Rules.mk
> >> > @@ -190,9 +190,11 @@ _clean_%/: FORCE
> >> >  	$(MAKE) -f $(BASEDIR)/Rules.mk -C $* clean
> >> >  
> >> >  %.o: %.c Makefile
> >> > +	echo `pwd`/$< >> $(BASEDIR)/sourcelist
> >> >  	$(CC) $(CFLAGS) -c $< -o $@
> >> >  
> >> >  %.o: %.S Makefile
> >> > +	echo `pwd`/$< >> $(BASEDIR)/sourcelist
> >> >  	$(CC) $(AFLAGS) -c $< -o $@
> >> 
> >> For one I'd prefer if this file was written only when actually processing
> >> the "cloc" target you add. 
> > 
> > I can make the echo command conditional on the cloc target using a
> > global flag.
> 
> A global flag would mean ifdef-ary here, which I'd like to avoid. Instead I
> was hoping for you to define a macro which expands to nothing in the
> non-cloc target case.
> 
> >> And then - is echo guaranteed to produce all
> >> its output with a single atomic write? Otherwise you risk producing a
> >> complete mess in sourcelist if someone hands -j to make.
> > 
> > I haven't seen this issue in my tests so far. POSIX guarantees that
> > write requests of PIPE_BUF bytes or less shall not be interleaved.
> > PIPE_BUF is 4K on Linux and is always greater than 512, which should be
> > fine here. Therefore it is down to the echo implementation, as you
> > pointed out.
> > 
> > Honestly, I would prefer to trust the echo implementation to do the
> > right thing, and risk a corruption in sourcelist, rather than
> > introducing file locks to solve the problem. What is your take on this?
> 
> Can't you use a different approach altogether, e.g. grep-ing the
> .*.d files once the whole build is done?

Actually, it seems possible. It feels a bit haskish to me, but it has
the benefit of having no impacts whatsoever to the normal build. I'll
change approach to what you suggested.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 0/6] arm: more kconfig configurability and small default configs

[Stefano Stabellini]

On Mon, 23 Apr 2018, Andrii Anisov wrote:
> Hello Stefano,
> 
> I'm really glad to see this series.
> 
> In the list of platform-specific bits you are trying to make selectable, you
> missed platform support code itself. Moreover, as it is said in your follow-up
> emails, set of UART, IOMMU, GIC is defined by SoC family. So wouldn't it be
> more natural to introduce platform configs which would couple specific drivers
> support? With a special platform i.e. "other" which enables all drivers and
> platform code.

I am not sure I understand your suggestion. But I think we are heading
in the direction you are hinting toward with Juergen's suggestion to
only keep kconfig options that are not "default". If you give a look at
v2, the rcar3.config is smaller and doesn't have much more than the
basic drivers for the platform.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 3/6] arm: make it possible to disable the SMMU driver

[Jan Beulich]

>>> Stefano Stabellini <sstabellini@kernel.org> 05/21/18 10:35 PM >>>
>On Tue, 24 Apr 2018, Julien Grall wrote:
>> On 04/20/2018 10:39 AM, Jan Beulich wrote:
>> > > > > On 19.04.18 at 17:44, <julien.grall@arm.com> wrote:
>> > > On 18/04/18 23:15, Stefano Stabellini wrote:
>> > > > --- a/xen/drivers/passthrough/Kconfig
>> > > > +++ b/xen/drivers/passthrough/Kconfig
>> > > > @@ -1,3 +1,5 @@
>> > > >       config HAS_PASSTHROUGH
>> > > >        bool
>> > > > +
>> > > > +source "drivers/passthrough/arm/Kconfig"
>> > > 
>> > > Can't we load arm/Kconfig only when arm is selected? This would avoid
>> > > weird option on x86.
>> > 
>> > How would those options appear for x86, given ...
>> > 
>> > > > --- /dev/null
>> > > > +++ b/xen/drivers/passthrough/arm/Kconfig
>> > > > @@ -0,0 +1,7 @@
>> > > > +
>> > > > +config HAS_SMMUv2
>> > > > +    bool "ARM SMMUv2 driver"
>> > > > +    default y
>> > > > +    depends on ARM
>> > 
>> > ... this "depends on"? Of course if more options are going to appear here,
>> > wrapping the whole file in "if ARM" would be useful anyway, even if only
>> > to limit redundancy.
>
>Sorry, I think I am misunderstanding your suggestion. If you are
>suggesting:
>
>#if ARM
>config HAS_SMMUv2
>bool "ARM SMMUv2 driver"
>default y
>depends on ARM
>#endif
>
>then, this won't work because we are not running the kconfig files
>through the preprocessor.

if ARM
config HAS_SMMUv2
bool "ARM SMMUv2 driver"
default y
endif

The "if" basically adds a respective "depends on" for all enclosed options.


Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/6] arm: make it possible to disable more kconfig options

[Julien Grall]

Hi,

On 05/21/2018 08:58 PM, Stefano Stabellini wrote:
> On Thu, 19 Apr 2018, Julien Grall wrote:
>>> +
>>> +config HAS_MEM_ACCESS
>>> +	bool
>>> +	prompt "Memory Access and VM events"
>>> +	default y
>>
>> Most of the memaccess code is not protected by HAS_MEM_ACCESS.  So you are
>> going to drop just a couple of hundreds line. Not sure if it is worth it in
>> the actual state.
> 
> Yes, the LOC count it is not worth it today, but I would still like to
> make it selectable because I don't want Xen to come to rely on having
> HAS_MEM_ACCESS enabled all the time. !MEM_ACCESS is a good and valid
> configuration. Also, we can go forward in making more stuff protected by
> HAS_MEM_ACCESS soon.

The common code already doesn't rely on memaccess thanks to when Arm was 
not support it. While I agree that we don't want HAS_MEM_ACCESS enabled 
all the time, I question the usefulness of that possibility today. What 
you are going to remove is about ~150 lines of pumbling to the 
userspace. That's it.

All the meat (and complexity) of memaccess is still here (~500 lines). 
You can achieve the same situation with using XSM. So I don't see the 
real benefits of it here.

It would be better to first guard all memaccess code and then expose the 
config if we still think it is useful.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/6] arm: make it possible to disable more kconfig options

[Stefano Stabellini]

On Tue, 22 May 2018, Julien Grall wrote:
> Hi,
> 
> On 05/21/2018 08:58 PM, Stefano Stabellini wrote:
> > On Thu, 19 Apr 2018, Julien Grall wrote:
> > > > +
> > > > +config HAS_MEM_ACCESS
> > > > +	bool
> > > > +	prompt "Memory Access and VM events"
> > > > +	default y
> > > 
> > > Most of the memaccess code is not protected by HAS_MEM_ACCESS.  So you are
> > > going to drop just a couple of hundreds line. Not sure if it is worth it
> > > in
> > > the actual state.
> > 
> > Yes, the LOC count it is not worth it today, but I would still like to
> > make it selectable because I don't want Xen to come to rely on having
> > HAS_MEM_ACCESS enabled all the time. !MEM_ACCESS is a good and valid
> > configuration. Also, we can go forward in making more stuff protected by
> > HAS_MEM_ACCESS soon.
> 
> The common code already doesn't rely on memaccess thanks to when Arm was not
> support it. While I agree that we don't want HAS_MEM_ACCESS enabled all the
> time, I question the usefulness of that possibility today. What you are going
> to remove is about ~150 lines of pumbling to the userspace. That's it.
> 
> All the meat (and complexity) of memaccess is still here (~500 lines). You can
> achieve the same situation with using XSM. So I don't see the real benefits of
> it here.
> 
> It would be better to first guard all memaccess code and then expose the
> config if we still think it is useful.

I am happy to hear that you also don't want HAS_MEM_ACCESS enabled all
the time. Then, it is just a question on when and how. Given that I am
doing kconfig changes now, I prefer to do them all at once, then move to
adding more #define (which I definitely intend to do next). It is very
easy to break these patches, they don't rebase easily, especially the
HAS_MEM_ACCESS rename. This is why I would prefer to make MEM_ACCES
optional as part of this series.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 0/6] arm: more kconfig configurability and small default configs

[Andrii Anisov]

Hello Stefano,


On 22.05.18 02:45, Stefano Stabellini wrote:
> I am not sure I understand your suggestion. But I think we are heading
> in the direction you are hinting toward with Juergen's suggestion to
> only keep kconfig options that are not "default". If you give a look at
> v2, the rcar3.config is smaller and doesn't have much more than the
> basic drivers for the platform.

What I did mean is closer to v3. With one more hint about a selecting 
platform code one can find in xen/arch/arm/platforms.
Something like:

diff --git a/xen/arch/arm/platforms/Makefile 
b/xen/arch/arm/platforms/Makefile
index 53a47e4..c420318 100644
--- a/xen/arch/arm/platforms/Makefile
+++ b/xen/arch/arm/platforms/Makefile
@@ -7,4 +7,4 @@ obj-$(CONFIG_ARM_32) += rcar2.o
  obj-$(CONFIG_ARM_64) += seattle.o
  obj-y += sunxi.o
  obj-$(CONFIG_ARM_64) += xgene-storm.o
-obj-$(CONFIG_ARM_64) += xilinx-zynqmp.o
+obj-$(CONFIG_MPSOC) += xilinx-zynqmp.o

Though, RCAR gen3 does not have the platform code yet. So nothing to 
select for it now ;)

-- 

*Andrii Anisov*



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel