tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: NVMM missing opcode REPE CMPS implementation

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 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,

Attachment: signature.asc
Description: PGP signature

Home | Main Index | Thread Index | Old Index