Re: [kvm-unit-tests PATCH v3 11/11] x86: AMD SEV-ES: Handle string IO for IOIO #VC

[Sean Christopherson <seanjc@google.com>]

On Fri, Apr 15, 2022, Marc Orr wrote:
> On Fri, Apr 15, 2022 at 11:30 AM Sean Christopherson <seanjc@google.com> wrote:
> > Yes, we can and probably should add wrappers for the raw string I/O tests too.
> > But, no matter what, somehwere there has to be a helper to translate raw string
> > I/O into SEV-ES string I/O.  I don't see why doing that in the #VC handler is any
> > easier than doing it in a helper.
> 
> Hmmm... yeah, if this patch really does get vetoed then rather than
> throw it away maybe we can convert it to be loaded with a helper now.
> 
> Note: I hear your arguments, but I still don't agree with throwing
> away this patch. At least not based on the arguments made in this
> email thread. I think having a default #VC handler to handle string IO
> is better than not having one. Individual tests can always override
> it.

What test is ever going to do its own string port I/O?  String MMIO is a different
beast because REP MOVS and REP STOS can be generated by the compiler almost at will,
e.g. memcpy(), memset(), struct initialization, random for-loops, etc...

Port I/O on the other requires very deliberate code.  I doubt it's even possible
to generate string port I/O without resorting to assembly.

Outside of emulator.c, the next closest instance is the use of KVM's "force emulation
prefix", which happens to sometimes decode as INSL due to using byte 0x6d :-)

realmode.c's print_serial() has string I/O, but (a) it's #ifdef'd out by default
and (b) would be trivial to convert to a common helper.

In other words, any test that does string I/O is going to have to either open code
it in inline asm or call a helper.  I'd much prefer we enable the latter.

> From reading the other email thread on the decoder, I get the
> sense that the real reason you're opposed to this patch is because
> you're opposed to pulling in the Linux decoder. I don't follow that
> patch as well as this one. So that may or may not be a valid reason to
> nuke this patch. I'll leave that for others to discuss.

Yeah, they're very intertwined, not having to pull in a massive decoder is a big
motivation for not wanting string I/O support in the #VC handler.  But, even were
that not the case, IMO bouncing through the #VC handler for string I/O is asinine
because the source of the #VC _knows_ that it wants to do string I/O.  Just call
a helper.