Ping: [PATCH] x86: replace a few do_div() uses

[Jan Beulich <jbeulich@xxxxxxxx>]

On 12.01.2022 10:28, Jan Beulich wrote:
> On 12.01.2022 10:22, Andrew Cooper wrote:
>> On 12/01/2022 09:00, Jan Beulich wrote:
>>> When the macro's "return value" is not used, the macro use can be
>>> replaced by a simply division, avoiding some obfuscation.
>>>
>>> According to my observations, no change to generated code.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> I like this change in principle, but see below.
>>
>> do_div() needs to be deleted, because it's far too easy screw up.  At a
>> bare minimum, it should be replaced with a static inline that takes it's
>> first parameter by pointer, because then at least every callsite reads
>> correctly in terms of the C language.
> 
> That ought to be a 2nd step, requiring agreement with Arm folks (and
> adjustments to their code).
> 
>>> --- a/xen/arch/x86/time.c
>>> +++ b/xen/arch/x86/time.c
>>> @@ -610,8 +610,7 @@ static uint64_t xen_timer_cpu_frequency(
>>>      struct vcpu_time_info *info = &this_cpu(vcpu_info)->time;
>>>      uint64_t freq;
>>>  
>>> -    freq = 1000000000ULL << 32;
>>> -    do_div(freq, info->tsc_to_system_mul);
>>> +    freq = (1000000000ULL << 32) / info->tsc_to_system_mul;
>>>      if ( info->tsc_shift < 0 )
>>>          freq <<= -info->tsc_shift;
>>
>> do_div()'s output is consumed here.  I don't think this hunk is safe to
>> convert.
> 
> If by "output" you mean its "return value", then it clearly isn't
> consumed. And I continue to think that I did express correctly the
> effect do_div() did have on "freq".

I think I did address both points (the earlier one was actually more a
remark imo anyway, not a request to change anything right in this patch),
so may I please ask for an ack (or a response clarifying what I'm not
understanding in what you have said)?

Thanks, Jan