Re: [PATCH v19 120/130] KVM: TDX: Add a method to ignore dirty logging

[Isaku Yamahata]

On Mon, Mar 18, 2024 at 05:43:33PM +0000,
"Edgecombe, Rick P" <rick.p.edgecombe@xxxxxxxxx> wrote:

> On Mon, 2024-03-18 at 10:12 -0700, Isaku Yamahata wrote:
> > I categorize as follows. Unless otherwise, I'll update this series.
> > 
> > - dirty log check
> >   As we will drop this ptach, we'll have no call site.
> > 
> > - KVM_BUG_ON() in main.c
> >   We should drop them because their logic isn't complex.
> What about "KVM: TDX: Add methods to ignore guest instruction
> emulation"? Is it cleanly blocked somehow?

KVM fault handler, kvm_mmu_page_fault(), is the caller into the emulation,
It should skip the emulation.

As the second guard, x86_emulate_instruction(), calls
check_emulate_instruction() callback to check if the emulation can/should be
done.  TDX callback can return it as X86EMUL_UNHANDLEABLE.  Then, the flow goes
to user space as error.  I'll update the vt_check_emulate_instruction().



> > - KVM_BUG_ON() in tdx.c
> >   - The error check of the return value from SEAMCALL
> >     We should keep it as it's unexpected error from TDX module. When
> > we hit
> >     this, we should mark the guest bugged and prevent further
> > operation.  It's
> >     hard to deduce the reason.  TDX mdoule might be broken.
> Yes. Makes sense.
> 
> > 
> >   - Other check
> >     We should drop them.
> 
> Offhand, I'm not sure what is in this category.

- Checking error code on TD enter/exit
  I'll revise how to check error from TD enter/exit.  We'll have new code.  I
  will update wrapper function to take struct kvm_tdx or struct tdx_vcpu, and
  revise to remove random check.  Cleanups related to kvm_rebooting,
  TDX_SW_ERROR, kvm_spurious_fault()

- Remaining random check for debug.
  The examples are as follows.  They were added for debug.


@@ -797,18 +788,14 @@ void tdx_vcpu_free(struct kvm_vcpu *vcpu)
         * list_{del,add}() on associated_tdvcpus list later.
         */
        tdx_disassociate_vp_on_cpu(vcpu);
-       WARN_ON_ONCE(vcpu->cpu != -1);
 
        /*
         * This methods can be called when vcpu allocation/initialization
         * failed. So it's possible that hkid, tdvpx and tdvpr are not assigned
         * yet.
         */
-       if (is_hkid_assigned(to_kvm_tdx(vcpu->kvm))) {
-               WARN_ON_ONCE(tdx->tdvpx_pa);
-               WARN_ON_ONCE(tdx->tdvpr_pa);
+       if (is_hkid_assigned(to_kvm_tdx(vcpu->kvm)))
                return;
-       }
 
        if (tdx->tdvpx_pa) {
                for (i = 0; i < tdx_info->nr_tdvpx_pages; i++) {
@@ -831,9 +818,9 @@ void tdx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
 {
 
        /* vcpu_deliver_init method silently discards INIT event. */
-       if (KVM_BUG_ON(init_event, vcpu->kvm))
+       if (init_event)
                return;
-       if (KVM_BUG_ON(is_td_vcpu_created(to_tdx(vcpu)), vcpu->kvm))
+       if (is_td_vcpu_created(to_tdx(vcpu)))
                return;
 
        /*
@@ -831,9 +818,9 @@ void tdx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
 {
 
        /* vcpu_deliver_init method silently discards INIT event. */
-       if (KVM_BUG_ON(init_event, vcpu->kvm))
+       if (init_event)
                return;
-       if (KVM_BUG_ON(is_td_vcpu_created(to_tdx(vcpu)), vcpu->kvm))
+       if (is_td_vcpu_created(to_tdx(vcpu)))
                return;
 
        /*
@@ -831,9 +818,9 @@ void tdx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
 {
 
        /* vcpu_deliver_init method silently discards INIT event. */
-       if (KVM_BUG_ON(init_event, vcpu->kvm))
+       if (init_event)
                return;
-       if (KVM_BUG_ON(is_td_vcpu_created(to_tdx(vcpu)), vcpu->kvm))
+       if (is_td_vcpu_created(to_tdx(vcpu)))
                return;
 
        /*

  
-- 
Isaku Yamahata <isaku.yamahata@xxxxxxxxx>