On 20/03/20 22:28, Sean Christopherson wrote:
> +void kvm_inject_l1_page_fault(struct kvm_vcpu *vcpu,
> + struct x86_exception *fault)
> +{
> + vcpu->arch.mmu->inject_page_fault(vcpu, fault);
> +}
> +
> bool kvm_inject_emulated_page_fault(struct kvm_vcpu *vcpu,
> struct x86_exception *fault)
> {
> @@ -619,7 +625,7 @@ bool kvm_inject_emulated_page_fault(struct kvm_vcpu *vcpu,
> if (mmu_is_nested(vcpu) && !fault->nested_page_fault)
> vcpu->arch.nested_mmu.inject_page_fault(vcpu, fault);
> else
> - vcpu->arch.mmu->inject_page_fault(vcpu, fault);
> + kvm_inject_l1_page_fault(vcpu, fault);
>
> return fault->nested_page_fault;
This all started with "I don't like the name of the function" but
thinking more about it, we can also write this as
if (mmu_is_nested(vcpu) && !fault->nested_page_fault)
vcpu->arch.walk_mmu->inject_page_fault(vcpu, fault);
else
vcpu->arch.mmu->inject_page_fault(vcpu, fault);
Now, if !mmu_is_nested(vcpu) then walk_mmu == mmu, so it's much simpler
up until this patch:
fault_mmu = fault->nested_page_fault ? vcpu->arch.mmu : vcpu->arch.walk_mmu;
fault_mmu->inject_page_fault(vcpu, fault);
(which also matches how fault->nested_page_fault is assigned to).
In patch 7 we add the invalidation in kvm_inject_l1_page_fault, but
is it necessary to do it only in the else?
+ if (!vcpu->arch.mmu->direct_map &&
+ (fault->error_code & PFERR_PRESENT_MASK))
+ vcpu->arch.mmu->invlpg(vcpu, fault->address,
+ vcpu->arch.mmu->root_hpa);
+
vcpu->arch.mmu->inject_page_fault(vcpu, fault);
}
The direct_map check is really just an optimization to avoid a
retpoline if ->invlpg is nonpaging_invlpg. We can change it to
!vcpu->arch.mmu->invlpg if nonpaging_invlpg is replaced with NULL,
and then the same "if" condition can also be used for the nested_mmu
i.e. what patch 7 writes as
+ /*
+ * No need to sync SPTEs, the fault is being injected into L2,
+ * whose page tables are not being shadowed.
+ */
vcpu->arch.nested_mmu.inject_page_fault(vcpu, fault);
Finally, patch 7 also adds a tlb_flush_gva call which is already present
in kvm_mmu_invlpg, and this brings the final form to look like this:
bool kvm_inject_emulated_page_fault(struct kvm_vcpu *vcpu,
struct x86_exception *fault)
{
struct kvm_mmu *fault_mmu;
WARN_ON_ONCE(fault->vector != PF_VECTOR);
fault_mmu = fault->nested_page_fault ? vcpu->arch.mmu : vcpu->arch.walk_mmu;
/*
* Invalidate the TLB entry for the faulting address, if it exists,
* else the access will fault indefinitely (and to emulate hardware).
*/
if (fault->error_code & PFERR_PRESENT_MASK)
__kvm_mmu_invlpg(vcpu, fault_mmu, fault->address);
fault_mmu->inject_page_fault(vcpu, fault);
return fault->nested_page_fault;
}
This will become a formal mini-series replacing patches 6 and 7
after I test it, so no need to do anything on your part.
Paolo
