Re: [Qemu-devel] [Bug 1080086] Re: MC146818 RTC breaks when SET bit in Register B is on.

[Alex Horn]

>> Out of curiosity, does anyone know how long this particular bug has
>> been undetected or how/when it was introduced?
>
> Probably it was introduced last September when the model was rewritten.
>  But it's really unlikely that the bug would have been detected.

Thanks for that quick assessment.

> On the other hand, the bug in commit b6db4ac (which also includes a
> qtest) was detected by autotest.  Could your tool find it, too?

That's a very respectable achievement. I understand that Autotest runs
predefined (i.e. hand-written) tests on a large scale. In contrast, we
seek a higher degree of automation but on a smaller scale. These
differences could make a comparison difficult. However, your example
is much appreciated because it helps us to set our work in
perspective.

>> This could help me explain to others my research interest in symbolic
>> execution of hardware models and its application in form of automated
>> test generation.
>
> Very interesting (at least to me :)).

Perhaps you find the following background helpful. To kick start our
work, we extracted from QEMU a standalone RTC hardware model [2]
because QOM, QMP, TCG or QTest would render the semi-automatic
analysis infeasible. As a next step, we would like to combine this
standalone hardware model with a Linux x86 driver [3]. The reported
bug is a good exemplar of the type of firmware/hardware interface
properties we are interested in. Note that this is only the initial
phase of our research and there is much work yet to be done.

Of course, any comments or collaboration are always welcome.

With kind regards,
Alex

[2] https://github.com/ahorn/benchmarks/tree/master/qemu-hw
[3] https://github.com/ahorn/benchmarks/tree/master/sw-hw/linux/rtc_x86

On 19 November 2012 11:42, Paolo Bonzini <address@hidden> wrote:
> Il 19/11/2012 12:34, Alex Horn ha scritto:
>>> [...] the patch is almost good for inclusion. I'd ask for two changes:
>>> 1) please test == 0, not != REG_B_SET;
>>> 2) please leave the fuzzicsng test last
>>
>> I have attached a new patch with the requested changes.
>>
>> This patch also improves the quality of the functional test by
>> checking that RTC_SECONDS is equal (==) to the previously written data
>> provided the SET flag in Register B is still enabled. This is
>> justified by the data sheet which states that an enabled SET bit
>> "stops an existing update" and prevents "a new one from occurring" [1,
>> p. 15]. In contrast, once the SET flag is disabled, the RTC_SECONDS
>> check uses an inequality (>=) as in the original test case.
>
> Right.
>
>> Out of curiosity, does anyone know how long this particular bug has
>> been undetected or how/when it was introduced?
>
> Probably it was introduced last September when the model was rewritten.
>  But it's really unlikely that the bug would have been detected.
>
> On the other hand, the bug in commit b6db4ac (which also includes a
> qtest) was detected by autotest.  Could your tool find it, too?
>
>> This could help me explain to others my research interest in symbolic
>> execution of hardware models and its application in form of automated
>> test generation.
>
> Very interesting (at least to me :)).
>
>> Finally, if there is interest to improve the robustness of the RTC
>> model, I could send a patch with several verification conditions (i.e.
>> assertions) which can help to expose these kind of bugs in the RTC
>> hardware model.
>
> Sure, that's welcome.
>
> In particular, I assume you verified the "next alarm" code to be correct? :)
>
> Paolo
>
>> Recall that most compiler can usually optimize these
>> assertions away unless a developer explicitly enables them. They also
>> serve as unambiguous code documentation.
>>
>> With best regards,
>> Alex
>>
>> [1] 
>> http://www.freescale.com/files/microcontrollers/doc/data_sheet/MC146818.pdf
>>
>> On 18 November 2012 08:52, Paolo Bonzini <address@hidden> wrote:
>>> Il 17/11/2012 19:47, Alex Horn ha scritto:
>>>> I have attached a patch for the most recent version of the file
>>>> hw/mc146818rtc.c [1]. The patch also features a functional test which
>>>> executes through the QTest framework.
>>>>
>>>> I would appreciate your thoughts on this.
>>>>
>>>> [1]
>>>> http://git.qemu.org/?p=qemu.git;a=blob;f=hw/mc146818rtc.c;h=98839f278d93452d071054e2a017b3d909b45ab2;hb=9cb535fe4ef08b01e583ec955767a0899ff79afe#l563
>>>>
>>>> ** Patch added: "register_b_set_flag.patch"
>>>>    
>>>> https://bugs.launchpad.net/qemu/+bug/1080086/+attachment/3436808/+files/register_b_set_flag.patch
>>>>
>>>
>>> Hi Alex, the patch is almost good for inclusion.  I'd ask for two
>>> changes: 1) please test == 0, not != REG_B_SET; 2) please leave the
>>> fuzzing test last, because it may leave some registers in an undefined
>>> state.
>>>
>>> Paolo
>