Messages in this thread | | | Date | Wed, 2 Mar 2022 16:42:45 +0300 | From | "Kirill A. Shutemov" <> | Subject | Re: [PATCHv4 11/30] x86/tdx: Handle in-kernel MMIO |
| |
On Thu, Feb 24, 2022 at 12:11:54PM -0800, Dave Hansen wrote: > > I found a few things lacking in that description. How's that for a rewrite?
Looks great, thanks. > > == 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. This will be implemented in the > > future, removing the bulk of MMIO #VEs. > > Given what is written here, this sounds like a great solution especially > compared to all the instruction decoding nasiness. What's wrong with it?
This will not cover non-virtio users. So #VE-based MMIO will remain as fallback mechanism.
> > + switch (mmio) { > > + case MMIO_WRITE: > > + memcpy(&val, reg, size); > > + return mmio_write(size, ve->gpa, val); > > + case MMIO_WRITE_IMM: > > + val = insn.immediate.value; > > + return mmio_write(size, ve->gpa, val); > > + case MMIO_READ: > > + case MMIO_READ_ZERO_EXTEND: > > + case MMIO_READ_SIGN_EXTEND: > > + break; > > + case MMIO_MOVS: > > + case MMIO_DECODE_FAILED: > > + return false; > > + default: > > + BUG(); > > + } > > Given the huge description above, it's borderline criminal to not > discuss what could led to this BUG().
This bug actually covers "Unknown insn_decode_mmio() decode value" case. I will add the comment.
> It could literally be some minor tweak in the compiler that changed a > non-io.h-using MMIO access to get converted over to a instruction that > can't be decoded. > > Could we spend a few lines of comments to help out the future poor sod > that sees "kernel bug at foo.c:1234"? Maybe: > > /* > * MMIO was accessed with an instruction that could not > * be decoded. It was likely not using io.h helpers or > * accessed MMIO accidentally. > */
Thanks, I will use the comment for MMIO_DECODE_FAILED handling.
-- Kirill A. Shutemov
| |