[PATCH 1/2] KVM: nVMX: Fix nested_vmx_check_msr_bitmap_controls

[PATCH 1/2] KVM: nVMX: Fix nested_vmx_check_msr_bitmap_controls

[Jim Mattson]

Allow the L1 guest to specify the last page of addressable guest
physical memory for an L2 MSR permission bitmap.

Fixes: 3af18d9c5fe95 ("KVM: nVMX: Prepare for using hardware MSR bitmap")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/vmx.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index c6f4ad44aa95..ae641c8d4284 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -4624,6 +4624,11 @@ static bool guest_state_valid(struct kvm_vcpu *vcpu)
 	return true;
 }
 
+static bool page_address_valid(struct kvm_vcpu *vcpu, gpa_t gpa)
+{
+	return PAGE_ALIGNED(gpa) && !(gpa >> cpuid_maxphyaddr(vcpu));
+}
+
 static int init_rmode_tss(struct kvm *kvm)
 {
 	gfn_t fn;
@@ -9582,9 +9587,6 @@ static void vmx_start_preemption_timer(struct kvm_vcpu *vcpu)
 static int nested_vmx_check_msr_bitmap_controls(struct kvm_vcpu *vcpu,
 						struct vmcs12 *vmcs12)
 {
-	int maxphyaddr;
-	u64 addr;
-
 	if (!nested_cpu_has(vmcs12, CPU_BASED_USE_MSR_BITMAPS))
 		return 0;
 
@@ -9592,10 +9594,8 @@ static int nested_vmx_check_msr_bitmap_controls(struct kvm_vcpu *vcpu,
 		WARN_ON(1);
 		return -EINVAL;
 	}
-	maxphyaddr = cpuid_maxphyaddr(vcpu);
 
-	if (!PAGE_ALIGNED(vmcs12->msr_bitmap) ||
-	   ((addr + PAGE_SIZE) >> maxphyaddr))
+	if (!page_address_valid(vcpu, vmcs12->msr_bitmap))
 		return -EINVAL;
 
 	return 0;
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 2/2] KVM: nVMX: Check host support for MSR bitmaps

[Jim Mattson]

If the host doesn't support MSR bitmaps, but the L2 guest is
configured to use MSR bitmaps, fail the emulated VM-entry.

Fixes: 3af18d9c5fe95 ("KVM: nVMX: Prepare for using hardware MSR bitmap")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/vmx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index ae641c8d4284..639188037744 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -9590,7 +9590,7 @@ static int nested_vmx_check_msr_bitmap_controls(struct kvm_vcpu *vcpu,
 	if (!nested_cpu_has(vmcs12, CPU_BASED_USE_MSR_BITMAPS))
 		return 0;
 
-	if (vmcs12_read_any(vcpu, MSR_BITMAP, &addr)) {
+	if (!cpu_has_vmx_msr_bitmap()) {
 		WARN_ON(1);
 		return -EINVAL;
 	}
-- 
2.13.0.219.gdb65acc882-goog

Re: [PATCH 1/2] KVM: nVMX: Fix nested_vmx_check_msr_bitmap_controls

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2290 bytes --]

Hi Jim,

[auto build test ERROR on kvm/linux-next]
[also build test ERROR on v4.12-rc2 next-20170525]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jim-Mattson/KVM-nVMX-Fix-nested_vmx_check_msr_bitmap_controls/20170526-042341
base:   https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next
config: x86_64-randconfig-x018-201721 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: the linux-review/Jim-Mattson/KVM-nVMX-Fix-nested_vmx_check_msr_bitmap_controls/20170526-042341 HEAD 23051b8d9722709205bf8450e1d1a18a0cd48a05 builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

   arch/x86/kvm/vmx.c: In function 'nested_vmx_check_msr_bitmap_controls':
>> arch/x86/kvm/vmx.c:9593:41: error: 'addr' undeclared (first use in this function)
     if (vmcs12_read_any(vcpu, MSR_BITMAP, &addr)) {
                                            ^~~~
   arch/x86/kvm/vmx.c:9593:41: note: each undeclared identifier is reported only once for each function it appears in

vim +/addr +9593 arch/x86/kvm/vmx.c

3af18d9c Wincy Van 2015-02-03  9587  static int nested_vmx_check_msr_bitmap_controls(struct kvm_vcpu *vcpu,
3af18d9c Wincy Van 2015-02-03  9588  						struct vmcs12 *vmcs12)
3af18d9c Wincy Van 2015-02-03  9589  {
3af18d9c Wincy Van 2015-02-03  9590  	if (!nested_cpu_has(vmcs12, CPU_BASED_USE_MSR_BITMAPS))
3af18d9c Wincy Van 2015-02-03  9591  		return 0;
3af18d9c Wincy Van 2015-02-03  9592  
3af18d9c Wincy Van 2015-02-03 @9593  	if (vmcs12_read_any(vcpu, MSR_BITMAP, &addr)) {
3af18d9c Wincy Van 2015-02-03  9594  		WARN_ON(1);
3af18d9c Wincy Van 2015-02-03  9595  		return -EINVAL;
3af18d9c Wincy Van 2015-02-03  9596  	}

:::::: The code at line 9593 was first introduced by commit
:::::: 3af18d9c5fe95a6b377dca7b9ff9c6d3ab7f0969 KVM: nVMX: Prepare for using hardware MSR bitmap

:::::: TO: Wincy Van <fanwenyi0529@gmail.com>
:::::: CC: Paolo Bonzini <pbonzini@redhat.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26997 bytes --]

[PATCH v2 1/2] KVM: nVMX: Fix nested_vmx_check_msr_bitmap_controls

[Jim Mattson]

Allow the L1 guest to specify the last page of addressable guest
physical memory for an L2 MSR permission bitmap.

Fixes: 3af18d9c5fe95 ("KVM: nVMX: Prepare for using hardware MSR bitmap")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/vmx.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index c6f4ad44aa95..0d72b4064a80 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -4624,6 +4624,11 @@ static bool guest_state_valid(struct kvm_vcpu *vcpu)
 	return true;
 }
 
+static bool page_address_valid(struct kvm_vcpu *vcpu, gpa_t gpa)
+{
+	return PAGE_ALIGNED(gpa) && !(gpa >> cpuid_maxphyaddr(vcpu));
+}
+
 static int init_rmode_tss(struct kvm *kvm)
 {
 	gfn_t fn;
@@ -9582,7 +9587,6 @@ static void vmx_start_preemption_timer(struct kvm_vcpu *vcpu)
 static int nested_vmx_check_msr_bitmap_controls(struct kvm_vcpu *vcpu,
 						struct vmcs12 *vmcs12)
 {
-	int maxphyaddr;
 	u64 addr;
 
 	if (!nested_cpu_has(vmcs12, CPU_BASED_USE_MSR_BITMAPS))
@@ -9592,10 +9596,8 @@ static int nested_vmx_check_msr_bitmap_controls(struct kvm_vcpu *vcpu,
 		WARN_ON(1);
 		return -EINVAL;
 	}
-	maxphyaddr = cpuid_maxphyaddr(vcpu);
 
-	if (!PAGE_ALIGNED(vmcs12->msr_bitmap) ||
-	   ((addr + PAGE_SIZE) >> maxphyaddr))
+	if (!page_address_valid(vcpu, vmcs12->msr_bitmap))
 		return -EINVAL;
 
 	return 0;
-- 
2.13.0.219.gdb65acc882-goog

[PATCH v2 2/2] KVM: nVMX: Check host support for MSR bitmaps

[Jim Mattson]

If the host doesn't support MSR bitmaps, but the L2 guest is
configured to use MSR bitmaps, fail the emulated VM-entry.

Fixes: 3af18d9c5fe95 ("KVM: nVMX: Prepare for using hardware MSR bitmap")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/vmx.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 0d72b4064a80..639188037744 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -9587,12 +9587,10 @@ static void vmx_start_preemption_timer(struct kvm_vcpu *vcpu)
 static int nested_vmx_check_msr_bitmap_controls(struct kvm_vcpu *vcpu,
 						struct vmcs12 *vmcs12)
 {
-	u64 addr;
-
 	if (!nested_cpu_has(vmcs12, CPU_BASED_USE_MSR_BITMAPS))
 		return 0;
 
-	if (vmcs12_read_any(vcpu, MSR_BITMAP, &addr)) {
+	if (!cpu_has_vmx_msr_bitmap()) {
 		WARN_ON(1);
 		return -EINVAL;
 	}
-- 
2.13.0.219.gdb65acc882-goog

Re: [PATCH v2 2/2] KVM: nVMX: Check host support for MSR bitmaps

[Radim Krčmář]

2017-05-25 17:04-0700, Jim Mattson:
> If the host doesn't support MSR bitmaps, but the L2 guest is
> configured to use MSR bitmaps, fail the emulated VM-entry.
> 
> Fixes: 3af18d9c5fe95 ("KVM: nVMX: Prepare for using hardware MSR bitmap")
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/kvm/vmx.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 0d72b4064a80..639188037744 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -9587,12 +9587,10 @@ static void vmx_start_preemption_timer(struct kvm_vcpu *vcpu)
>  static int nested_vmx_check_msr_bitmap_controls(struct kvm_vcpu *vcpu,
>  						struct vmcs12 *vmcs12)
>  {
> -	u64 addr;
> -
>  	if (!nested_cpu_has(vmcs12, CPU_BASED_USE_MSR_BITMAPS))
>  		return 0;
>  
> -	if (vmcs12_read_any(vcpu, MSR_BITMAP, &addr)) {
> +	if (!cpu_has_vmx_msr_bitmap()) {

VMCS bitmaps are emulated and exposed regardless of CPU features.
Doesn't this change break L1 hypervisor on CPUs that don't support MSR
bitmaps?

I think the check can just go away and can be merged into the first
patch.

Thanks.

>  		WARN_ON(1);
>  		return -EINVAL;
>  	}
> -- 
> 2.13.0.219.gdb65acc882-goog
> 

[PATCH v2] KVM: nVMX: Fix nested_vmx_check_msr_bitmap_controls

[Jim Mattson]

Allow the L1 guest to specify the last page of addressable guest
physical memory for an L2 MSR permission bitmap. Also remove the
vmcs12_read_any() check that should never fail.

Fixes: 3af18d9c5fe95 ("KVM: nVMX: Prepare for using hardware MSR bitmap")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/vmx.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 60fa010d3fa1..8b403682dcdf 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -4624,6 +4624,11 @@ static bool guest_state_valid(struct kvm_vcpu *vcpu)
 	return true;
 }
 
+static bool page_address_valid(struct kvm_vcpu *vcpu, gpa_t gpa)
+{
+	return PAGE_ALIGNED(gpa) && !(gpa >> cpuid_maxphyaddr(vcpu));
+}
+
 static int init_rmode_tss(struct kvm *kvm)
 {
 	gfn_t fn;
@@ -9559,20 +9564,10 @@ static void vmx_start_preemption_timer(struct kvm_vcpu *vcpu)
 static int nested_vmx_check_msr_bitmap_controls(struct kvm_vcpu *vcpu,
 						struct vmcs12 *vmcs12)
 {
-	int maxphyaddr;
-	u64 addr;
-
 	if (!nested_cpu_has(vmcs12, CPU_BASED_USE_MSR_BITMAPS))
 		return 0;
 
-	if (vmcs12_read_any(vcpu, MSR_BITMAP, &addr)) {
-		WARN_ON(1);
-		return -EINVAL;
-	}
-	maxphyaddr = cpuid_maxphyaddr(vcpu);
-
-	if (!PAGE_ALIGNED(vmcs12->msr_bitmap) ||
-	   ((addr + PAGE_SIZE) >> maxphyaddr))
+	if (!page_address_valid(vcpu, vmcs12->msr_bitmap))
 		return -EINVAL;
 
 	return 0;
-- 
2.13.1.518.g3df882009-goog