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

Chao Gao
2023-11-14T08:59:20Z

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.