>
> So if we were to straight-forwardly implement that based on how TDX
> currently handles checking for the shared bit in GPA, paired with how
> SEV-SNP handles checking for private bit in fault flags, it would look
> something like:
>
> bool kvm_fault_is_private(kvm, gpa, err)
> {
> /* SEV-SNP handling */
> if (kvm->arch.mmu_private_fault_mask)
> return !!(err & arch.mmu_private_fault_mask);
>
> /* TDX handling */
> if (kvm->arch.gfn_shared_mask)
> return !!(gpa & arch.gfn_shared_mask);
The logic of the two are identical. I think they need to be converged.
Either SEV-SNP should convert the error code private bit to the gfn_shared_mask,
or TDX's shared bit should be converted to some private error bit.
Perhaps converting SEV-SNP makes more sense because if I recall correctly SEV
guest also has a C-bit, correct?
Or, ...
>
> return false;
> }
>
> kvm_mmu_do_page_fault(vcpu, gpa, err, ...)
> {
> struct kvm_page_fault fault = {
> ...
> .is_private = kvm_fault_is_private(vcpu->kvm, gpa, err)
... should we do something like:
.is_private = static_call(kvm_x86_fault_is_private)(vcpu->kvm, gpa,
err);
?
> };
>
> ...
> }
>
> And then arch.mmu_private_fault_mask and arch.gfn_shared_mask would be
> set per-KVM-instance, just like they are now with current SNP and TDX
> patchsets, since stuff like KVM self-test wouldn't be setting those
> masks, so it makes sense to do it per-instance in that regard.
>
> But that still gets a little awkward for the KVM self-test use-case where
> .is_private should sort of be ignored in favor of whatever the xarray
> reports via kvm_mem_is_private().
>
I must have missed something. Why does KVM self-test have impact to how does
KVM handles private fault?
> In your Misc. series I believe you
> handled this by introducing a PFERR_HASATTR_MASK bit so we can determine
> whether existing value of fault->is_private should be
> ignored/overwritten or not.
>
> So maybe kvm_fault_is_private() needs to return an integer value
> instead, like:
>
> enum {
> KVM_FAULT_VMM_DEFINED,
> KVM_FAULT_SHARED,
> KVM_FAULT_PRIVATE,
> }
>
> bool kvm_fault_is_private(kvm, gpa, err)
> {
> /* SEV-SNP handling */
> if (kvm->arch.mmu_private_fault_mask)
> (err & arch.mmu_private_fault_mask) ? KVM_FAULT_PRIVATE : KVM_FAULT_SHARED
>
> /* TDX handling */
> if (kvm->arch.gfn_shared_mask)
> (gpa & arch.gfn_shared_mask) ? KVM_FAULT_SHARED : KVM_FAULT_PRIVATE
>
> return KVM_FAULT_VMM_DEFINED;
> }
>
> And then down in __kvm_faultin_pfn() we do:
>
> if (fault->is_private == KVM_FAULT_VMM_DEFINED)
> fault->is_private = kvm_mem_is_private(vcpu->kvm, fault->gfn);
> else if (fault->is_private != kvm_mem_is_private(vcpu->kvm, fault->gfn))
> return kvm_do_memory_fault_exit(vcpu, fault);
>
> if (fault->is_private)
> return kvm_faultin_pfn_private(vcpu, fault);
What does KVM_FAULT_VMM_DEFINED mean, exactly?
Shouldn't the fault type come from _hardware_?
