Re: [PATCH v2.1 14/12] xen: Switch to new TRACE() API

[Jan Beulich <jbeulich@suse.com>]

On 20.09.2021 21:32, Andrew Cooper wrote:
> (Almost) no functional change.
> 
> irq_move_cleanup_interrupt() changes two smp_processor_id() calls to the 'me'
> local variable which manifests as a minor code improvement.  All other
> differences in the compiled binary are to do with line numbers changing.
> 
> Some conversion notes:
>  * HVMTRACE_LONG_[234]D() and TRACE_2_LONG_[234]D() were latently buggy.  They
>    blindly discard extra parameters, but luckily no users are impacted.  They
>    are also obfuscated wrappers, depending on exactly one or two parameters
>    being TRC_PAR_LONG() to compile successfully.
>  * HVMTRACE_LONG_1D() behaves unlike its named companions, and takes exactly
>    one 64bit parameter which it splits manually.  It's one user,
>    vmx_cr_access()'s LMSW path, is gets adjusted to use TRACE_PARAM64().
>  * TRACE_?D() and TRACE_2_LONG_*() change to TRACE_TIME() as cycles is always.
>  * HVMTRACE_ND() is opencoded for VMENTRY/VMEXIT records to include cycles.
>    These are converted to TRACE_TIME(), with the old modifier parameter
>    expressed as an OR at the callsite.  One callsite, svm_vmenter_helper() had
>    a nested tb_init_done check, which is dropped.  (The optimiser also spotted
>    this, which is why it doesn't manifest as a binary difference.)
>  * All HVMTRACE_?D() change to TRACE() as cycles is explicitly skipped.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

> I'm in two minds as to whether to split this up by subsystem or not.  It is
> 95% x86, and isn't a massive patch.

Either way looks fine to me in this case; splitting might allow parts
to go in before you've managed to get acks from all relevant people.
If anything I might have preferred seeing e.g. all the HVM*() macros
getting replaced and dropped at the same time, rather than the
dropping (combined with others) getting split off.

Jan