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
