* [PATCH v2 1/3] KVM: vmx: fix ept reserved bits for 1-GByte page
@ 2014-08-19 9:04 Wanpeng Li
2014-08-19 9:04 ` [PATCH v2 2/3] KVM: x86: fix check legal type of Variable Range MTRRs Wanpeng Li
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Wanpeng Li @ 2014-08-19 9:04 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Marcelo Tosatti, Gleb Natapov, Zhang Yang, kvm, linux-kernel,
Wanpeng Li
EPT misconfig handler in kvm will check which reason lead to EPT
misconfiguration after vmexit. One of the reasons is that an EPT
paging-structure entry is configured with settings reserved for
future functionality. However, the handler can't identify if
paging-structure entry of reserved bits for 1-GByte page are
configured, since PDPTE which point to 1-GByte page will reserve
bits 29:12 instead of bits 7:3 which are reserved for PDPTE that
references an EPT Page Directory. This patch fix it by reserve
bits 29:12 for 1-GByte page.
Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
---
v1 -> v2:
* same "if" statement cover both 2MB and 1GB pages
* return 0xf8 for level == 4
* get the level by checking the return value of ept_rsvd_mask
arch/x86/kvm/vmx.c | 19 +++++++------------
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index cad37d5..2763f37 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -5521,17 +5521,12 @@ static u64 ept_rsvd_mask(u64 spte, int level)
for (i = 51; i > boot_cpu_data.x86_phys_bits; i--)
mask |= (1ULL << i);
- if (level > 2)
- /* bits 7:3 reserved */
- mask |= 0xf8;
- else if (level == 2) {
- if (spte & (1ULL << 7))
- /* 2MB ref, bits 20:12 reserved */
- mask |= 0x1ff000;
- else
- /* bits 6:3 reserved */
- mask |= 0x78;
- }
+ if (spte & (1ULL << 7))
+ /* 1GB/2MB page, bits 29:12 or 20:12 reserved respectively */
+ mask |= (PAGE_SIZE << ((level - 1) * 9)) - PAGE_SIZE;
+ else
+ /* bits 6:3 reserved */
+ mask |= 0x78;
return mask;
}
@@ -5561,7 +5556,7 @@ static void ept_misconfig_inspect_spte(struct kvm_vcpu *vcpu, u64 spte,
WARN_ON(1);
}
- if (level == 1 || (level == 2 && (spte & (1ULL << 7)))) {
+ if (level == 1 || (rsvd_bits & 0x38)) {
u64 ept_mem_type = (spte & 0x38) >> 3;
if (ept_mem_type == 2 || ept_mem_type == 3 ||
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/3] KVM: x86: fix check legal type of Variable Range MTRRs
2014-08-19 9:04 [PATCH v2 1/3] KVM: vmx: fix ept reserved bits for 1-GByte page Wanpeng Li
@ 2014-08-19 9:04 ` Wanpeng Li
2014-08-19 9:04 ` [PATCH v2 3/3] KVM: x86: #GP when attempts to write reserved bits " Wanpeng Li
2014-08-19 9:09 ` [PATCH v2 1/3] KVM: vmx: fix ept reserved bits for 1-GByte page Paolo Bonzini
2 siblings, 0 replies; 10+ messages in thread
From: Wanpeng Li @ 2014-08-19 9:04 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Marcelo Tosatti, Gleb Natapov, Zhang Yang, kvm, linux-kernel,
Wanpeng Li
The first entry in each pair(IA32_MTRR_PHYSBASEn) defines the base
address and memory type for the range; the second entry(IA32_MTRR_PHYSMASKn)
contains a mask used to determine the address range. The legal values
for the type field of IA32_MTRR_PHYSBASEn are 0,1,4,5, and 6. However,
IA32_MTRR_PHYSMASKn don't have type field. This patch avoid check if
the type field is legal for IA32_MTRR_PHYSMASKn.
Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
---
v1 -> v2:
* WARN_ON if not fall in variable Range MTRRs
* the base/mask can be separated just with an "&"
arch/x86/kvm/x86.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 5f5edb6..fb3ea7a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1747,7 +1747,13 @@ static bool mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
}
/* variable MTRRs */
- return valid_mtrr_type(data & 0xff);
+ WARN_ON(!(msr >= 0x200 && msr < 0x200 + 2 * KVM_NR_VAR_MTRR));
+
+ if ((msr & 1) == 0)
+ /* MTRR base */
+ return valid_mtrr_type(data & 0xff);
+ /* MTRR mask */
+ return true;
}
static int set_msr_mtrr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 3/3] KVM: x86: #GP when attempts to write reserved bits of Variable Range MTRRs
2014-08-19 9:04 [PATCH v2 1/3] KVM: vmx: fix ept reserved bits for 1-GByte page Wanpeng Li
2014-08-19 9:04 ` [PATCH v2 2/3] KVM: x86: fix check legal type of Variable Range MTRRs Wanpeng Li
@ 2014-08-19 9:04 ` Wanpeng Li
2014-08-19 9:11 ` Paolo Bonzini
2014-08-29 16:47 ` Paolo Bonzini
2014-08-19 9:09 ` [PATCH v2 1/3] KVM: vmx: fix ept reserved bits for 1-GByte page Paolo Bonzini
2 siblings, 2 replies; 10+ messages in thread
From: Wanpeng Li @ 2014-08-19 9:04 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Marcelo Tosatti, Gleb Natapov, Zhang Yang, kvm, linux-kernel,
Wanpeng Li
Section 11.11.2.3 of the SDM mentions "All other bits in the IA32_MTRR_PHYSBASEn
and IA32_MTRR_PHYSMASKn registers are reserved; the processor generates a
general-protection exception(#GP) if software attempts to write to them". This
patch do it in kvm.
Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
---
arch/x86/kvm/x86.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index fb3ea7a..b85da5f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1726,6 +1726,7 @@ static bool valid_mtrr_type(unsigned t)
static bool mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
{
int i;
+ u64 mask = 0;
if (!msr_mtrr_valid(msr))
return false;
@@ -1749,10 +1750,21 @@ static bool mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
/* variable MTRRs */
WARN_ON(!(msr >= 0x200 && msr < 0x200 + 2 * KVM_NR_VAR_MTRR));
- if ((msr & 1) == 0)
+ for (i = 63; i > boot_cpu_data.x86_phys_bits; i--)
+ mask |= (1ULL << i);
+ if ((msr & 1) == 0) {
/* MTRR base */
- return valid_mtrr_type(data & 0xff);
- /* MTRR mask */
+ if (!valid_mtrr_type(data & 0xff))
+ return false;
+ mask |= 0xf00;
+ } else
+ /* MTRR mask */
+ mask |= 0x7ff;
+ if (data & mask) {
+ kvm_inject_gp(vcpu, 0);
+ return false;
+ }
+
return true;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] KVM: vmx: fix ept reserved bits for 1-GByte page
2014-08-19 9:04 [PATCH v2 1/3] KVM: vmx: fix ept reserved bits for 1-GByte page Wanpeng Li
2014-08-19 9:04 ` [PATCH v2 2/3] KVM: x86: fix check legal type of Variable Range MTRRs Wanpeng Li
2014-08-19 9:04 ` [PATCH v2 3/3] KVM: x86: #GP when attempts to write reserved bits " Wanpeng Li
@ 2014-08-19 9:09 ` Paolo Bonzini
2014-08-19 11:16 ` Wanpeng Li
2 siblings, 1 reply; 10+ messages in thread
From: Paolo Bonzini @ 2014-08-19 9:09 UTC (permalink / raw)
To: Wanpeng Li; +Cc: Marcelo Tosatti, Gleb Natapov, Zhang Yang, kvm, linux-kernel
Il 19/08/2014 11:04, Wanpeng Li ha scritto:
> EPT misconfig handler in kvm will check which reason lead to EPT
> misconfiguration after vmexit. One of the reasons is that an EPT
> paging-structure entry is configured with settings reserved for
> future functionality. However, the handler can't identify if
> paging-structure entry of reserved bits for 1-GByte page are
> configured, since PDPTE which point to 1-GByte page will reserve
> bits 29:12 instead of bits 7:3 which are reserved for PDPTE that
> references an EPT Page Directory. This patch fix it by reserve
> bits 29:12 for 1-GByte page.
>
> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
> ---
> v1 -> v2:
> * same "if" statement cover both 2MB and 1GB pages
> * return 0xf8 for level == 4
I think you dropped this check by mistake.
> * get the level by checking the return value of ept_rsvd_mask
>
> arch/x86/kvm/vmx.c | 19 +++++++------------
> 1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index cad37d5..2763f37 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -5521,17 +5521,12 @@ static u64 ept_rsvd_mask(u64 spte, int level)
> for (i = 51; i > boot_cpu_data.x86_phys_bits; i--)
> mask |= (1ULL << i);
>
> - if (level > 2)
> - /* bits 7:3 reserved */
> - mask |= 0xf8;
> - else if (level == 2) {
> - if (spte & (1ULL << 7))
> - /* 2MB ref, bits 20:12 reserved */
> - mask |= 0x1ff000;
> - else
> - /* bits 6:3 reserved */
> - mask |= 0x78;
> - }
> + if (spte & (1ULL << 7))
You need to go this way if level == 1 too. Otherwise, you would report
bits 6:3 reserved if the hypervisor is using the ignored bit 7 (Table
28-6, Format of an EPT Page-Table Entry).
> + /* 1GB/2MB page, bits 29:12 or 20:12 reserved respectively */
> + mask |= (PAGE_SIZE << ((level - 1) * 9)) - PAGE_SIZE;
> + else
> + /* bits 6:3 reserved */
> + mask |= 0x78;
>
> return mask;
> }
> @@ -5561,7 +5556,7 @@ static void ept_misconfig_inspect_spte(struct kvm_vcpu *vcpu, u64 spte,
> WARN_ON(1);
> }
>
> - if (level == 1 || (level == 2 && (spte & (1ULL << 7)))) {
> + if (level == 1 || (rsvd_bits & 0x38)) {
- rsvd_bits will always be zero here. You need to check the return
value of ept_rsvd_mask(). Let's call it rsvd_mask in the rest of this
email.
- the test is inverted, you need to check that bits 5:3 are _not_
reserved, hence (rsvd_mask & 0x38) == 0.
- once you do this, the test also covers level 1.
I suggest that you write a testcase for kvm-unit-tests.
Paolo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] KVM: x86: #GP when attempts to write reserved bits of Variable Range MTRRs
2014-08-19 9:04 ` [PATCH v2 3/3] KVM: x86: #GP when attempts to write reserved bits " Wanpeng Li
@ 2014-08-19 9:11 ` Paolo Bonzini
2014-08-29 16:47 ` Paolo Bonzini
1 sibling, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2014-08-19 9:11 UTC (permalink / raw)
To: Wanpeng Li; +Cc: Marcelo Tosatti, Gleb Natapov, Zhang Yang, kvm, linux-kernel
Il 19/08/2014 11:04, Wanpeng Li ha scritto:
> Section 11.11.2.3 of the SDM mentions "All other bits in the IA32_MTRR_PHYSBASEn
> and IA32_MTRR_PHYSMASKn registers are reserved; the processor generates a
> general-protection exception(#GP) if software attempts to write to them". This
> patch do it in kvm.
>
> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
> ---
> arch/x86/kvm/x86.c | 18 +++++++++++++++---
> 1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index fb3ea7a..b85da5f 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1726,6 +1726,7 @@ static bool valid_mtrr_type(unsigned t)
> static bool mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> {
> int i;
> + u64 mask = 0;
>
> if (!msr_mtrr_valid(msr))
> return false;
> @@ -1749,10 +1750,21 @@ static bool mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> /* variable MTRRs */
> WARN_ON(!(msr >= 0x200 && msr < 0x200 + 2 * KVM_NR_VAR_MTRR));
>
> - if ((msr & 1) == 0)
> + for (i = 63; i > boot_cpu_data.x86_phys_bits; i--)
> + mask |= (1ULL << i);
> + if ((msr & 1) == 0) {
> /* MTRR base */
> - return valid_mtrr_type(data & 0xff);
> - /* MTRR mask */
> + if (!valid_mtrr_type(data & 0xff))
> + return false;
> + mask |= 0xf00;
> + } else
> + /* MTRR mask */
> + mask |= 0x7ff;
> + if (data & mask) {
> + kvm_inject_gp(vcpu, 0);
> + return false;
> + }
> +
> return true;
> }
>
>
Thanks, these two patches look good. Please write a testcase for
kvm-unit-tests (x86/msr.c), too.
Paolo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] KVM: vmx: fix ept reserved bits for 1-GByte page
2014-08-19 9:09 ` [PATCH v2 1/3] KVM: vmx: fix ept reserved bits for 1-GByte page Paolo Bonzini
@ 2014-08-19 11:16 ` Wanpeng Li
2014-08-19 12:07 ` Paolo Bonzini
0 siblings, 1 reply; 10+ messages in thread
From: Wanpeng Li @ 2014-08-19 11:16 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Marcelo Tosatti, Gleb Natapov, Zhang Yang, kvm, linux-kernel
On Tue, Aug 19, 2014 at 11:09:49AM +0200, Paolo Bonzini wrote:
>Il 19/08/2014 11:04, Wanpeng Li ha scritto:
>> EPT misconfig handler in kvm will check which reason lead to EPT
>> misconfiguration after vmexit. One of the reasons is that an EPT
>> paging-structure entry is configured with settings reserved for
>> future functionality. However, the handler can't identify if
>> paging-structure entry of reserved bits for 1-GByte page are
>> configured, since PDPTE which point to 1-GByte page will reserve
>> bits 29:12 instead of bits 7:3 which are reserved for PDPTE that
>> references an EPT Page Directory. This patch fix it by reserve
>> bits 29:12 for 1-GByte page.
>>
>> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
>> ---
>> v1 -> v2:
>> * same "if" statement cover both 2MB and 1GB pages
>> * return 0xf8 for level == 4
>
>I think you dropped this check by mistake.
Indeed. I will do it in next version.
>
>> * get the level by checking the return value of ept_rsvd_mask
>>
>> arch/x86/kvm/vmx.c | 19 +++++++------------
>> 1 file changed, 7 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index cad37d5..2763f37 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -5521,17 +5521,12 @@ static u64 ept_rsvd_mask(u64 spte, int level)
>> for (i = 51; i > boot_cpu_data.x86_phys_bits; i--)
>> mask |= (1ULL << i);
>>
>> - if (level > 2)
>> - /* bits 7:3 reserved */
>> - mask |= 0xf8;
>> - else if (level == 2) {
>> - if (spte & (1ULL << 7))
>> - /* 2MB ref, bits 20:12 reserved */
>> - mask |= 0x1ff000;
>> - else
>> - /* bits 6:3 reserved */
>> - mask |= 0x78;
>> - }
>> + if (spte & (1ULL << 7))
>
>You need to go this way if level == 1 too. Otherwise, you would report
>bits 6:3 reserved if the hypervisor is using the ignored bit 7 (Table
>28-6, Format of an EPT Page-Table Entry).
>
Agreed. What still need to do here is to update the comments in order to
include level == 1, right?
>> + /* 1GB/2MB page, bits 29:12 or 20:12 reserved respectively */
>> + mask |= (PAGE_SIZE << ((level - 1) * 9)) - PAGE_SIZE;
>> + else
>> + /* bits 6:3 reserved */
>> + mask |= 0x78;
>>
>> return mask;
>> }
>> @@ -5561,7 +5556,7 @@ static void ept_misconfig_inspect_spte(struct kvm_vcpu *vcpu, u64 spte,
>> WARN_ON(1);
>> }
>>
>> - if (level == 1 || (level == 2 && (spte & (1ULL << 7)))) {
>> + if (level == 1 || (rsvd_bits & 0x38)) {
>
>- rsvd_bits will always be zero here. You need to check the return
>value of ept_rsvd_mask(). Let's call it rsvd_mask in the rest of this
>email.
>
>- the test is inverted, you need to check that bits 5:3 are _not_
>reserved, hence (rsvd_mask & 0x38) == 0.
>
>- once you do this, the test also covers level 1.
Agreed.
>
>I suggest that you write a testcase for kvm-unit-tests.
>
Ok.
Regards,
Wanpeng Li
>Paolo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] KVM: vmx: fix ept reserved bits for 1-GByte page
2014-08-19 11:16 ` Wanpeng Li
@ 2014-08-19 12:07 ` Paolo Bonzini
0 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2014-08-19 12:07 UTC (permalink / raw)
To: Wanpeng Li; +Cc: Marcelo Tosatti, Gleb Natapov, Zhang Yang, kvm, linux-kernel
Il 19/08/2014 13:16, Wanpeng Li ha scritto:
> On Tue, Aug 19, 2014 at 11:09:49AM +0200, Paolo Bonzini wrote:
>> Il 19/08/2014 11:04, Wanpeng Li ha scritto:
>>> EPT misconfig handler in kvm will check which reason lead to EPT
>>> misconfiguration after vmexit. One of the reasons is that an EPT
>>> paging-structure entry is configured with settings reserved for
>>> future functionality. However, the handler can't identify if
>>> paging-structure entry of reserved bits for 1-GByte page are
>>> configured, since PDPTE which point to 1-GByte page will reserve
>>> bits 29:12 instead of bits 7:3 which are reserved for PDPTE that
>>> references an EPT Page Directory. This patch fix it by reserve
>>> bits 29:12 for 1-GByte page.
>>>
>>> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
>>> ---
>>> v1 -> v2:
>>> * same "if" statement cover both 2MB and 1GB pages
>>> * return 0xf8 for level == 4
>>
>> I think you dropped this check by mistake.
>
> Indeed. I will do it in next version.
>
>>
>>> * get the level by checking the return value of ept_rsvd_mask
>>>
>>> arch/x86/kvm/vmx.c | 19 +++++++------------
>>> 1 file changed, 7 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>> index cad37d5..2763f37 100644
>>> --- a/arch/x86/kvm/vmx.c
>>> +++ b/arch/x86/kvm/vmx.c
>>> @@ -5521,17 +5521,12 @@ static u64 ept_rsvd_mask(u64 spte, int level)
>>> for (i = 51; i > boot_cpu_data.x86_phys_bits; i--)
>>> mask |= (1ULL << i);
>>>
>>> - if (level > 2)
>>> - /* bits 7:3 reserved */
>>> - mask |= 0xf8;
>>> - else if (level == 2) {
>>> - if (spte & (1ULL << 7))
>>> - /* 2MB ref, bits 20:12 reserved */
>>> - mask |= 0x1ff000;
>>> - else
>>> - /* bits 6:3 reserved */
>>> - mask |= 0x78;
>>> - }
>>> + if (spte & (1ULL << 7))
>>
>> You need to go this way if level == 1 too. Otherwise, you would report
>> bits 6:3 reserved if the hypervisor is using the ignored bit 7 (Table
>> 28-6, Format of an EPT Page-Table Entry).
>>
>
> Agreed. What still need to do here is to update the comments in order to
> include level == 1, right?
Yes.
>>> + /* 1GB/2MB page, bits 29:12 or 20:12 reserved respectively */
>>> + mask |= (PAGE_SIZE << ((level - 1) * 9)) - PAGE_SIZE;
>>> + else
>>> + /* bits 6:3 reserved */
>>> + mask |= 0x78;
>>>
>>> return mask;
>>> }
>>> @@ -5561,7 +5556,7 @@ static void ept_misconfig_inspect_spte(struct kvm_vcpu *vcpu, u64 spte,
>>> WARN_ON(1);
>>> }
>>>
>>> - if (level == 1 || (level == 2 && (spte & (1ULL << 7)))) {
>>> + if (level == 1 || (rsvd_bits & 0x38)) {
>>
>> - rsvd_bits will always be zero here. You need to check the return
>> value of ept_rsvd_mask(). Let's call it rsvd_mask in the rest of this
>> email.
>>
>> - the test is inverted, you need to check that bits 5:3 are _not_
>> reserved, hence (rsvd_mask & 0x38) == 0.
>>
>> - once you do this, the test also covers level 1.
>
> Agreed.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] KVM: x86: #GP when attempts to write reserved bits of Variable Range MTRRs
2014-08-19 9:04 ` [PATCH v2 3/3] KVM: x86: #GP when attempts to write reserved bits " Wanpeng Li
2014-08-19 9:11 ` Paolo Bonzini
@ 2014-08-29 16:47 ` Paolo Bonzini
2014-08-29 16:52 ` Jan Kiszka
2014-09-01 0:22 ` Wanpeng Li
1 sibling, 2 replies; 10+ messages in thread
From: Paolo Bonzini @ 2014-08-29 16:47 UTC (permalink / raw)
To: Wanpeng Li
Cc: Marcelo Tosatti, Gleb Natapov, Zhang Yang, kvm, linux-kernel,
Jan Kiszka
Il 19/08/2014 11:04, Wanpeng Li ha scritto:
> Section 11.11.2.3 of the SDM mentions "All other bits in the IA32_MTRR_PHYSBASEn
> and IA32_MTRR_PHYSMASKn registers are reserved; the processor generates a
> general-protection exception(#GP) if software attempts to write to them". This
> patch do it in kvm.
>
> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
This breaks if the guest maxphyaddr is higher than the host's (which
sometimes happens depending on your hardware and how QEMU is
configured).
You need to use cpuid_maxphyaddr, like this
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index a375dfc42f6a..916e89515210 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1726,7 +1726,7 @@ static bool valid_mtrr_type(unsigned t)
static bool mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
{
int i;
- u64 mask = 0;
+ u64 mask;
if (!msr_mtrr_valid(msr))
return false;
@@ -1750,8 +1750,7 @@ static bool mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
/* variable MTRRs */
WARN_ON(!(msr >= 0x200 && msr < 0x200 + 2 * KVM_NR_VAR_MTRR));
- for (i = 63; i > boot_cpu_data.x86_phys_bits; i--)
- mask |= (1ULL << i);
+ mask = (~0ULL) << cpuid_maxphyaddr(vcpu);
if ((msr & 1) == 0) {
/* MTRR base */
if (!valid_mtrr_type(data & 0xff))
Jan, can you see if this patch fixes the SeaBIOS triple fault you reported?
Paolo
> ---
> arch/x86/kvm/x86.c | 18 +++++++++++++++---
> 1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index fb3ea7a..b85da5f 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1726,6 +1726,7 @@ static bool valid_mtrr_type(unsigned t)
> static bool mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> {
> int i;
> + u64 mask = 0;
>
> if (!msr_mtrr_valid(msr))
> return false;
> @@ -1749,10 +1750,21 @@ static bool mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> /* variable MTRRs */
> WARN_ON(!(msr >= 0x200 && msr < 0x200 + 2 * KVM_NR_VAR_MTRR));
>
> - if ((msr & 1) == 0)
> + for (i = 63; i > boot_cpu_data.x86_phys_bits; i--)
> + mask |= (1ULL << i);
> + if ((msr & 1) == 0) {
> /* MTRR base */
> - return valid_mtrr_type(data & 0xff);
> - /* MTRR mask */
> + if (!valid_mtrr_type(data & 0xff))
> + return false;
> + mask |= 0xf00;
> + } else
> + /* MTRR mask */
> + mask |= 0x7ff;
> + if (data & mask) {
> + kvm_inject_gp(vcpu, 0);
> + return false;
> + }
> +
> return true;
> }
>
>
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] KVM: x86: #GP when attempts to write reserved bits of Variable Range MTRRs
2014-08-29 16:47 ` Paolo Bonzini
@ 2014-08-29 16:52 ` Jan Kiszka
2014-09-01 0:22 ` Wanpeng Li
1 sibling, 0 replies; 10+ messages in thread
From: Jan Kiszka @ 2014-08-29 16:52 UTC (permalink / raw)
To: Paolo Bonzini, Wanpeng Li
Cc: Marcelo Tosatti, Gleb Natapov, Zhang Yang, kvm, linux-kernel
On 2014-08-29 18:47, Paolo Bonzini wrote:
> Il 19/08/2014 11:04, Wanpeng Li ha scritto:
>> Section 11.11.2.3 of the SDM mentions "All other bits in the IA32_MTRR_PHYSBASEn
>> and IA32_MTRR_PHYSMASKn registers are reserved; the processor generates a
>> general-protection exception(#GP) if software attempts to write to them". This
>> patch do it in kvm.
>>
>> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
>
> This breaks if the guest maxphyaddr is higher than the host's (which
> sometimes happens depending on your hardware and how QEMU is
> configured).
>
> You need to use cpuid_maxphyaddr, like this
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index a375dfc42f6a..916e89515210 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1726,7 +1726,7 @@ static bool valid_mtrr_type(unsigned t)
> static bool mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> {
> int i;
> - u64 mask = 0;
> + u64 mask;
>
> if (!msr_mtrr_valid(msr))
> return false;
> @@ -1750,8 +1750,7 @@ static bool mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> /* variable MTRRs */
> WARN_ON(!(msr >= 0x200 && msr < 0x200 + 2 * KVM_NR_VAR_MTRR));
>
> - for (i = 63; i > boot_cpu_data.x86_phys_bits; i--)
> - mask |= (1ULL << i);
> + mask = (~0ULL) << cpuid_maxphyaddr(vcpu);
> if ((msr & 1) == 0) {
> /* MTRR base */
> if (!valid_mtrr_type(data & 0xff))
>
>
> Jan, can you see if this patch fixes the SeaBIOS triple fault you reported?
Yep, it does.
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] KVM: x86: #GP when attempts to write reserved bits of Variable Range MTRRs
2014-08-29 16:47 ` Paolo Bonzini
2014-08-29 16:52 ` Jan Kiszka
@ 2014-09-01 0:22 ` Wanpeng Li
1 sibling, 0 replies; 10+ messages in thread
From: Wanpeng Li @ 2014-09-01 0:22 UTC (permalink / raw)
To: Paolo Bonzini, Jan Kiszka
Cc: Marcelo Tosatti, Gleb Natapov, Zhang Yang, kvm, linux-kernel
On Fri, Aug 29, 2014 at 06:47:54PM +0200, Paolo Bonzini wrote:
>Il 19/08/2014 11:04, Wanpeng Li ha scritto:
>> Section 11.11.2.3 of the SDM mentions "All other bits in the IA32_MTRR_PHYSBASEn
>> and IA32_MTRR_PHYSMASKn registers are reserved; the processor generates a
>> general-protection exception(#GP) if software attempts to write to them". This
>> patch do it in kvm.
>>
>> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
>
>This breaks if the guest maxphyaddr is higher than the host's (which
>sometimes happens depending on your hardware and how QEMU is
>configured).
>
>You need to use cpuid_maxphyaddr, like this
>
>diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>index a375dfc42f6a..916e89515210 100644
>--- a/arch/x86/kvm/x86.c
>+++ b/arch/x86/kvm/x86.c
>@@ -1726,7 +1726,7 @@ static bool valid_mtrr_type(unsigned t)
> static bool mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> {
> int i;
>- u64 mask = 0;
>+ u64 mask;
>
> if (!msr_mtrr_valid(msr))
> return false;
>@@ -1750,8 +1750,7 @@ static bool mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> /* variable MTRRs */
> WARN_ON(!(msr >= 0x200 && msr < 0x200 + 2 * KVM_NR_VAR_MTRR));
>
>- for (i = 63; i > boot_cpu_data.x86_phys_bits; i--)
>- mask |= (1ULL << i);
>+ mask = (~0ULL) << cpuid_maxphyaddr(vcpu);
> if ((msr & 1) == 0) {
> /* MTRR base */
> if (!valid_mtrr_type(data & 0xff))
>
>
Got it, thanks Paolo and Jan.
Regards,
Wanpeng Li
>Jan, can you see if this patch fixes the SeaBIOS triple fault you reported?
>
>Paolo
>
>> ---
>> arch/x86/kvm/x86.c | 18 +++++++++++++++---
>> 1 file changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index fb3ea7a..b85da5f 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -1726,6 +1726,7 @@ static bool valid_mtrr_type(unsigned t)
>> static bool mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
>> {
>> int i;
>> + u64 mask = 0;
>>
>> if (!msr_mtrr_valid(msr))
>> return false;
>> @@ -1749,10 +1750,21 @@ static bool mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
>> /* variable MTRRs */
>> WARN_ON(!(msr >= 0x200 && msr < 0x200 + 2 * KVM_NR_VAR_MTRR));
>>
>> - if ((msr & 1) == 0)
>> + for (i = 63; i > boot_cpu_data.x86_phys_bits; i--)
>> + mask |= (1ULL << i);
>> + if ((msr & 1) == 0) {
>> /* MTRR base */
>> - return valid_mtrr_type(data & 0xff);
>> - /* MTRR mask */
>> + if (!valid_mtrr_type(data & 0xff))
>> + return false;
>> + mask |= 0xf00;
>> + } else
>> + /* MTRR mask */
>> + mask |= 0x7ff;
>> + if (data & mask) {
>> + kvm_inject_gp(vcpu, 0);
>> + return false;
>> + }
>> +
>> return true;
>> }
>>
>>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-09-01 0:20 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-19 9:04 [PATCH v2 1/3] KVM: vmx: fix ept reserved bits for 1-GByte page Wanpeng Li
2014-08-19 9:04 ` [PATCH v2 2/3] KVM: x86: fix check legal type of Variable Range MTRRs Wanpeng Li
2014-08-19 9:04 ` [PATCH v2 3/3] KVM: x86: #GP when attempts to write reserved bits " Wanpeng Li
2014-08-19 9:11 ` Paolo Bonzini
2014-08-29 16:47 ` Paolo Bonzini
2014-08-29 16:52 ` Jan Kiszka
2014-09-01 0:22 ` Wanpeng Li
2014-08-19 9:09 ` [PATCH v2 1/3] KVM: vmx: fix ept reserved bits for 1-GByte page Paolo Bonzini
2014-08-19 11:16 ` Wanpeng Li
2014-08-19 12:07 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox