KVM MMU: check reserved bits only when CR4.PSE=1 or CR4.PAE=1

PATCH Superseded Xiao Guangrong 2 participants 3 messages Mar 17, 2010 – Mar 18, 2010

Subsystems (3)

x86 KVM KVM x86

lore

Share & tools

lore

Xiao Guangrong

author

2010-03-17 03:43 UTC #0 lore raw

PATCH v2 1/1 KVM MMU: check reserved bits only when CR4.PSE=1 or CR4.PAE=1

- The RSV bit is possibility set in error code when #PF occurred only if CR4.PSE=1 or CR4.PAE=1

- context->rsvd_bits_mask[1][0] is always 0

Changlog:
Move this operation to reset_rsvds_bits_mask() address Avi Kivity's suggestion

Signed-off-by Xiao Guangrong <[email protected]>

---
 arch/x86/kvm/mmu.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

arch/x86/kvm/mmu.c +9 -3

index b137515..c49f8ec 100644
@@ -2288,18 +2288,26 @@ static void reset_rsvds_bits_mask(struct kvm_vcpu *vcpu, int level)
 
 	if (!is_nx(vcpu))
 		exb_bit_rsvd = rsvd_bits(63, 63);
+
+	context->rsvd_bits_mask[1][0] = 0;
 	switch (level) {
 	case PT32_ROOT_LEVEL:
 		/* no rsvd bits for 2 level 4K page table entries */
 		context->rsvd_bits_mask[0][1] = 0;
 		context->rsvd_bits_mask[0][0] = 0;
+
+		/* check rsvd bits only when CR4.PSE=1 or CR4.PAE=1 */
+		if (!is_pse(vcpu)) {
+			context->rsvd_bits_mask[1][1] = 0;
+			break;
+		}
+
 		if (is_cpuid_PSE36())
 			/* 36bits PSE 4MB page */
 			context->rsvd_bits_mask[1][1] = rsvd_bits(17, 21);
 		else
 			/* 32 bits PSE 4MB page */
 			context->rsvd_bits_mask[1][1] = rsvd_bits(13, 21);
-		context->rsvd_bits_mask[1][0] = context->rsvd_bits_mask[1][0];
 		break;
 	case PT32E_ROOT_LEVEL:
 		context->rsvd_bits_mask[0][2] =

@@ -2312,7 +2320,6 @@ static void reset_rsvds_bits_mask(struct kvm_vcpu *vcpu, int level)
 		context->rsvd_bits_mask[1][1] = exb_bit_rsvd |
 			rsvd_bits(maxphyaddr, 62) |
 			rsvd_bits(13, 20);		/* large page */
-		context->rsvd_bits_mask[1][0] = context->rsvd_bits_mask[1][0];
 		break;
 	case PT64_ROOT_LEVEL:
 		context->rsvd_bits_mask[0][3] = exb_bit_rsvd |

@@ -2330,7 +2337,6 @@ 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] = context->rsvd_bits_mask[1][0];
 		break;
 	}
 }

Signature · 1 line

1.6.1.2

Marcelo Tosatti

2010-03-18 02:49 UTC #1 lore raw

Quoted 18 lines · Xiao Guangrong

- The RSV bit is possibility set in error code when #PF occurred only if CR4.PSE=1 or CR4.PAE=1
- context->rsvd_bits_mask[1][0] is always 0
Changlog:
Move this operation to reset_rsvds_bits_mask() address Avi Kivity's suggestion
Signed-off-by Xiao Guangrong <[email protected]>
---
 arch/x86/kvm/mmu.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)
arch/x86/kvm/mmu.c +0 -0
index b137515..c49f8ec 100644

arch/x86/kvm/mmu.c static void reset_rsvds_bits_mask(struct kvm_vcpu *vcpu, int level) · line 2292

Quoted 5 lines

arch/x86/kvm/mmu.c +2 -0

@@ -2288,18 +2288,26 @@ static void reset_rsvds_bits_mask(struct kvm_vcpu *vcpu, int level)
 
 	if (!is_nx(vcpu))
 		exb_bit_rsvd = rsvd_bits(63, 63);
+
+	context->rsvd_bits_mask[1][0] = 0;

So if the guest enables PAT at PTE level you completly disable reserved bit checking? You should only disable checking for [1][1] if !PSE.

Xiao Guangrong

author

2010-03-18 11:32 UTC #2 lore raw

Hi Marcelo,

Thanks for your review.

Quoted 20 lines · Marcelo Tosatti

Quoted 19 lines · Xiao Guangrong
- The RSV bit is possibility set in error code when #PF occurred only if CR4.PSE=1 or CR4.PAE=1
- context->rsvd_bits_mask[1][0] is always 0
Changlog:
Move this operation to reset_rsvds_bits_mask() address Avi Kivity's suggestion
Signed-off-by Xiao Guangrong <[email protected]>
---
 arch/x86/kvm/mmu.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)
arch/x86/kvm/mmu.c +0 -0
index b137515..c49f8ec 100644
@@ -2288,18 +2288,26 @@ static void reset_rsvds_bits_mask(struct kvm_vcpu *vcpu, int level)
 

Quoted 7 lines

arch/x86/kvm/mmu.c static void reset_rsvds_bits_mask(struct kvm_vcpu *vcpu, int level) · line 2292
Quoted 4 lines
arch/x86/kvm/mmu.c +2 -0
 	if (!is_nx(vcpu))
 		exb_bit_rsvd = rsvd_bits(63, 63);
+
+	context->rsvd_bits_mask[1][0] = 0;
So if the guest enables PAT at PTE level you completly disable reserved bit checking? You should only disable checking for [1][1] if !PSE.

Sorry, i make a mistake here because i see the current code is redundant:

	context->rsvd_bits_mask[1][0] = context->rsvd_bits_mask[1][0];

in every case.

This code is imported by commit fd2e987d, i think this is Avi Kivity's typo :-) i think the correct way is:

	ontext->rsvd_bits_mask[1][0] = context->rsvd_bits_mask[0][0];

I'll send a new version patch to fix it if you not object.

Xiao