On 22/05/2024 2:40 pm, Jan Beulich wrote:
> On 22.05.2024 15:17, Andrew Cooper wrote:
>> --- a/xen/arch/x86/mm/shadow/multi.c
>> +++ b/xen/arch/x86/mm/shadow/multi.c
>> @@ -1974,13 +1974,17 @@ typedef u32 guest_va_t;
>> typedef u32 guest_pa_t;
>> #endif
>>
>> -static inline void trace_shadow_gen(u32 event, guest_va_t va)
>> +/* Shadow trace event with GUEST_PAGING_LEVELS folded into the event field.
>> */
>> +static void sh_trace(uint32_t event, unsigned int extra, const void
>> *extra_data)
>> +{
>> + trace(event | ((GUEST_PAGING_LEVELS - 2) << 8), extra, extra_data);
>> +}
>> +
>> +/* Shadow trace event with the guest's linear address. */
>> +static void sh_trace_va(uint32_t event, guest_va_t va)
>> {
>> if ( tb_init_done )
>> - {
>> - event |= (GUEST_PAGING_LEVELS-2)<<8;
>> - trace(event, sizeof(va), &va);
>> - }
>> + sh_trace(event, sizeof(va), &va);
>> }
> If any tb_init_done check, then perhaps rather in sh_trace()? With that
> (and provided you agree)
> Reviewed-by: Jan Beulich <jbeul...@suse.com>
Sadly not. That leads to double reads of tb_init_done when tracing is
compiled in.
When GCC can't fully inline the structure initialisation, it can't prove
that a function call modified tb_init_done. This is why I arranged all
the trace cleanup in this way.
~Andrew
