Re: [PATCH v3 02/12] x86/HVM: improve CET-IBT pruning of ENDBR

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Feb 26, 2024 at 05:42:20PM +0100, Jan Beulich wrote:
> __init{const,data}_cf_clobber can have an effect only for pointers
> actually populated in the respective tables. While not the case for SVM
> right now, VMX installs a number of pointers only under certain
> conditions. Hence the respective functions would have their ENDBR purged
> only when those conditions are met. Invoke "pruning" functions after
> having copied the respective tables, for them to install any "missing"
> pointers.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> This is largely cosmetic for present hardware, which when supporting
> CET-IBT likely also supports all of the advanced VMX features for which
> hook pointers are installed conditionally. The only case this would make
> a difference there is when use of respective features was suppressed via
> command line option (where available). For future hooks it may end up
> relevant even by default, and it also would be if AMD started supporting
> CET-IBT; right now it matters only for .pi_update_irte, as iommu_intpost
> continues to default to off.
> 
> Originally I had meant to put the SVM and VMX functions in presmp-
> initcalls, but hvm/{svm,vmx}/built_in.o are linked into hvm/built_in.o
> before hvm/hvm.o. And I don't think I want to fiddle with link order
> here.
> ---
> v3: Re-base.
> v2: Use cpu_has_xen_ibt in prune_{svm,vmx}().
> 
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -161,10 +161,15 @@ static int __init cf_check hvm_enable(vo
>      else if ( cpu_has_svm )
>          fns = start_svm();
>  
> +    if ( fns )
> +        hvm_funcs = *fns;
> +
> +    prune_vmx();
> +    prune_svm();

Isn't it actually the opposite of pruning.  What the helpers do is
fill all the pointers in the structure.  I would rather name them
{vmx,svm}_fill_hvm_funcs() or similar.  And possibly pull the
cpu_has_xen_ibt check outside the functions:

if ( cpu_has_xen_ibt )
{
    /*
     * Now that svm_function_table was copied, populate all function pointers
     * which may have been left at NULL, for __initdata_cf_clobber to have as
     * much of an effect as possible.
     */
    vmx_fill_hvm_funcs();
    svm_fill_hvm_funcs();
}

I would be nice to avoid directly exporting more vmx and smv specific
helpers, as if we ever want to compile out vmx or svm it would be more
churn to deal with those.  I however cannot think of any good way to
do this here, so it's fine to export those functions.

Thanks, Roger.