[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 01/10] hvm/hpet: Add manual unit test code.
On 04/10/14 02:11, Jan Beulich wrote: On 09.04.14 at 20:35,<dslutz@xxxxxxxxxxx> wrote: In a xen source tree (with this patch applied): cd tools/tests/vhpet xen_source=../../.. sed -e "/#include/d" -e "1i#include \"emul.h\"\n" <$xen_source/xen/arch/x86/hvm/hpet.c >hpet.c cp $xen_source/xen/include/asm-x86/hpet.h . gcc -g -o test_vhpet hpet.c main.cAt least up to here this clearly should be in a Makefile, just like e.g. done for the x86 instruction emulator test. Opps, I just noticed that the "optional" got dropped. I had not planned on this being required to get these patches applied. I have not found a simple way to use git-format-patch and have just 1 subject different. Will work on using a makefile. I will be happy to make this into a much better test case. Since that will not happen quickly, it make sense to me to drop just this 1 patch and start a new set that includes it. I have no real clue on how to get it to be part of "normal" testing. This is because "make test" gives me: ... building 'checkpoint' extensiongcc -pthread -fno-strict-aliasing -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv -DNDEBUG -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv -fPIC -I../../tools/include -I../../tools/libxc -I../../tools/xenstore -I/usr/include/python2.7 -c xen/lowlevel/checkpoint/checkpoint.c -o build/temp.linux-x86_64-2.7/xen/lowlevel/checkpoint/checkpoint.o -fno-strict-aliasing -Werror gcc -pthread -fno-strict-aliasing -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv -DNDEBUG -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv -fPIC -I../../tools/include -I../../tools/libxc -I../../tools/xenstore -I/usr/include/python2.7 -c xen/lowlevel/checkpoint/libcheckpoint.c -o build/temp.linux-x86_64-2.7/xen/lowlevel/checkpoint/libcheckpoint.o -fno-strict-aliasing -Werror xen/lowlevel/checkpoint/libcheckpoint.c: In function 'stop_suspend_thread':xen/lowlevel/checkpoint/libcheckpoint.c:842:7: error: variable 'err' set but not used [-Werror=unused-but-set-variable] cc1: all warnings being treated as errors error: command 'gcc' failed with exit status 1 Build failed 0x100 make[1]: *** [test] Error 1 make[1]: Leaving directory `/home/don/xen/tools/python' make: *** [test] Error 2 dcs-xen-54:~/xen>make tests make: *** No rule to make target `tests'. Stop. dcs-xen-54:~/xen>make check make: *** No rule to make target `check'. Stop. dcs-xen-54:~/xen>e Makefile dcs-xen-54:~/xen>make -C tools test make: Entering directory `/home/don/xen/tools' make: *** No rule to make target `test'. Stop. make: Leaving directory `/home/don/xen/tools' And 4.3.1 on CentOS 5.10: ... ====================================================================== ERROR: Invalid Test (xen.xm.tests.test_create) ---------------------------------------------------------------------- Traceback (most recent call last): File "test.py", line 634, in get_suite mod = package_import(modname) File "test.py", line 608, in package_import mod = __import__(modname)File "/home/don/xen-4.3.1/tools/python/build/lib.linux-x86_64-2.4/xen/xm/tests/test_create.py", line 6, in ? import xen.xend.XendOptionsFile "/home/don/xen-4.3.1/tools/python/build/lib.linux-x86_64-2.4/xen/xend/XendOptions.py", line 35, in ? from xen.util import auxbinFile "/home/don/xen-4.3.1/tools/python/build/lib.linux-x86_64-2.4/xen/util/auxbin.py", line 22, in ? from xen.util.path import * ImportError: No module named path ---------------------------------------------------------------------- Ran 5 tests in 0.048s FAILED (errors=3) make[1]: Leaving directory `/home/don/xen-4.3.1/tools/python' ./test_vhpet >foo Most of this is in main.c's comment. Will add to commit message also.Just referring to the comments there would be enough I guess. But I'm still struggling to see how this would reproduce the Linux side issue, and not just some random problem manifesting in similar symptoms. I will attempt to answer this. During my looking for what was happening, I added debug code that would allow me to force "diff" to selected values. This was how I found out that "-diff > HPET_TINY_TIME_SPAN" would cause linux to report this and crash. On closer looking into this, I was able to determine that the use extInt path would also fail even if I got xen to provide this interrupt. The reason being that (uint32_t)(-HPET_TINY_TIME_SPAN-1) Sets the "delta" (ns) to more then 60 seconds in the future. And the timer test happens in the 1st 5 seconds of a linux boot on my test server. So I send out the v1 patch. Later I found out that any value that is > ~3 seconds would also cause a linux crash even if xen is changed to provide extInt interrupts. (And you said in: http://lists.xen.org/archives/html/xen-devel/2014-02/msg01857.html But in the end I think you're barking up the wrong tree: All of this code would be of no interest at all if Linux didn't for some reason go into that fallback code. And with that fallback code existing mainly (if not exclusively) to deal with (half-)broken hardware/firmware, we should really make sure our emulation isn't broken (i.e. the fallback never being made use of). So finding the "TBD reason" is what is really needed. (This e-mail also has the way I was able to get xen to "handle" extInt. and so far this "adjustment" to xen is not ready to be posted as a fix; including a good reason to add the code.) This caused me to send lots of time reading the hpet spec and hopefully understanding most of it. At the same time (and while waiting for a longer running test of Windows Vista) I would add additional debug output and trace output along with changes. The the turn around time was slow enough that it made sense to me to take the code and run it standalone. This is what this code does. It allows you to use an almost unmodified copy of hpet.c in a stand alone mode that needs nothing else from the xen source tree nor even have xen installed. Also during the long test time I noticed that using xen-hvmctx the output of cmp was not changing, but master clock was. This is what leads me to patch #7: hvm/hpet: Call hpet_get_comparator during hpet_save. Which is not needed do to the way hpet_get_comparator() works until patch #10: hvm/hpet: handle 1st period special Which will not work correctly without patch #7. Also during the long time run I found out using "xentrace: Add TRC_HVM_EMUL" (at the testing time it was TRC_HW_VCHIP, but that is mostly a rename) and some hvm_debug code I had added showed that the delta between guest_time_hpet() was offten ~200. However I would some times get much larger values. I also found out that linux expects that "delta" should be "period". This was the key to finding the issue that patch #3: hvm/hpet: Only set comparator or period not both Fixes. However since the spec says that "delta" can be any relation to "period"; using what linux wanted ("delta" should never be greater then "period") would be a bad change. Since the server that would report: ..MP-BIOS bug: 8254 timer.. Stopped doing this, and we were unable to reproduce this anywhere else; the best I could do was guess that some how the delta between the 2 calls to guest_time_hpet() in: tn_cmp = hpet_get_comparator(h, tn); cur_tick = hpet_read_maincounter(h); Would be a possible cause. It was not until patches #2 to #9 had been coded and tested and I was still looking for a good way to do patch #10 (most of my attempts would break something else) was I able to find the right set of conditions to get the test code to report on a bad "delta". I am not sure at what time I noticed patch #9: hvm/hpet: Correctly limit period to a maximum. This patch set order is not in the order of issues found. Just one that makes sense to me and was easy to separate into only 1 change per patch. -Don Slutz Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |