[2/6] MMU: don't bail on PAT bits in PTE

Commit Message

Joerg Roedel May 15, 2009, 1:19 p.m. UTC

On Fri, May 15, 2009 at 12:53:42PM +0200, Alexander Graf wrote:
>
> On 15.05.2009, at 12:25, Michael S. Tsirkin wrote:
>
>> On Fri, May 15, 2009 at 10:22:16AM +0200, Alexander Graf wrote:
>>> A 64bit PTE can have bit7 set to 1 which means "Use this bit for the 
>>> PAT".
>>> Currently KVM's MMU code treats this bit as reserved, even though  
>>> it's not.
>>>
>>> As long as we're not required to make use of the PAT bits which is  
>>> only
>>> required for DMA/MMIO from my understanding, we can safely ignore it.
>>>
>>> Hyper-V uses this bit for kernel PTEs.
>>>
>>> Signed-off-by: Alexander Graf <[e-mail]>
>>> ---
>>> arch/x86/kvm/mmu.c |    2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
>>> index 8fcdae9..cce055a 100644
>>> --- a/arch/x86/kvm/mmu.c
>>> +++ b/arch/x86/kvm/mmu.c
>>> @@ -2169,7 +2169,7 @@ static void reset_rsvds_bits_mask(struct  
>>> kvm_vcpu *vcpu, int level)
>>> 		context->rsvd_bits_mask[1][1] = exb_bit_rsvd |
>>> 			rsvd_bits(maxphyaddr, 51) |
>>> 			rsvd_bits(13, 20);		/* large page */
>>> -		context->rsvd_bits_mask[1][0] = ~0ull;
>>> +		context->rsvd_bits_mask[1][0] = 0ull;
>>> 		break;
>>> 	}
>>> }
>>
>> Just to make sure I understand what this does: if guest sets bit7,  
>> will
>> bit7 get set in shadow PTEs as well?
>
> I don't see any code that interprets bit7, so the shadow PTE should be  
> completely unaffected.
>
> But to be sure I asked Jörg to take a look at it as well, as he's more  
> familiar with the x86 SPT code than I am :-).

The PAT bit is not propagated into the shadow page tables. Anyway, the
problem is fixed the wrong way in this patch. The real problem is that a
4kb pte is checked with mask considered for large pages (which do not
exist on walker level 0). The attached patch fixes it the better way
imho.

From 7530aef3ed580b70a74224f8c04857754501c496 Mon Sep 17 00:00:00 2001
From: Joerg Roedel <[e-mail]>
Date: Fri, 15 May 2009 15:14:19 +0200
Subject: [PATCH] kvm/mmu: fix reserved bit checking on 4kb pte level

The reserved bits checking code looks at bit 7 of the pte to determine
if it has to use the mask for a large pte or a normal pde. This does not
work on 4kb pte level because bit 7 is used there for PAT. Account this
in the checking function.

Signed-off-by: Joerg Roedel <[e-mail]>
---
 arch/x86/kvm/mmu.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

Comments

Avi Kivity May 17, 2009, 9:51 a.m. UTC | #1

Joerg Roedel wrote:
> Subject: [PATCH] kvm/mmu: fix reserved bit checking on 4kb pte level
>
> The reserved bits checking code looks at bit 7 of the pte to determine
> if it has to use the mask for a large pte or a normal pde. This does not
> work on 4kb pte level because bit 7 is used there for PAT. Account this
> in the checking function.
>
>  
>  static bool is_rsvd_bits_set(struct kvm_vcpu *vcpu, u64 gpte, int level)
>  {
> -	int bit7;
> +	int bit7 = 0;
> +
> +	if (level != PT_PAGE_TABLE_LEVEL)
> +		bit7 = (gpte >> 7) & 1;
>  
> -	bit7 = (gpte >> 7) & 1;
>  	return (gpte & vcpu->arch.mmu.rsvd_bits_mask[bit7][level-1]) != 0;
>  }
>  
>   

If we make rsvd_bits_mask[1][0] == rsvd_bits_mask[0][0], we don't need 
the extra check.  That's why it is named bit7 and not pse (need to make 
sure bit 7 is not reserved in this case).

Patch

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 479e748..8d9552e 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -2124,9 +2124,11 @@  static void paging_free(struct kvm_vcpu *vcpu)
 
 static bool is_rsvd_bits_set(struct kvm_vcpu *vcpu, u64 gpte, int level)
 {
-	int bit7;
+	int bit7 = 0;
+
+	if (level != PT_PAGE_TABLE_LEVEL)
+		bit7 = (gpte >> 7) & 1;
 
-	bit7 = (gpte >> 7) & 1;
 	return (gpte & vcpu->arch.mmu.rsvd_bits_mask[bit7][level-1]) != 0;
 }