Re: [PATCH] x86/hap: Inline "flush_vcpu" in "flush_tlb"

["Teddy Astie" <teddy.astie@xxxxxxxxxx>]

Le 29/09/2025 à 16:09, Roger Pau Monné a écrit :
> On Mon, Sep 29, 2025 at 12:36:30PM +0000, Teddy Astie wrote:
>> flush_vcpu static function here is only used in one place which is just below
>> where it is defined. Inline the function to reduce the noise and clarify
>> what we are doing.
>
> Did you check that the compiler doesn't inline it already?
>
> It seems like an obvious optimization for the compiler to do.
>

I assume that the compiler already does it, it's mostly meant to be a
cosmetic change.

>> No functional change.
>>
>> Signed-off-by: Teddy Astie <teddy.astie@xxxxxxxxxx>
>> ---
>>   xen/arch/x86/mm/hap/hap.c | 7 +------
>>   1 file changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
>> index 2f69ff9c7b..407c80afab 100644
>> --- a/xen/arch/x86/mm/hap/hap.c
>> +++ b/xen/arch/x86/mm/hap/hap.c
>> @@ -721,11 +721,6 @@ static pagetable_t cf_check hap_update_cr3(struct vcpu 
>> *v, bool noflush)
>>       return pagetable_null();
>>   }
>>
>> -static bool flush_vcpu(const struct vcpu *v, const unsigned long 
>> *vcpu_bitmap)
>> -{
>> -    return !vcpu_bitmap || test_bit(v->vcpu_id, vcpu_bitmap);
>
> The same construct is used in shadow code also, maybe it would be
> helpful to place the flush_vcpu() helper in a common header as static
> inline?
>

maybe, but given the simplicity of the function, it does feel a bit
excessive it for hap code.

> OTOH we don't care much for shadow, so it might be simpler to leave
> shadow as-is and do the change just for HAP, but would be good to
> mention in the commit message why shadow is not adjusted in the same
> way.

Something like

"We only do this for hap where this function is only used once."

?

>
> Thanks, Roger.
>

Teddy


--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech