|
[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 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))
HAP_ONLY_BUILD is likely not the best name. Maybe CONFIG_HAP_REQUIRED
or some such? (Seeing the usage below)
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
"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.
> endif
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index da56944e74..ce58632b02 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -91,9 +91,11 @@ struct hvm_function_table __ro_after_init hvm_funcs;
> unsigned long __section(".bss.page_aligned") __aligned(PAGE_SIZE)
> hvm_io_bitmap[HVM_IOBITMAP_SIZE / BYTES_PER_LONG];
>
> +#ifdef CONFIG_OPT_HAP
> /* Xen command-line option to enable HAP */
> static bool __initdata opt_hap_enabled = true;
> boolean_param("hap", opt_hap_enabled);
> +#endif /* CONFIG_OPT_HAP */
Hm, if you nuke the option like that, it needs to be reflected in
xen-command-line.pandoc document.
>
> #ifndef opt_hvm_fep
> /* Permit use of the Forced Emulation Prefix in HVM guests */
> @@ -144,15 +146,21 @@ static bool __init hap_supported(struct
> hvm_function_table *fns)
> if ( !fns->caps.hap )
> {
> printk("HVM: Hardware Assisted Paging (HAP) not detected\n");
> +
> + if ( !IS_ENABLED(CONFIG_OPT_HAP) )
> + panic("HAP is compile-time mandatory\n");
>From a user perspective, it's a weird error message IMO. I would
rather say:
"HVM: Hardware Assisted Paging (HAP) is mandatory but not detected\n".
Not fully convinced about that wording, but I would certainly drop the
"compile-time" part of yours. A user is not likely to care/know about
compile-time subtlety of the error message.
> +
> 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?
> {
> 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)) += ...
Thanks, Roger.
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |