Hi Jiamei,
On 30/06/2022 02:53, Jiamei Xie wrote:
virt_vtimer_save is calculating the new time for the vtimer in:
"v->arch.virt_timer.cval + v->domain->arch.virt_timer_base.offset
- boot_count".
In this formula, "cval + offset" might cause uint64_t overflow.
Changing it to "ticks_to_ns(v->domain->arch.virt_timer_base.offset -
boot_count) + ticks_to_ns(v->arch.virt_timer.cval)" can avoid overflow,
From the commit message, I can't quite make the connection between
"cval + offset" will overflow and "ticks_to_ns(...) + ticks_to_ns(...)"
will help.
So how about the following:
"
virt_vtimer_save() will calculate the next deadline when the vCPU is
scheduled out. At the moment, Xen will use the following equation:
virt_timer.cval + virt_time_base.offset - boot_count
The three values are 64-bit and one (cval) is controlled by domain. In
theory, it would be possible that the domain has started a long time
after the system boot. So virt_time_base.offset - boot_count may be a
large numbers.
This means a domain may inadvertently set a cval so the result would
overflow. Consequently, the deadline would be set very far in the
future. This could result to loss of timer interrupts or the vCPU
getting block "forever".
One way to solve the problem, would be to separately
1) compute when the domain was created in ns
2) convert cval to ns
3) Add 1 and 2 together
The first part of the equation never change (the value is set/known at
domain creation). So take the opportunity to store it in domain structure.
"
The code itself looks good to me.
Cheers,
--
Julien Grall
