Re: [PATCH v19 110/130] KVM: TDX: Handle TDX PV MMIO hypercall

[Isaku Yamahata]

On Thu, Jul 18, 2024 at 03:33:25PM +0800,
Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx> wrote:

> > > > +
> > > > +		memcpy(&val, vcpu->run->mmio.data, size);
> > > > +		tdvmcall_set_return_val(vcpu, val);
> > > > +		trace_kvm_mmio(KVM_TRACE_MMIO_READ, size, gpa, &val);
> > > > +	}
> > > Tracepoint for KVM_TRACE_MMIO_WRITE is missing when it is handled in
> > > userspace.
> > tdx_mmio_write() has it before existing to the user space.  It matches with
> > how write_mmio() behaves in x86.c.
> > 
> > Hmm, to match with other code, we should remove
> > trace_kvm_mmio(KVM_TRACE_MMIO_READ) and keep KVM_TRACE_MMIO_READ_UNSATISFIED
> > in tdx_emulate_mmio().  That's how read_prepare() and read_exit_mmio() behaves.
> > 
> > For MMIO read
> > - When kernel can handle the MMIO, KVM_TRACE_MMIO_READ with data.
> > - When exiting to the user space, KVM_TRACE_MMIO_READ_UNSATISFIED before
> >    the exit.  No trace after the user space handled the MMIO.
> 
> For MMIO read, in the emulator, there is still a trace after the userspace
> handled the MMIO.
> In complete_emulated_mmio(), if all fragments have been handled, it will
> set vcpu->mmio_read_completed to 1 and call complete_emulated_io().
> complete_emulated_io
>     kvm_emulate_instruction(vcpu, EMULTYPE_NO_DECODE)
>         x86_emulate_instruction
>             x86_emulate_insn
>                 emulator_read_write
>                     read_prepare
>                         At this point, vcpu->mmio_read_completed is 1,
>                         it traces KVM_TRACE_MMIO_READ with data
>                         and then clear vcpu->mmio_read_completed
> 
> So to align with emulator, we should keep the trace for KVM_TRACE_MMIO_READ.

Oops, you're right. Agreed to keep it.
-- 
Isaku Yamahata <isaku.yamahata@xxxxxxxxx>