[Public] Hi,
> -----Original Message----- > From: Jan Beulich <jbeul...@suse.com> > Sent: Tuesday, June 10, 2025 9:01 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 v4 03/20] xen/x86: remove "depends > on !PV_SHIM_EXCLUSIVE" > > On 28.05.2025 11:16, 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. To make sure unchanging produced config > > file based on "pvshim_defconfig", we shall explicitly state according > > Kconfig is not set > > > > Add "default y" for SHADOW_PAGING and TBOOT, otherwise we will have > > unset values when running make defconfig based on "x86_64_defconfig". > > I fear I don't understand this, perhaps related to me also not seeing how ... > If we just removed "default !PV_SHIM_EXCLUSIVE", .config file generated by "make defconfig" will change, having unsetting values for SHADOW_PAGING (# CONFIG_SHADOW_PAGING is not set) If changing it to "default y" is too casual, maybe we shall add "CONFIG_ SHADOW_PAGING=y" in x86_64_defconfig, to at least retain consistent .config file. My fault, Only considering "SHADOW_PAGING" is enough, as TBOOT depends on UNSUPPORTED firstly > > --- a/xen/arch/x86/Kconfig > > +++ b/xen/arch/x86/Kconfig > > @@ -143,7 +143,7 @@ config XEN_IBT > > > > config SHADOW_PAGING > > bool "Shadow Paging" > > - default !PV_SHIM_EXCLUSIVE > > + default y > > depends on PV || HVM > > help > > > > @@ -175,7 +175,7 @@ config BIGMEM > > config TBOOT > > bool "Xen tboot support (UNSUPPORTED)" > > depends on INTEL && UNSUPPORTED > > - default !PV_SHIM_EXCLUSIVE > > + default y > > select CRYPTO > > help > > Allows support for Trusted Boot using the Intel(R) Trusted > > Execution > > ... these two fit with title and description. The justification for removing > the !PV_SHIM_EXCLUSIVE here is not "breaks allyesconfig". > Hmmm, it is the consequence of "removing the !PV_SHIM_EXCLUSIVE" Maybe I shall add more explanation in commit message? > > @@ -288,7 +288,6 @@ config PV_SHIM_EXCLUSIVE > > > > If unsure, say N. > > > > -if !PV_SHIM_EXCLUSIVE > > > > config HYPERV_GUEST > > bool "Hyper-V Guest" > > @@ -298,7 +297,6 @@ config HYPERV_GUEST > > > > If unsure, say N. > > > > -endif > > > > config REQUIRE_NX > > bool "Require NX (No eXecute) support" > > Please don't leave double blank lines. > > > --- a/xen/arch/x86/configs/pvshim_defconfig > > +++ b/xen/arch/x86/configs/pvshim_defconfig > > @@ -26,3 +26,8 @@ CONFIG_EXPERT=y > > # CONFIG_INTEL_IOMMU is not set > > # CONFIG_DEBUG is not set > > # CONFIG_GDBSX is not set > > +# CONFIG_SHADOW_PAGING is not set > > +# CONFIG_TBOOT is not set > > +# HYPERV_HYPERV_GUEST is not set > > This one doesn't look right, simply by its name. > > > +# CONFIG_HVM is not set > > +# CONFIG_VGA is not set > > Just to mention it - I'm unsure whether adding such at the end isn't going to > cause > issues. But maybe I'm paranoid ... > It could be too casual.. I will only leave VGA here, as we're sure that with removing "!PV_SHIM_EXCLUSIVE", CONFIG_VGA is setting as y in pvshim_defconfig > > --- a/xen/drivers/video/Kconfig > > +++ b/xen/drivers/video/Kconfig > > @@ -3,10 +3,10 @@ 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 > > + default y > > help > > Enable VGA output for the Xen hypervisor. > > Like above, this change also doesn't really fit with title and description. I have added " (also the functionally equivalent "if !...") " in commit message to also cover above change > > Jan