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