Re: [Qemu-arm] [PATCH v4] timer: a9gtimer: remove loop to auto-increment comparator

[P J P]

+-- On Wed, 12 Oct 2016, Peter Maydell wrote --+
| > -            while (gtb->compare < update.new) {
| > +            if (gtb->compare < update.new) {
| >
| > +                    inc = update.new - gtb->compare - 1;
| 
| Can you explain why the '- 1' here ?

  I think I did that because while was running till gtb->compare < update.new.
To contain gtb->compare under update.new.

| Something still doesn't look right here. Consider the case where
| update.new is only just bigger than gtb->compare (actually the
| usual case, I think). In this case 'update.new - gtb->compare - 1'
| is smaller than gtb->inc and so the QEMU_ALIGN_DOWN will produce
| an inc value of 0. That would be wrong, because we should definitely
| have done an auto-increment.

   Yes, but in those cases 'gtb->compare += inc' would exceed update.new. Is 
that okay ?

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F