Re: [PATCH v3 16/27] KVM: VMX: Virtualize FRED nested exception tracking

[Xin Li <xin@xxxxxxxxx>]

On 10/23/2024 11:24 PM, Chao Gao wrote:
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index b9b82aaea9a3..3830084b569b 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -736,6 +736,7 @@ struct kvm_queued_exception {
> > 	u32 error_code;
> > 	unsigned long payload;
> > 	bool has_payload;
> > +	bool nested;
> > 	u64 event_data;
>
> how "nested" is migrated in live migration?

Damn, I forgot it!

Looks we need to add it to kvm_vcpu_ioctl_x86_{get,set}_vcpu_events(),
but the real question is how to add it to struct kvm_vcpu_events.

> > };
>
> [..]
>
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index d81144bd648f..03f42b218554 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -1910,8 +1910,11 @@ void vmx_inject_exception(struct kvm_vcpu *vcpu)
> > 		vmcs_write32(VM_ENTRY_INSTRUCTION_LEN,
> > 			     vmx->vcpu.arch.event_exit_inst_len);
> > 		intr_info |= INTR_TYPE_SOFT_EXCEPTION;
> > -	} else
> > +	} else {
> > 		intr_info |= INTR_TYPE_HARD_EXCEPTION;
> > +		if (ex->nested)
> > +			intr_info |= INTR_INFO_NESTED_EXCEPTION_MASK;
>
> how about moving the is_fred_enable() check from kvm_multiple_exception() to here? i.e.,
>
> 		if (ex->nested && is_fred_enabled(vcpu))
> 			intr_info |= INTR_INFO_NESTED_EXCEPTION_MASK;
>
> It is slightly clearer because FRED details don't bleed into kvm_multiple_exception().

But FRED is all about events, including exception/interrupt/trap/...

logically VMX nested exception only works when FRED is enabled, see how 
it is set at 2 places in kvm_multiple_exception().

> > +	}
> >
> > 	vmcs_write32(VM_ENTRY_INTR_INFO_FIELD, intr_info);
> >
> > @@ -7290,6 +7293,7 @@ static void __vmx_complete_interrupts(struct kvm_vcpu *vcpu,
> > 		kvm_requeue_exception(vcpu, vector,
> > 				      idt_vectoring_info & VECTORING_INFO_DELIVER_CODE_MASK,
> > 				      error_code,
> > +				      idt_vectoring_info & VECTORING_INFO_NESTED_EXCEPTION_MASK,
> > 				      event_data);
> > 		break;
> > 	}
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 7a55c1eb5297..8546629166e9 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -874,6 +874,11 @@ static void kvm_multiple_exception(struct kvm_vcpu *vcpu, unsigned int nr,
> > 		vcpu->arch.exception.pending = true;
> > 		vcpu->arch.exception.injected = false;
> >
> > +		vcpu->arch.exception.nested = vcpu->arch.exception.nested ||
> > +					      (is_fred_enabled(vcpu) &&
> > +					       (vcpu->arch.nmi_injected ||
> > +					        vcpu->arch.interrupt.injected));
> > +
> > 		vcpu->arch.exception.has_error_code = has_error;
> > 		vcpu->arch.exception.vector = nr;
> > 		vcpu->arch.exception.error_code = error_code;
> > @@ -903,8 +908,13 @@ static void kvm_multiple_exception(struct kvm_vcpu *vcpu, unsigned int nr,
> > 		vcpu->arch.exception.injected = false;
> > 		vcpu->arch.exception.pending = false;
> >
> > +		/* #DF is NOT a nested event, per its definition. */
> > +		vcpu->arch.exception.nested = false;
> > +
> > 		kvm_queue_exception_e(vcpu, DF_VECTOR, 0);
> > 	} else {
> > +		vcpu->arch.exception.nested = is_fred_enabled(vcpu);
> > +
> > 		/* replace previous exception with a new one in a hope
> > 		   that instruction re-execution will regenerate lost
> > 		   exception */