[0/5] KVM: x86/mmu: Clean up is_tdp_mmu_root and root_hpa checks

Message

David Matlack June 17, 2021, 11:19 p.m. UTC

This series is spun off from Sean's suggestions on [PATCH 1/8] of my TDP
MMU Fast Page Fault series [1].

Patches 1-2 and 4 cleans up some redundant code in the page fault handling path:
 - Redundant checks for TDP MMU enablement.
 - Redundant checks for root_hpa validity.

Patch 3 refactors is_tdp_mmu_root into a simpler function prototype.

Note to reviewers: I purposely opted not to remove the root_hpa check
from is_tdp_mmu even though it is theoretically redundant in the current
code. My rational is that it could be called from outside the page fault
handling code in the future where root_hpa can be invalid. This seems
more likely to happen than with the other functions since is_tdp_mmu()
is not inherently tied to page fault handling.

The cost of getting this wrong is high since the result would be we end
up calling executing pfn_to_page(-1 >> PAGE_SHIFT)->private in
to_shadow_page. A better solution might be to move the VALID_PAGE check
into to_shadow_page but I did not want to expand the scope of this
series.

To test this series I ran all kvm-unit-tests and KVM selftests on an
Intel Cascade Lake machine.

[1] https://lore.kernel.org/kvm/[e-mail]/

David Matlack (4):
  KVM: x86/mmu: Remove redundant is_tdp_mmu_root check
  KVM: x86/mmu: Remove redundant is_tdp_mmu_enabled check
  KVM: x86/mmu: Refactor is_tdp_mmu_root into is_tdp_mmu
  KVM: x86/mmu: Remove redundant root_hpa checks

 arch/x86/kvm/mmu/mmu.c     | 19 ++++++-------------
 arch/x86/kvm/mmu/tdp_mmu.c |  5 -----
 arch/x86/kvm/mmu/tdp_mmu.h |  5 ++---
 3 files changed, 8 insertions(+), 21 deletions(-)

Comments

Paolo Bonzini June 18, 2021, 10:45 a.m. UTC | #1

On 18/06/21 01:19, David Matlack wrote:
> This series is spun off from Sean's suggestions on [PATCH 1/8] of my TDP
> MMU Fast Page Fault series [1].
> 
> Patches 1-2 and 4 cleans up some redundant code in the page fault handling path:
>   - Redundant checks for TDP MMU enablement.
>   - Redundant checks for root_hpa validity.
> 
> Patch 3 refactors is_tdp_mmu_root into a simpler function prototype.
> 
> Note to reviewers: I purposely opted not to remove the root_hpa check
> from is_tdp_mmu even though it is theoretically redundant in the current
> code. My rational is that it could be called from outside the page fault
> handling code in the future where root_hpa can be invalid. This seems
> more likely to happen than with the other functions since is_tdp_mmu()
> is not inherently tied to page fault handling.
> 
> The cost of getting this wrong is high since the result would be we end
> up calling executing pfn_to_page(-1 >> PAGE_SHIFT)->private in
> to_shadow_page. A better solution might be to move the VALID_PAGE check
> into to_shadow_page but I did not want to expand the scope of this
> series.
> 
> To test this series I ran all kvm-unit-tests and KVM selftests on an
> Intel Cascade Lake machine.
> 
> [1] https://lore.kernel.org/kvm/[e-mail]/
> 
> David Matlack (4):
>    KVM: x86/mmu: Remove redundant is_tdp_mmu_root check
>    KVM: x86/mmu: Remove redundant is_tdp_mmu_enabled check
>    KVM: x86/mmu: Refactor is_tdp_mmu_root into is_tdp_mmu
>    KVM: x86/mmu: Remove redundant root_hpa checks
> 
>   arch/x86/kvm/mmu/mmu.c     | 19 ++++++-------------
>   arch/x86/kvm/mmu/tdp_mmu.c |  5 -----
>   arch/x86/kvm/mmu/tdp_mmu.h |  5 ++---
>   3 files changed, 8 insertions(+), 21 deletions(-)
> 

Queued, thanks.

Paolo