[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 13:53 +0100, Jan Beulich wrote: > On 28.11.2023 12:49, Oleksii wrote: > > On Tue, 2023-11-28 at 10:58 +0100, Jan Beulich wrote: > > > On 28.11.2023 10:28, Oleksii wrote: > > > > 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. > > > > > > Right. But if it's indeed temporary, what's the point of this > > > patch? > > > The entire series is (supposed to be) to improve code structure > > > for > > > longer term purposes. If a non-generic grant_table.h is going to > > > appear for PPC and RISC-V, I don't see why the present dummy > > > would > > > need removing. That's only useful if an arch can be expected to > > > live > > > with GRANT_TABLE=n even when otherwise feature-complete, and at > > > that > > > point modifying the Kconfig dependencies would (imo) be > > > appropriate. > > I agree. Let's proceed by adding the dependency in the KConfig > > file. > > > > So which one option will be better: > > 1. depends on !PPC && !RISCV > > 2. depends on ARM || X86 > > Agreeing and then making this suggestion contradicts one another. > Unless > the long-term plan is for PPC and RISC-V to not use grant tables. On RISC-V side, I can run guests in dom0less and still don't use grant_tables. And I am not sure when I'll start to implement it. Only one thing I defined in grant_table.h is: #define gnttab_dom0_frames() \ min_t(unsigned int, opt_max_grant_frames, PFN_DOWN(_etext - _stext)) But I think it can moved somewhere or dropped as it was defined because of: void __init create_dom0(void) { struct domain *dom0; struct xen_domctl_createdomain dom0_cfg = { .flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap, .max_evtchn_port = -1, .max_grant_frames = gnttab_dom0_frames(), .max_maptrack_frames = -1, .grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version), }; ... Which was copied from Arm. Taking into account that opt_max_grant_frames is 0 in case when CONFIG_GRANT_TABLE=n then ".max_grant_frames =" will be 0 in case of !CONFIG_GRANT_TABLE so for time being the macros gnttab_dom0_frames can be dropped until CONFIG_GRANT_TABLE will be introduced. But I am not aware of PPC plans of usage this config. ~ Oleksii
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |