On Thu, 2023-12-07 at 11:00 +0100, Jan Beulich wrote:
> On 07.12.2023 10:22, Oleksii wrote:
> > On Tue, 2023-12-05 at 16:38 +0100, Jan Beulich wrote:
> > > > On 24.11.2023 11:30, Oleksii Kurochko wrote:
> > > > > > The patch also fixes the build script as conf util expects
> > > > > > to have each config on separate line.
> > > >
> > > > The approach doesn't really scale (it's already odd that you
> > > > add
> > > > the
> > > > (apparently) same set four times. There's also zero
> > > > justification
> > > > for
> > > > this kind of an adjustment (as per discussion elsewhere I don't
> > > > think
> > > > we should go this route, and hence arguments towards convincing
> > > > me
> > > > [and
> > > > perhaps others] would be needed here).
> > I agree that this may not be the best approach, but it seems like
> > we
> > don't have too many options to turn off a config for randconfig.
> >
> > To be honest, in my opinion (IMO), randconfig should either be
> > removed
> > or allowed to fail until most of the functionality is ready.
> > Otherwise,
> > there should be a need to introduce HAS_* or depend on
> > 'SUPPORTED_ARCHS' for each config, or introduce a lot of stubs.
> >
> > Could you please suggest a better option?
>
> As to dropping randconfig tests, I'd like to refer you to Andrew, who
> is of the opinion that it was wrong to drop them for ppc. (I'm
> agreeing
> with him when taking a theoretical perspective, but I'm not happy
> with
> the practical consequences.)
>
> As to a better approach: Instead of listing the same set of options
> several times, can't there be a template config which is used to
> force
> randconfig to not touch certain settings? In fact at least for non-
> randconfig purposes I thought tiny64_defconfig / riscv64_defconfig
> already serve kind of a similar purpose. Imo the EXTRA_*CONFIG
> overrides
> are there for at most very few special case settings, not for
> purposes
> like you use them here.
The template will be the really a good option.
What do you think about the following patch which introduces arch-
specific allrandom.config?
diff --git a/xen/Makefile b/xen/Makefile
index ca571103c8..cb1eca76c2 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -336,11 +336,14 @@ ifeq ($(config-build),y)
# *config targets only - make sure prerequisites are updated, and
descend
# in tools/kconfig to make the *config target
+ARCH_ALLRANDOM_CONFIG :=
$(srctree)/arch/$(SRCARCH)/configs/allrandom.config
+
# Create a file for KCONFIG_ALLCONFIG which depends on the
environment.
# This will be use by kconfig targets
allyesconfig/allmodconfig/allnoconfig/randconfig
filechk_kconfig_allconfig = \
$(if $(findstring n,$(XEN_HAS_CHECKPOLICY)), echo
'CONFIG_XSM_FLASK_POLICY=n';) \
- $(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG);) \
+ $(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG); \
+ $(if $(wildcard $(ARCH_ALLRANDOM_CONFIG)), cat
$(ARCH_ALLRANDOM_CONFIG);) ) \
:
If this patch is OK then it can be reused for patches:
https://lore.kernel.org/xen-devel/cdc20255540a66ba0b6946ac6d48c11029cd3385.1701453087.git.oleksii.kuroc...@gmail.com/
https://lore.kernel.org/xen-devel/d42a34866edc70a12736b5c6976aa1b44b4ebd8a.1701453087.git.oleksii.kuroc...@gmail.com/
>
> > > > > > --- a/automation/gitlab-ci/build.yaml
> > > > > > +++ b/automation/gitlab-ci/build.yaml
> > > > > > @@ -522,6 +522,38 @@ archlinux-current-gcc-riscv64:
> > > > > > CONTAINER: archlinux:current-riscv64
> > > > > > KBUILD_DEFCONFIG: tiny64_defconfig
> > > > > > HYPERVISOR_ONLY: y
> > > > > > + EXTRA_XEN_CONFIG:
> > > > > > + CONFIG_COVERAGE=n
> > > > > > + CONFIG_GRANT_TABLE=n
> > > > > > + CONFIG_SCHED_CREDIT=n
> > > > > > + CONFIG_SCHED_CREDIT2=n
> > > > > > + CONFIG_SCHED_RTDS=n
> > > > > > + CONFIG_SCHED_NULL=n
> > > > > > + CONFIG_SCHED_ARINC653=n
> > > > > > + CONFIG_TRACEBUFFER=n
> > > > > > + CONFIG_HYPFS=n
> > > > > > + CONFIG_GRANT_TABLE=n
> > > > > > + CONFIG_SPECULATIVE_HARDEN_ARRAY=n
> > > > > > + CONFIG_ARGO=n
> > > > > > + CONFIG_HYPFS_CONFIG=n
> > > > > > + CONFIG_CORE_PARKING=n
> > > > > > + CONFIG_DEBUG_TRACE=n
> > > > > > + CONFIG_IOREQ_SERVER=n
> > > > > > + CONFIG_CRASH_DEBUG=n
> > > > > > + CONFIG_KEXEC=n
> > > > > > + CONFIG_LIVEPATCH=n
> > > > > > + CONFIG_MEM_ACCESS=n
> > > > > > + CONFIG_NUMA=n
> > > > > > + CONFIG_PERF_COUNTERS=n
> > > > > > + CONFIG_HAS_PMAP=n
> > > > > > + CONFIG_TRACEBUFFER=n
> > > > > > + CONFIG_XENOPROF=n
> > > > > > + CONFIG_COMPAT=n
> > > > > > + CONFIG_COVERAGE=n
> > > > > > + CONFIG_UBSAN=n
> > > > > > + CONFIG_NEEDS_LIBELF=n
> > > > > > + CONFIG_XSM=n
> > > > > > +
> > > > > >
> > > > > > archlinux-current-gcc-riscv64-debug:
> > > > > > extends: .gcc-riscv64-cross-build-debug
> > > >
> > > > I think I've said so elsewhere before: Please avoid introducing
> > > > double
> > > > blank lines, unless entirely unavoidable (for reasons I cannot
> > > > think
> > > > of).
> > Sorry for that; I am not doing that on purpose. It's just a big
> > patch
> > series, and I missed that double blank. I will try to be more
> > attentive.
> >
> > Do you think it makes sense to add a script to perform basic code
> > style
> > checks, similar to what Linux has?
>
> Such a script would be nice, but it doesn't exist and re-using
> existing
> checkers has also proven controversial. There's actually an ongoing
> discussion on this topic which you may want to follow.
Yes, I would like to follow. I'll search the topic in ML.
Thanks.
~ Oleksii
