Re: [Qemu-devel] [PATCH 15/15] qtest: add rtc-test test-case

[Anthony Liguori <anthony@codemonkey.ws>]

On 01/10/2012 01:59 PM, Paolo Bonzini wrote:
> On 01/10/2012 08:10 PM, Anthony Liguori wrote:
>> + sec = cmos_read(0x00);
>> + min = cmos_read(0x02);
>> + hour = cmos_read(0x04);
>> + mday = cmos_read(0x07);
>> + mon = cmos_read(0x08);
>> + year = cmos_read(0x09);
>
> Please use identifiers for register numbers.
>
>> + /*
>> + * This check assumes a few things. First, we cannot guarantee that we get
>> + * a consistent reading from the wall clock because we may hit an edge of
>> + * the clock while reading. To work around this, we read four clock readings
>> + * such that at least two of them should match. We need to assume that one
>> + * reading is corrupt so we need four readings to ensure that we have at
>> + * least two consecutive identical readings
>> + *
>> + * It's also possible that we'll cross an edge reading the host clock so
>> + * simply check to make sure that the clock reading is within the period of
>> + * when we expect it to be.
>> + */
>
> This seems broken to me.

It's not broken, although it may be ugly.

> The right thing to do would be to run the test with
> vm_clock for the rtc_clock, add a way for the qtest machine to bump the vm_clock
> to the next event,

I actually was looking at this yesterday.  Just bumping to the next event is not 
enough, you want to be able to control how time progresses.  I was thinking of 
adding another qtest_clock and allowing the rtc to use the qtest_clock.

What's nice about that is that you can simulate long periods of time (2 years) 
in a short period of time and do long term drift testing.

I know the math in the rtc is broken right now because we assume that there's a 
rational conversion from RTC cycle frequency to nanoseconds which is not the 
case.  We need a more sophisticated approach that can maintain an irrational 
conversion factor (in the form of a fractional multiplier).

Regards,

Anthony Liguori

> and busy loop running that method using UIP like you would do
> on hardware.
>
> Paolo
>
>