On Thu, Sep 01, 2022 at 09:50:10AM -0700, David Matlack wrote:
> On Tue, Aug 30, 2022 at 4:57 PM Isaku Yamahata <isaku.yamahata@xxxxxxxxx> wrote:
> > On Fri, Aug 26, 2022 at 04:12:25PM -0700, David Matlack <dmatlack@xxxxxxxxxx> wrote:
> > > int kvm_tdp_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
> > > {
> > > /*
> > > @@ -4355,6 +4384,11 @@ int kvm_tdp_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
> > > }
> > > }
> > >
> > > +#ifdef CONFIG_X86_64
> > > + if (tdp_mmu_enabled)
> > > + return kvm_tdp_mmu_page_fault(vcpu, fault);
> > > +#endif
> > > +
> > > return direct_page_fault(vcpu, fault);
> > > }
> >
> > Now we mostly duplicated page_fault method. We can go one step further.
> > kvm->arch.mmu.page_fault can be set for each case. Maybe we can do it later
> > if necessary.
>
> Hm, interesting idea. We would have to refactor the MTRR max_level
> code in kvm_tdp_page_fault() into a helper function, but otherwise
> that idea would work. I will give it a try in the next version.
So I took a stab at this. Refactoring the max_level adjustment for MTRRs
into a helper function is easy of course. But changing page_fault also
requires changes in kvm_mmu_do_page_fault() for CONFIG_RETPOLINE and
fault->is_tdp. I'm not saying it's not possible (it definitely is) but
it's not trivial to do it in a clean way, so I suggest we table this for
the time being.
