[Public] Hi,
> -----Original Message----- > From: Jan Beulich <jbeul...@suse.com> > Sent: Monday, June 30, 2025 4:21 PM > To: Penny, Zheng <penny.zh...@amd.com> > Cc: Huang, Ray <ray.hu...@amd.com>; Andrew Cooper > <andrew.coop...@citrix.com>; Roger Pau Monné <roger....@citrix.com>; > Anthony PERARD <anthony.per...@vates.tech>; Orzel, Michal > <michal.or...@amd.com>; Julien Grall <jul...@xen.org>; Stefano Stabellini > <sstabell...@kernel.org>; xen-devel@lists.xenproject.org > Subject: Re: [PATCH v5 01/18] xen/x86: remove "depends > on !PV_SHIM_EXCLUSIVE" > > On 16.06.2025 08:41, Penny Zheng wrote: > > Remove all "depends on !PV_SHIM_EXCLUSIVE" (also the functionally > > equivalent "if !...") in Kconfig file, since negative dependancy will > > badly affect allyesconfig. > > > > Signed-off-by: Penny Zheng <penny.zh...@amd.com> > > Reviewed-by: Stefano Stabellini <sstabell...@kernel.org> > > --- > > v2 -> v3: > > - remove comment for PV_SHIM_EXCLUSIVE > > --- > > v3 -> v4: > > - explicitly state "CONFIG_xxx is not set" in "pvshim_defconfig" > > Where did these changes go? Nothing is said about ... > > > - Add "default y" for SHADOW_PAGING and TBOOT > > - refactor commit message > > --- > > v4 -> v5: > > - For not breaking allyesconfig, changes to defaults are actually not > > needed. > > So remove them all > > - Leave one blank lines > > ... their (complete) dropping here. Aiui overrides for anything where you > remove the > dependency (and where the intended setting for the shim is different from the > general default) would still be needed here. > Since I checked, before and after this commit, result of "make defconfig pvshim_defconfig" doesn't really change for above options, so I remove them I'll add them back to emphasize intended setting for the shim is different from the general default > And then there's still a non-"depends on" change left ... > > > --- a/xen/drivers/video/Kconfig > > +++ b/xen/drivers/video/Kconfig > > @@ -3,7 +3,7 @@ config VIDEO > > bool > > > > config VGA > > - bool "VGA support" if !PV_SHIM_EXCLUSIVE > > + bool "VGA support" > > select VIDEO > > depends on X86 > > default y if !PV_SHIM_EXCLUSIVE > > ... here, which (as indicated before) imo doesn't belong here, but at the > very least > would need covering in the description. > Hmmm. Although " bool "VGA support" if !PV_SHIM_EXCLUSIVE " doesn't make CONFIG_VGA the option disappear when PV_SHIM_EXCLUSIVE=y, it still make it unconfigurable. So I treat it dependency too here... Maybe I shall add the following in the description: ``` Although " if !PV_SHIM_EXCLUSIVE " for CONFIG_VGA is not truly a dependency, setting PV_SHIM_EXCLUSIVE y still makes it unconfigurable. So we remove it here too ``` > Also, just to repeat what I said in reply to the cover letter: Imo this > change needs to > move 2nd to last in the series, and it then wants committing together with > the last > patch (which you will want to put in as a remark to the eventual committer). Yes, I'll move it to 2nd to last. Shall I mention "It shall be committed together with ...." in commit message or change log? > > Jan
