[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v4 13/14] xen: ifdef inclusion of <asm/grant_table.h> in <xen/grant_table.h>
On Tue, 2023-11-28 at 08:57 +0100, Jan Beulich wrote: > On 27.11.2023 20:38, Oleksii wrote: > > On Mon, 2023-11-27 at 15:41 +0100, Jan Beulich wrote: > > > On 27.11.2023 15:13, Oleksii Kurochko wrote: > > > > --- a/xen/arch/ppc/include/asm/grant_table.h > > > > +++ /dev/null > > > > @@ -1,5 +0,0 @@ > > > > -/* SPDX-License-Identifier: GPL-2.0-only */ > > > > -#ifndef __ASM_PPC_GRANT_TABLE_H__ > > > > -#define __ASM_PPC_GRANT_TABLE_H__ > > > > - > > > > -#endif /* __ASM_PPC_GRANT_TABLE_H__ */ > > > > > > Removing this header would be correct only if GRANT_TABLE had a > > > "depends on > > > !PPC", I'm afraid. Recall that the earlier randconfig adjustment > > > in > > > CI was > > > actually requested to be undone, at which point what an arch's > > > defconfig > > > says isn't necessarily what a randconfig should use. > > We can do depends on !PPC && !RISCV but shouldn't it be enough only > > to > > turn CONFIG_GRANT_TABLE off in defconfig and set > > CONFIG_GRANT_TABLE=n > > in EXTRA_XEN_CONFIG? > > That _might_ be sufficient for CI, but we shouldn't take CI as the > only > thing in the world. Personally I consider any kind of overriding for, > in particular, randconfig at bets bogus. Whatever may not be selected > (or must be selected) should be arranged for by Kconfig files > themselves. > That said, there may be _rare_ exceptions. But what PPC's and RISC- > V's > defconfig-s have right now is more than "rare" imo. > > > Some time ago I also tried to redefine "Config GRANT_TABLE" in > > arch- > > specific Kconfig + defconfig + EXTRA_XEN_CONFIG and it works for > > me. > > Could it be solution instead of "depends on..." ? > > Why would we want to re-define an setting? There wants to be one > single > place where a common option is defined. Or maybe I don't understand > what you're suggesting ... I just thought this change is temporary because grant_table.h will be introduced later or earlier, and it will be needed to remove "depends on !PPC && !RISCV". And not to litter common KConfig with the change which will be removed, it will be better to redefine it in arch- specific Kconfig with default n. But after your words about one place, I realized that it would be better to update a place where a common option is defined. The only thing I would like to change is probably it will be better to do the opposite, add "depends on" arches that support CONFIG_GRANT_TABLE now so it will not need to update "depends on" for new arches or arches that don't support CONFIG_GRANT_TABLE. > > > One more question I have do we really need this randconfig? On > > RISC-V > > side, I launched several time this patch series ( from v1 to v4 + > > runs > > during test of patch series ) and I haven't faced case > > when CONFIG_GRANT_TABLE=n. ( but I turned the config off in > > defconfig + > > EXTRA_XEN_CONFIG ). > > That override is why in CI you wouldn't see an issue. But as said - > CI > isn't everything. >From this point of view it will be better to add "depends on !PPC && !RISCV" to "Config GRANT_TABLE". ~ Oleksii
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |