On Fri, Feb 21, 2025 at 05:36:39PM +0100, Paolo Bonzini wrote:
> 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.
>
No problem. We can make the changes in the next version.
> > 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.
>
Okay, that works, too.
> > 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.
Thank you for the feedback.
Wei.
