Re: [PATCH v1 12/23] KVM: VMX: Handle FRED event data

[Chao Gao]

On Tue, Nov 14, 2023 at 12:34:02PM +0800, Li, Xin3 wrote:
>> >+			else if (is_nm_fault(intr_info) &&
>> >+				 vcpu->arch.guest_fpu.fpstate->xfd)
>> 
>> does this necessarily mean the #NM is caused by XFD?
>
>Then the event data should be 0.  Or I missed something obvious? I.e.,
>it can be easily differentiated and we should just explicitly set it
>to 0.

vcpu->arch.guest_fpu.fpstate->xfd just means the guest is enabling XFD.
I don't think we can conclude that this #NM is caused by XFD only from
this.  i.e., there may be some false positives.

>> >+			u64 event_data = vmcs_read64(event_data_field);
>> >+
>> >+			switch (vector) {
>> >+			case DB_VECTOR:
>> >+				get_debugreg(vcpu->arch.dr6, 6);
>> >+				WARN_ON(vcpu->arch.dr6 != (event_data ^
>> DR6_RESERVED));
>> >+				vcpu->arch.dr6 = event_data ^ DR6_RESERVED;
>> >+				break;
>> >+			case NM_VECTOR:
>> >+				if (vcpu->arch.guest_fpu.fpstate->xfd) {
>> >+					rdmsrl(MSR_IA32_XFD_ERR, vcpu-
>> >arch.guest_fpu.xfd_err);
>> >+					WARN_ON(vcpu-
>> >arch.guest_fpu.xfd_err != event_data);
>> >+					vcpu->arch.guest_fpu.xfd_err =
>> event_data;
>> >+				} else {
>> >+					WARN_ON(event_data != 0);
>> >+				}
>> >+				break;
>> >+			case PF_VECTOR:
>> >+				WARN_ON(vcpu->arch.cr2 != event_data);
>> >+				vcpu->arch.cr2 = event_data;
>> >+				break;
>> >+			default:
>> >+				WARN_ON(event_data != 0);
>> 
>> I am not sure if this WARN_ON() can be triggeded by nested VMX. It is legitimate
>> for L1 VMM to inject any event w/ an event_data.
>> 
>> FRED spec says:
>> 
>> Section 5.2.1 specifies the event data that FRED event delivery of certain events
>> saves on the stack. When FRED event delivery is used for an event injected by VM
>> entry, the event data saved is the value of the injected-event-data field in the
>> VMCS. This value is used instead of what is specified in Section 5.2.1 and is done
>> for __ALL__ injected events using FRED event delivery
>
>5.2.1 Saving Information on the Regular Stack also says:
>- For any other event, the event data are not currently defined and will
>  be zero until they are.
>
>Or you mean something else?

IIUC, L1 KVM can inject a nested exception whose vector isn't #DB, or #NM or
#PF with a non-zero event_data to L2. If delivering the nested exception causes a
VM-exit to L0 KVM, the assertion that event_data is always 0 for vectors other
than #DB/#NM/#PF fails.