On 14/02/2022 13:35, Andrew Cooper wrote: > On 14/02/2022 13:10, Jan Beulich wrote: >> On 14.02.2022 13:56, Andrew Cooper wrote: >>> --- a/xen/arch/x86/hvm/hvm.c >>> +++ b/xen/arch/x86/hvm/hvm.c >>> @@ -88,7 +88,7 @@ unsigned int opt_hvm_debug_level __read_mostly; >>> integer_param("hvm_debug", opt_hvm_debug_level); >>> #endif >>> >>> -struct hvm_function_table hvm_funcs __read_mostly; >>> +struct hvm_function_table __ro_after_init hvm_funcs; >> Strictly speaking this is an unrelated change. I'm fine with it living here, >> but half a sentence would be nice in the description. > I could split it out, but we could probably make 200 patches of > "sprinkle some __ro_after_init around, now that it exists". > >>> --- a/xen/arch/x86/hvm/svm/svm.c >>> +++ b/xen/arch/x86/hvm/svm/svm.c >>> @@ -2513,7 +2513,7 @@ static void cf_check svm_set_reg(struct vcpu *v, >>> unsigned int reg, uint64_t val) >>> } >>> } >>> >>> -static struct hvm_function_table __initdata svm_function_table = { >>> +static struct hvm_function_table __initdata_cf_clobber svm_function_table >>> = { >>> .name = "SVM", >>> .cpu_up_prepare = svm_cpu_up_prepare, >>> .cpu_dead = svm_cpu_dead, >>> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c >>> index 41db538a9e3d..758df3321884 100644 >>> --- a/xen/arch/x86/hvm/vmx/vmx.c >>> +++ b/xen/arch/x86/hvm/vmx/vmx.c >>> @@ -2473,7 +2473,7 @@ static void cf_check vmx_set_reg(struct vcpu *v, >>> unsigned int reg, uint64_t val) >>> vmx_vmcs_exit(v); >>> } >>> >>> -static struct hvm_function_table __initdata vmx_function_table = { >>> +static struct hvm_function_table __initdata_cf_clobber vmx_function_table >>> = { >>> .name = "VMX", >>> .cpu_up_prepare = vmx_cpu_up_prepare, >>> .cpu_dead = vmx_cpu_dead, >> While I'd like to re-raise my concern regarding the non-pointer fields >> in these structure instances (just consider a sequence of enough bool >> bitfields, which effectively can express any value, including ones >> which would appear like pointers into .text), since for now all is okay >> afaict: >> Reviewed-by: Jan Beulich <jbeul...@suse.com> > I should probably put something in the commit message too. It is a > theoretical risk, but not (IMO) a practical one.
Updated commit message: x86/hvm: Use __initdata_cf_clobber for hvm_funcs Now that all calls through hvm_funcs are fully altcall'd, harden all the svm and vmx function pointer targets. This drops 106 endbr64 instructions. Clobbering does come with a theoretical risk. The non-pointer fields of {svm,vmx}_function_table can in theory happen to form a bit pattern matching a pointer into .text at a legal endbr64 instruction, but this is expected to be implausible for anything liable to pass code review. While at it, move hvm_funcs into __ro_after_init now that this exists. ~Andrew