Re: [RFC v2-fix-v2 1/2] x86/sev-es: Abstract out MMIO instruction decoding

[Dan Williams]

On Wed, Jun 2, 2021 at 12:42 PM Kuppuswamy Sathyanarayanan
<sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx> wrote:
>
> From: "Kirill A. Shutemov" <kirill.shutemov@xxxxxxxxxxxxxxx>
>

Lead in with the what because I read 2 paragraphs to figure out that
this was a prep patch.

"In preparation for sharing MMIO instruction decode between SEV-ES and
TDX factor out the common decode into a new insn_decode_mmio()
helper."

> For regular virtual machine, MMIO is handled by the VMM: KVM
> emulates instruction that caused MMIO. But, this model doesn't
> work for a secure VMs (like SEV or TDX) as VMM doesn't have
> access to the guest memory and register state. VMM needs
> assistance in handling MMIO: it induces exception in the guest.
> Guest has to decode the instruction and handle it on its own.
>
> Instruction decoding logic is similar between AMD SEV and TDX
> code. So extract the decoding code to insn-eval.c where it can
> be used by both SEV and TDX.
>
> This code adds no functional changes. It is only build-tested
> for SEV.

The diff is such that I could not verify "no functional change" change
without doing more careful analysis. Typically with non-trivial
refactoring they are split out over a few patches with a final removal
of replaced infra at the end. This does the entire conversion all at
once.

How about an approach that has vc_handle_mmio() handle
MMIO_DECODE_FAILED for missing support in the common helper until the
final patch that can do:

> +       mmio = insn_decode_mmio(insn, &bytes);
> +       if (mmio == MMIO_DECODE_FAILED)
> +               return ES_DECODE_FAILED;

...i.e. insn_decode_mmio() is finally prepared to handle all scenarios
and vc_handle_mmio_twobyte_ops() can finally be deleted. This also
helps a future bisect that finds "whoops, 'no functional changes' was
incorrect".

>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
> Cc: Tom Lendacky <thomas.lendacky@xxxxxxx>
> Cc: Joerg Roedel <jroedel@xxxxxxx>

Missing Sathya signed-off-by...