Re: [PATCH 2/2] x86: Force HAP to be enabled when PV and shadow paging are compiled out

["Roger Pau Monné" <roger.pau@citrix.com>]

On Fri, Feb 13, 2026 at 06:30:54PM +0100, Alejandro Vallejo wrote:
> On Fri Feb 13, 2026 at 4:09 PM CET, Roger Pau Monné wrote:
> > On Fri, Feb 13, 2026 at 02:37:30PM +0100, Alejandro Vallejo wrote:
> >> Makes hap_enabled() a compile-time constant. This removes a number
> >> of hooks that normally go reach onto shadow paging code, clears
> >> many branches in a number of places and generally improves codegen
> >> throughout.
> >> 
> >> Take the chance to fully remove the shadow/ folder as it's now fully
> >> compiled out.
> >> 
> >> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
> >> ---
> >> bloat-o-meter against prior commit (defconfig:-pv,-shadow on both):
> >> 
> >>   add/remove: 0/12 grow/shrink: 2/31 up/down: 67/-1609 (-1542)
> >>   Function                                     old     new   delta
> >>   unmap_mmio_regions                          1340    1374     +34
> >>   map_mmio_regions                             211     244     +33
> >>   opt_hap_enabled                                1       -      -1
> >>   shadow_vcpu_init                               2       -      -2
> >>   __setup_str_opt_hap_enabled                    4       -      -4
> >>   _update_paging_modes                           6       -      -6
> >>   _toggle_log_dirty                              6       -      -6
> >>   _clean_dirty_bitmap                            6       -      -6
> >>   cpuid_viridian_leaves                        728     714     -14
> >>   iommu_domain_init                            291     276     -15
> >>   p2m_pt_change_entry_type_global              214     198     -16
> >>   paging_teardown                               91      74     -17
> >>   paging_set_allocation                        384     367     -17
> >>   paging_enable                                 76      59     -17
> >>   p2m_init_one                                 295     278     -17
> >>   ept_sync_domain                              201     184     -17
> >>   arch_set_paging_mempool_size                 437     420     -17
> >>   p2m_free_one                                  78      59     -19
> >>   paging_vcpu_teardown                          36      15     -21
> >>   p2m_pt_init                                  125     104     -21
> >>   p2m_pt_change_entry_type_range               218     197     -21
> >>   arch_do_physinfo                              76      53     -23
> >>   sh_none_ops                                   24       -     -24
> >>   paging_final_teardown                        134     110     -24
> >>   __setup_opt_hap_enabled                       24       -     -24
> >>   paging_vcpu_init                              41      15     -26
> >>   paging_domain_init                           167     141     -26
> >>   p2m_mem_access_sanity_check                   71      42     -29
> >>   hvm_enable                                   449     419     -30
> >>   init_guest_cpu_policies                     1247    1213     -34
> >>   paging_domctl                               3357    3318     -39
> >>   __start_xen                                 9456    9416     -40
> >>   arch_sanitise_domain_config                  794     747     -47
> >>   symbols_offsets                            29636   29588     -48
> >>   p2m_set_entry                                305     247     -58
> >>   guest_cpuid                                 1919    1858     -61
> >>   ept_dump_p2m_table                           817     751     -66
> >>   recalculate_cpuid_policy                     874     806     -68
> >>   shadow_domain_init                            71       -     -71
> >>   mmio_order                                    73       -     -73
> >>   hvm_shadow_max_featuremask                    76       -     -76
> >>   hvm_shadow_def_featuremask                    76       -     -76
> >>   dm_op                                       3594    3510     -84
> >>   symbols_sorted_offsets                     58464   58368     -96
> >>   symbols_names                             103425  103213    -212
> >>   Total: Before=3644618, After=3643076, chg -0.04%
> >> ---
> >>  xen/arch/x86/Kconfig               | 2 ++
> >>  xen/arch/x86/hvm/Kconfig           | 3 +++
> >>  xen/arch/x86/hvm/hvm.c             | 8 ++++++++
> >>  xen/arch/x86/include/asm/hvm/hvm.h | 2 +-
> >>  xen/arch/x86/mm/Makefile           | 2 +-
> >>  xen/include/xen/sched.h            | 3 +++
> >>  6 files changed, 18 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> >> index 2ce4747f6e..190f419720 100644
> >> --- a/xen/arch/x86/Kconfig
> >> +++ b/xen/arch/x86/Kconfig
> >> @@ -67,6 +67,7 @@ source "arch/Kconfig"
> >>  config PV
> >>  	def_bool y
> >>  	prompt "PV support"
> >> +	select OPT_HAP
> >>  	help
> >>  	  Interfaces to support PV domains. These require guest kernel support
> >>  	  to run as a PV guest, but don't require any specific hardware support.
> >> @@ -147,6 +148,7 @@ config SHADOW_PAGING
> >>  	bool "Shadow Paging"
> >>  	default !PV_SHIM_EXCLUSIVE
> >>  	depends on PV || HVM
> >> +	select OPT_HAP
> >>  	help
> >>  	  Shadow paging is a software alternative to hardware paging support
> >>  	  (Intel EPT, AMD NPT).
> >> diff --git a/xen/arch/x86/hvm/Kconfig b/xen/arch/x86/hvm/Kconfig
> >> index f32bf5cbb7..310e09847b 100644
> >> --- a/xen/arch/x86/hvm/Kconfig
> >> +++ b/xen/arch/x86/hvm/Kconfig
> >> @@ -92,4 +92,7 @@ config MEM_SHARING
> >>  	bool "Xen memory sharing support (UNSUPPORTED)" if UNSUPPORTED
> >>  	depends on INTEL_VMX
> >>  
> >> +config OPT_HAP
> >> +	bool
> >
> > Can't you define this outside of Kconfig, like:
> >
> > #define HAP_ONLY_BUILD (!IS_ENABLED(CONFIG_PV) && !IS_ENABLED(CONFIG_SHADOW_PAGING))
> 
> Sure.
> 
> >
> > HAP_ONLY_BUILD is likely not the best name.  Maybe CONFIG_HAP_REQUIRED
> > or some such?  (Seeing the usage below)
> 
> Definitely not CONFIG_*, or it'd be an accident about to happen when mistakenly
> used on IS_ENABLED(). HAP_EXCLUSIVE?

I would prefer HAP_REQUIRED, but that's a question of taste.  Both
would be used in the same way.

We have some CONFIG_ defines that won't work with IS_ENABLED()
already, but let's not propagate that further.

> >
> > FWIW, with the current naming I've assume this was supposed to mean
> > "Option HAP" or some such, when is "HAP is Optional".  We usually use
> 
> It was. Originally it had a help message and a default, but I quickly noticed
> that served no purpose. It has that weird polarity so the build would remain
> with new options being additive only.
> 
> In retrospect it can go back to a more natural HAP_EXCLUSIVE that removes
> a bunch of !s in the code.
> 
> > "opt" as a shortcut for "option" in several places on the Xen code
> > base, like "opt_hap_enabled".  I also think using it in the positive
> > for so the variable meaning "required" instead of "optional" makes
> > some of the logic easier to follow below.
> 
> It does, but in Kconfig it's nicer if an option being enabled yields a strictly
> more capable hypervisor, I think. Makes allyesconfig and allnoconfig work as
> intended.

Oh, I see.  Moving it out of Kconfig makes even more sense I think.

> >> +
> >>          return false;
> >>      }
> >>  
> >> +#ifdef CONFIG_OPT_HAP
> >>      if ( !opt_hap_enabled )
> >
> > You could possibly do:
> >
> > #ifdef CONFIG_OPT_HAP
> > /* Xen command-line option to enable HAP */
> > static bool __initdata opt_hap_enabled = true;
> > boolean_param("hap", opt_hap_enabled);
> > #else /* CONFIG_OPT_HAP */
> > # define opt_hap_enabled true
> > #endif /* !CONFIG_OPT_HAP */
> >
> > Above, and avoid the ifdefs here?
> 
> Whatever poison you prefer. It's just ugliness motion.

Yeah, at least this keeps all the ifdefs mostly in the same visual
region.

> >
> >>      {
> >>          fns->caps.hap = false;
> >>          printk("HVM: Hardware Assisted Paging (HAP) detected but disabled\n");
> >>          return false;
> >>      }
> >> +#endif /* CONFIG_OPT_HAP */
> >>  
> >>      return true;
> >>  }
> >> diff --git a/xen/arch/x86/include/asm/hvm/hvm.h b/xen/arch/x86/include/asm/hvm/hvm.h
> >> index dc609bf4cb..b330d65d6d 100644
> >> --- a/xen/arch/x86/include/asm/hvm/hvm.h
> >> +++ b/xen/arch/x86/include/asm/hvm/hvm.h
> >> @@ -703,7 +703,7 @@ static inline bool hvm_is_singlestep_supported(void)
> >>  
> >>  static inline bool hvm_hap_supported(void)
> >>  {
> >> -    return hvm_funcs.caps.hap;
> >> +    return !IS_ENABLED(CONFIG_OPT_HAP) ?: hvm_funcs.caps.hap;
> >
> > return CONFIG_HAP_REQUIRED ?: hvm_funcs.caps.hap;
> >
> > IMO is easier to read (same below for the hap_enabled() early return).
> >
> >>  }
> >>  
> >>  /* returns true if hardware supports alternate p2m's */
> >> diff --git a/xen/arch/x86/mm/Makefile b/xen/arch/x86/mm/Makefile
> >> index 960f6e8409..64fde82c50 100644
> >> --- a/xen/arch/x86/mm/Makefile
> >> +++ b/xen/arch/x86/mm/Makefile
> >> @@ -1,4 +1,4 @@
> >> -obj-y += shadow/
> >> +obj-$(CONFIG_OPT_HAP) += shadow/
> >
> > I think you can use:
> >
> > obj-$(findstring y,$(CONFIG_PV) $(CONFIG_SHADOW_PAGING)) += ...
> 
> Hmmm. I guess I shouldn't just include it twice, like we do for other .o files.

I see, so use:

obj-$(CONFIG_PV) += shadow/
obj-$(CONFIG_SHADOW_PAGING) += shadow/

Maybe that's simpler really, and it's a pattern we already use
elsewhere.

Thanks, Roger.