Re: [PATCH v2 2/8] xen/x86: Simplify header dependencies in x86/hvm

[Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>]

On 22/05/2024 10:33, Jan Beulich wrote:
> On 08.05.2024 14:39, Alejandro Vallejo wrote:
>> Otherwise it's not possible to call functions described in hvm/vlapic.h from 
>> the
>> inline functions of hvm/hvm.h.
>>
>> This is because a static inline in vlapic.h depends on hvm.h, and pulls it
>> transitively through vpt.h. The ultimate cause is having hvm.h included in 
>> any
>> of the "v*.h" headers, so break the cycle moving the guilty inline into 
>> hvm.h.
>>
>> No functional change.
>>
>> Signed-off-by: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>
> 
> In principle:
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> But see below for one possible adjustment.
> 
>> ---
>> v2:
>>   * New patch. Prereq to moving vlapic_cpu_policy_changed() onto hvm.h
> 
> That hook invocation living outside of hvm/hvm.h was an outlier anyway,
> so even without the planned further work this is probably a good move.
> 
>> --- a/xen/arch/x86/include/asm/hvm/hvm.h
>> +++ b/xen/arch/x86/include/asm/hvm/hvm.h
>> @@ -798,6 +798,12 @@ static inline void hvm_update_vlapic_mode(struct vcpu 
>> *v)
>>          alternative_vcall(hvm_funcs.update_vlapic_mode, v);
>>  }
>>  
>> +static inline void hvm_vlapic_sync_pir_to_irr(struct vcpu *v)
>> +{
>> +    if ( hvm_funcs.sync_pir_to_irr )
>> +        alternative_vcall(hvm_funcs.sync_pir_to_irr, v);
>> +}
> 
> The hook doesn't have "vlapic" in its name. Therefore instead or prepending
> hvm_ to the original name or the wrapper, how about replacing the vlapic_
> that was there. That would then also fit better with the naming scheme used
> for other hooks and their wrappers. Happy to adjust while committing, so
> long as you don't disagree.
> 
> Jan

Sounds reasonable. I wasn't sure whether vlapic was adding anything more
than a namespace prefix to the function name. Are you happy to adjust
that on commit?

If so, I'm good with it in the form you propose.

Cheers,
Alejandro