On 2/21/25 09:36, Wei Liu wrote:
This patch series attempts to make the instruction emulator in HVF a common
component for the i386 target. It removes HVF specific code by either using a
set of hooks or moving it to better locations. The new incoming MSHV
accelerator will implement the hooks, and where necessary, enhance the emulator
and / or add new hooks.

Good!

This patch series is in RFC state. The patches have been lightly tested by
running a Linux VM on an Intel-based Mac.  We hope to get some feedback on the
overall approach, and let the community bikeshed a bit about names and
location.

For the bikeshedding my only suggestion is to replace mmio_buf with emu_mmio_buf, and replace x86-insn-emul, with just "emulate" or something like that. That is, no need to repeat x86 inside the target/i386 directory, especially since the filenames also start with x86.

First two patches fix issues in the existing code. They can be applied
regardless of the discussion around the overall approach.

These four can also be applied:

 target/i386/hvf: use x86_segment in x86_decode.c
 target/i386/hvf: move and rename {load, store}_regs
 target/i386/hvf: move and rename simulate_{rdmsr, wrmsr}
 target/i386/hvf: drop some dead code

The checkpatch script complains about a few things. Some are from the original
code I didn't touch. For the code I changed or moved, it complains that some
lines are long (>80). Seeing that the rule was not followed strictly in the old
code base, I held off fixing that class of issues. The other thing it complains
is there is no entry for the new directory in MAINTAINERS. We can fix these
issues if they are deemed important.

Yes, no problem. The new directory thing is just a warning but I think you could add a new entry with both MSHV and HVF people on it.

Please let us know what you think. The alternative is to duplicate the
instruction emulator code in the mshv accelerator. That looks to be a worse
option.
Yes, definitely.

Paolo


Reply via email to