On Thu, Nov 17, 2022 at 5:35 PM Robert Hoo <robert.hu@xxxxxxxxxxxxxxx> wrote:
>
> On Thu, 2022-11-17 at 11:14 -0500, Paolo Bonzini wrote:
> > +
> > if (fault->nx_huge_page_workaround_enabled)
> > disallowed_hugepage_adjust(fault,
> > iter.old_spte, iter.level);
> >
> And here can also be improved, I think.
>
> tdp_mmu_for_each_pte(iter, mmu, fault->gfn, fault->gfn + 1) {
> - if (fault->nx_huge_page_workaround_enabled)
> + if (fault->huge_page_disallowed)
>
> in the case of !fault->exec && fault->nx_huge_page_workaround_enabled,
> huge page should be still allowed, shouldn't it?
>
> If you agree, I can send out a patch for this. I've roughly tested
> this, with an ordinary guest boot, works normally.
This check handles the case where a read or write fault occurs within
a region that has already been split due to an NX huge page. If we
recovered the NX Huge Page on such faults, the guest could end up
continuously faulting on the same huge page (e.g. if writing to one
page and executing from another within a GPA region backed by a huge
page). So instead, NX Huge Page recovery is done periodically by a
background thread.
That being said, I'm not surprised you didn't encounter any issues
when testing. Now that the TDP MMU fully splits NX Huge Pages on
fault, such faults should be rare at best. Perhaps even impossible?
Hm, can we can drop the call to disallowed_hugepage_adjust() entirely?
