|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 2/2] x86: Force HAP to be enabled when PV and shadow paging are compiled out
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@xxxxxxx>
> >> ---
> >> 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.
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |