Dear Maxime, On Sat, Oct 31, 2020 at 08:24:05AM +0100, Maxime Villard wrote: > Le 29/10/2020 à 21:54, Reinoud Zandijk a écrit : > > i stumbled on an instruction that NVMM couldn't code and thus couldn't > > emulate either. It was as stated the REPE CMPS (0xF3A7) instruction as stated > > in https://c9x.me/x86/html/file_module_x86_id_279.html and confirmed by > > disassembly by ndisasm (from nasm). > > > > Appended is the implementation of imentioned instruction together with its > > byte sized sibling 0xF3A6. When installing the modified libnvmm, qemu behaves > > like NVMM is not used. I think the implementation does the right thing but > > feel free to double check! > > > > Tested and found by NetBSD/amd64 9.99.74 (19 oct) on an Intel Celeron 2957U by > > executing: > > qemu-system-x86_64 -accel nvmm -nographic -netdev \ > > user,id=n0,tftp=/usr/mdec,bootfile=pxeboot_ia32.bin -device \ > > e1000,netdev=n0 -boot n > This is incorrect and you should revert. x86_emul_cmp deals with single memory > operands, not double. And rdi+rsi must be incremented/decremented depending on > PSL_D. Also you added printfs in the library, wtf? Reverted it for now on your request. I wasn't aware of that; i assumed the REPE prefix code would deal with that! As for the printf()'s I cleaned them up before the commit and the only remainder would spit out the offending instruction it couldn't decode and ask to report this to NetBSD. > As a general rule each instruction that libnvmm can emulate should have unit > tests in t_mem_assist -- in fact here a single test case would have shown that > the code is obviously wrong. Adding a specific test case would be better yes; is this instruction just hardly ever used, trapped normally (ie CPU issue) or is there a rare case where it could need mem assist say on crossing page boundaries or uninitialized memory? Since libnvmm's x86_decode() doesn't recognize REPE CMPS it aborted Qemu, hence the patch that tried to fix this. With the patch, Qemu behaves the same as without NVMM though apparently with a side effect. I'll dig into this. Is there code already that i could use to enhance the patch to fix this? Its triggering is quite rare apparently given that you managed to run Win10 on it just fine :) With regards, Reinoud
Attachment:
signature.asc
Description: PGP signature