|
[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 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?
>
> 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.
>
>> 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.
Ack.
>
>>
>> #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.
Sure.
>
>> +
>> 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.
>
>> {
>> 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.
Cheers,
Alejandro
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |