diff mbox series

[01/12] KVM: X86: Fix when shadow_root_level=5 && guest root_level<4

Message ID 20211124122055.64424-2-jiangshanlai@gmail.com (mailing list archive)
State New, archived
Headers show
Series KVM: X86: misc fixes and cleanup | expand

Commit Message

Lai Jiangshan Nov. 24, 2021, 12:20 p.m. UTC 
From: Lai Jiangshan <laijs@linux.alibaba.com>

If the is an L1 with nNPT in 32bit, the shadow walk starts with
pae_root.

Fixes: a717a780fc4e ("KVM: x86/mmu: Support shadowing NPT when 5-level paging is enabled in host)
Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
---
 arch/x86/kvm/mmu/mmu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Sean Christopherson Dec. 9, 2021, 1:16 a.m. UTC | #1 
On Wed, Nov 24, 2021, Lai Jiangshan wrote:
> From: Lai Jiangshan <laijs@linux.alibaba.com>
> 
> If the is an L1 with nNPT in 32bit, the shadow walk starts with
> pae_root.
> 
> Fixes: a717a780fc4e ("KVM: x86/mmu: Support shadowing NPT when 5-level paging is enabled in host)

Have you actually run with 5-level nNPT?  I don't have access to hardware, at least
not that I know of :-)

I'm staring at kvm_mmu_sync_roots() and don't see how it can possibly work for
5-level nNPT with a 4-level NPT guest.
Sean Christopherson Dec. 9, 2021, 1:21 a.m. UTC | #2 
On Thu, Dec 09, 2021, Sean Christopherson wrote:
> On Wed, Nov 24, 2021, Lai Jiangshan wrote:
> > From: Lai Jiangshan <laijs@linux.alibaba.com>
> > 
> > If the is an L1 with nNPT in 32bit, the shadow walk starts with
> > pae_root.
> > 
> > Fixes: a717a780fc4e ("KVM: x86/mmu: Support shadowing NPT when 5-level paging is enabled in host)
> 
> Have you actually run with 5-level nNPT?  I don't have access to hardware, at least
> not that I know of :-)
> 
> I'm staring at kvm_mmu_sync_roots() and don't see how it can possibly work for
> 5-level nNPT with a 4-level NPT guest.

Oh, and fast_pgd_switch() will also break kvm_mmu_sync_prev_roots() / is_unsync_root()
by putting a root into the prev_roots array that doesn't have a shadow page associated
with the root.
Lai Jiangshan Dec. 10, 2021, 9:34 a.m. UTC | #3 
On 2021/12/9 09:16, Sean Christopherson wrote:
> On Wed, Nov 24, 2021, Lai Jiangshan wrote:
>> From: Lai Jiangshan <laijs@linux.alibaba.com>
>>
>> If the is an L1 with nNPT in 32bit, the shadow walk starts with
>> pae_root.
>>
>> Fixes: a717a780fc4e ("KVM: x86/mmu: Support shadowing NPT when 5-level paging is enabled in host)
> 
> Have you actually run with 5-level nNPT?  I don't have access to hardware, at least
> not that I know of :-)

The code is just obvious incorrect for shadow_root_level=5 && guest root_level<4.

> 
> I'm staring at kvm_mmu_sync_roots() and don't see how it can possibly work for
> 5-level nNPT with a 4-level NPT guest.
> 

It doesn't use pml5_root for 5-level nNPT with a 4-level NPT guest, so
kvm_mmu_sync_roots() can work in a silence way with an "unexpected" root shadow
page.  It has problems for 5-level nNPT with a 4-level NPT guest.

See:
https://lore.kernel.org/lkml/20211210092508.7185-1-jiangshanlai@gmail.com/

especially patch4.

Your this reply motivated me to complete the changelog of a patchset and send
it, thanks!

Although the patchset is immature, it would be better than losing it.
diff mbox series

Patch

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 6948f2d696c3..701c67c55239 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -2171,10 +2171,10 @@  static void shadow_walk_init_using_root(struct kvm_shadow_walk_iterator *iterato
 	iterator->shadow_addr = root;
 	iterator->level = vcpu->arch.mmu->shadow_root_level;
 
-	if (iterator->level == PT64_ROOT_4LEVEL &&
+	if (iterator->level >= PT64_ROOT_4LEVEL &&
 	    vcpu->arch.mmu->root_level < PT64_ROOT_4LEVEL &&
 	    !vcpu->arch.mmu->direct_map)
-		--iterator->level;
+		iterator->level = PT32E_ROOT_LEVEL;
 
 	if (iterator->level == PT32E_ROOT_LEVEL) {
 		/*