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>
