On 22.11.2023 13:01, Roger Pau Monné wrote:
> On Wed, Nov 22, 2023 at 11:42:16AM +0100, Jan Beulich wrote:
>> On 22.11.2023 11:08, Roger Pau Monné wrote:
>>> On Thu, Nov 16, 2023 at 02:33:14PM +0100, Jan Beulich wrote:
>>>> --- a/xen/arch/x86/hvm/svm/svm.c
>>>> +++ b/xen/arch/x86/hvm/svm/svm.c
>>>> @@ -2587,6 +2587,19 @@ const struct hvm_function_table * __init
>>>> return &svm_function_table;
>>>> }
>>>>
>>>> +void __init prune_svm(void)
>>>> +{
>>>> + /*
>>>> + * 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.
>>>> + */
>>>> + if ( !IS_ENABLED(CONFIG_XEN_IBT) )
>>>
>>> Shouldn't this better use cpu_has_xen_ibt?
>>>
>>> Otherwise the clobbering done in _apply_alternatives() won't be
>>> engaged, so it's pointless to set the extra fields.
>>
>> That's better answered in the context of ...
>>
>>>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>>>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>>>> @@ -3032,6 +3032,30 @@ const struct hvm_function_table * __init
>>>> return &vmx_function_table;
>>>> }
>>>>
>>>> +void __init prune_vmx(void)
>>>> +{
>>>> + /*
>>>> + * Now that vmx_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.
>>>> + */
>>>> + if ( !IS_ENABLED(CONFIG_XEN_IBT) )
>>>> + return;
>>>> +
>>>> + vmx_function_table.set_descriptor_access_exiting =
>>>> + vmx_set_descriptor_access_exiting;
>>>> +
>>>> + vmx_function_table.update_eoi_exit_bitmap =
>>>> vmx_update_eoi_exit_bitmap;
>>>> + vmx_function_table.process_isr = vmx_process_isr;
>>>> + vmx_function_table.handle_eoi = vmx_handle_eoi;
>>>> +
>>>> + vmx_function_table.pi_update_irte = vmx_pi_update_irte;
>>>> +
>>>> + vmx_function_table.deliver_posted_intr = vmx_deliver_posted_intr;
>>>> + vmx_function_table.sync_pir_to_irr = vmx_sync_pir_to_irr;
>>>> + vmx_function_table.test_pir = vmx_test_pir;
>>
>> ... this: The goal of having a compile time conditional was to have the
>> compiler eliminate the code when not needed. Otherwise there's no real
>> reason to have a conditional there in the first place - we can as well
>> always install all these pointers.
>
> Maybe do:
>
> if ( !IS_ENABLED(CONFIG_XEN_IBT) || !cpu_has_xen_ibt )
>
> then?
Maybe. Yet then perhaps cpu_has_xen_ibt might better include the build-time
check already?
Jan
