On Wed, Jun 03, 2020 at 04:23:25PM +0200, Joerg Roedel wrote:

quoted

quoted

+		 */
+		io_bytes   = (exit_info_1 >> 4) & 0x7;
+		ghcb_count = sizeof(ghcb->shared_buffer) / io_bytes;
+
+		op_count    = (exit_info_1 & IOIO_REP) ? regs->cx : 1;
+		exit_info_2 = min(op_count, ghcb_count);
+		exit_bytes  = exit_info_2 * io_bytes;
+
+		es_base = insn_get_seg_base(ctxt->regs, INAT_SEG_REG_ES);
+
+		if (!(exit_info_1 & IOIO_TYPE_IN)) {
+			ret = vc_insn_string_read(ctxt,
+					       (void *)(es_base + regs->si),

SEV(-ES) is 64-bit only, why bother with the es_base charade?

User-space can also cause IOIO #VC exceptions, and user-space can be
32-bit legacy code with segments, so es_base has to be taken into
account.

Is there actually a use case for this?  Exposing port IO to userspace
doesn't exactly improve security.

Given that i386 ABI requires EFLAGS.DF=0 upon function entry/exit, i.e. is
the de facto default, the DF bug implies this hasn't been tested.  And I
don't see how this could possibly have worked for SEV given that the kernel
unrolls string I/O because the VMM can't emulate string I/O.  Presumably
someone would have complained if they "needed" to run legacy crud.  The
host and guest obviously need major updates, so supporting e.g. DPDK with
legacy virtio seems rather silly.

quoted

quoted

+					       ghcb->shared_buffer, io_bytes,
+					       exit_info_2, df);

df handling is busted, it's aways non-zero.  Same goes for the SI/DI
adjustments below.

Right, this is fixed now.

quoted

Batching the memory accesses and I/O accesses separately is technically
wrong, e.g. a #DB on a memory access will result in bogus data being shown
in the debugger.  In practice it seems unlikely to matter, but I'm curious
as to why string I/O is supported in the first place.  I didn't think there
was that much string I/O in the kernel?

True, #DBs won't be correct anymore. Currently debugging is not
supported in SEV-ES guests anyway, but if it is supported the #DB
exception would happen in the #VC handler and not on the original
instruction.

As in, the guest can't debug itself?  Or the host can't debug the guest?