Re: [PATCH v2 04/12] x86/hvm: Reduce stack usage from HVMTRACE_ND()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.09.2021 17:38, Andrew Cooper wrote:
> On 21/09/2021 12:00, Jan Beulich wrote:
>> On 20.09.2021 19:25, Andrew Cooper wrote:
>>> v2:
>>>  * Adjust callers of HVMTRACE_ND() too
>> What does this refer to? The sole difference to v1 that I can spot
>> is ...
> 
> Oh - its me who was confused.
> 
> I thought I had failed to include the changes in vmx.c/svm.c in v1.  In
> which case, no change to that in v2.

Good:
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

>>>  * Drop _d[] for the 0 case.
>> ... the one corresponding to this line, i.e. ...
>>
>>> --- a/xen/include/asm-x86/hvm/trace.h
>>> +++ b/xen/include/asm-x86/hvm/trace.h
>>> @@ -67,38 +67,30 @@
>>>  #define TRACE_2_LONG_4D(_e, d1, d2, d3, d4, ...) \
>>>      TRACE_6D(_e, d1, d2, d3, d4)
>>>  
>>> -#define HVMTRACE_ND(evt, modifier, cycles, count, d1, d2, d3, d4, d5, d6) \
>>> +#define HVMTRACE_ND(evt, modifier, cycles, ...)                           \
>>>      do {                                                                  \
>>>          if ( unlikely(tb_init_done) && DO_TRC_HVM_ ## evt )               \
>>>          {                                                                 \
>>> -            struct {                                                      \
>>> -                u32 d[6];                                                 \
>>> -            } _d;                                                         \
>>> -            _d.d[0]=(d1);                                                 \
>>> -            _d.d[1]=(d2);                                                 \
>>> -            _d.d[2]=(d3);                                                 \
>>> -            _d.d[3]=(d4);                                                 \
>>> -            _d.d[4]=(d5);                                                 \
>>> -            _d.d[5]=(d6);                                                 \
>>> +            uint32_t _d[] = { __VA_ARGS__ };                              \
>>>              __trace_var(TRC_HVM_ ## evt | (modifier), cycles,             \
>>> -                        sizeof(*_d.d) * count, &_d);                      \
>>> +                        sizeof(_d), sizeof(_d) ? _d : NULL);              \
>> ... the addition of a conditional operator here (which I assume was
>> something a particular compiler didn't like in v1).
> 
> And was covered in the commit message:
> 
>> The 0 case needs a little help.  All object in C must have a unique address
>> and _d is passed by pointer.  Explicitly permit the optimiser to drop the
>> array.

Right, I had associated text and change this way. It was really just
the revision log which was confusing.

Jan