lkml.org 
[lkml]   [2022]   [Jan]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 08/26] x86/tdx: Handle in-kernel MMIO
On Tue, Dec 14, 2021 at 06:02:46PM +0300, Kirill A. Shutemov wrote:
> In non-TDX VMs, MMIO is implemented by providing the guest a mapping
> which will cause a VMEXIT on access and then the VMM emulating the
> instruction that caused the VMEXIT. That's not possible in TDX guests
> because it requires exposing guest register and memory state to
> potentially malicious VMM.

What does that mean exactly? Aren't TDX registers encrypted just like
SEV-ES ones? If so, they can't really be exposed...

> In TDX the MMIO regions are instead configured to trigger a #VE
> exception in the guest. The guest #VE handler then emulates the MMIO
> instruction inside the guest and converts them into a controlled

s/them/it/

> hypercall to the host.
>
> MMIO addresses can be used with any CPU instruction that accesses the

s/the //

> memory. This patch, however, covers only MMIO accesses done via io.h

"Here are covered only the MMIO accesses ... "

> helpers, such as 'readl()' or 'writeq()'.
>
> MMIO access via other means (like structure overlays) may result in
> MMIO_DECODE_FAILED and an oops.

Why? They won't cause a EXIT_REASON_EPT_VIOLATION #VE or?

> AMD SEV has the same limitations to MMIO handling.

See, the other guy is no better here. :-P

> === Potential alternative approaches ===
>
> == Paravirtualizing all MMIO ==
>
> An alternative to letting MMIO induce a #VE exception is to avoid
> the #VE in the first place. Similar to the port I/O case, it is
> theoretically possible to paravirtualize MMIO accesses.
>
> Like the exception-based approach offered by this patch, a fully

"... offered here, a fully ..."

> paravirtualized approach would be limited to MMIO users that leverage
> common infrastructure like the io.h macros.
>
> However, any paravirtual approach would be patching approximately
> 120k call sites. With a conservative overhead estimation of 5 bytes per
> call site (CALL instruction), it leads to bloating code by 600k.
>
> Many drivers will never be used in the TDX environment and the bloat
> cannot be justified.

I like the conservative approach here.

> == Patching TDX drivers ==
>
> Rather than touching the entire kernel, it might also be possible to
> just go after drivers that use MMIO in TDX guests. Right now, that's
> limited only to virtio and some x86-specific drivers.
>
> All virtio MMIO appears to be done through a single function, which
> makes virtio eminently easy to patch. Future patches will implement this
> idea,

"This will be implemented in the future, ... "

> +static int tdx_handle_mmio(struct pt_regs *regs, struct ve_info *ve)
> +{
> + char buffer[MAX_INSN_SIZE];
> + unsigned long *reg, val = 0;
> + struct insn insn = {};
> + enum mmio_type mmio;
> + int size;
> + u8 sign_byte;
> + bool err;
> +
> + if (copy_from_kernel_nofault(buffer, (void *)regs->ip, MAX_INSN_SIZE))
> + return -EFAULT;
> +
> + insn_init(&insn, buffer, MAX_INSN_SIZE, 1);
> + insn_get_length(&insn);

There is insn_decode() - see how it is used and use it here pls.

> + case MMIO_READ_SIGN_EXTEND:
> + err = tdx_mmio_read(size, ve->gpa, &val);
> + if (err)
> + break;
> +
> + if (size == 1)
> + sign_byte = (val & 0x80) ? 0xff : 0x00;
> + else
> + sign_byte = (val & 0x8000) ? 0xff : 0x00;
> +
> + /* Sign extend based on operand size */
> + memset(reg, sign_byte, insn.opnd_bytes);
> + memcpy(reg, &val, size);
> + break;

You can simplify this a bit:

case MMIO_READ_SIGN_EXTEND: {
u8 sign_byte = 0, msb = 7;

err = tdx_mmio_read(size, ve->gpa, &val);
if (err)
break;

if (size > 1)
msb = 15;

if (val & BIT(msb))
sign_byte = -1;

/* Sign extend based on operand size */
memset(reg, sign_byte, insn.opnd_bytes);
memcpy(reg, &val, size);
break;
}



> + case MMIO_MOVS:
> + case MMIO_DECODE_FAILED:
> + return -EFAULT;
> + }
> +
> + if (err)
> + return -EFAULT;


<---- newline here.

> + return insn.length;
> +}
> +

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

\
 
 \ /
  Last update: 2022-01-05 11:39    [W:0.461 / U:1.316 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site