[PATCH 1/2] KVM: VMX: remove I/O port 0x80 bypass on Intel hosts

public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH 1/2] KVM: VMX: remove I/O port 0x80 bypass on Intel hosts
@ 2017-12-01 18:21 Jim Mattson
  2017-12-01 18:21 ` [PATCH 2/2] KVM: VMX: Use just one page for I/O permission bitmaps Jim Mattson
                   ` (3 more replies)
  0 siblings, 4 replies; 20+ messages in thread
From: Jim Mattson @ 2017-12-01 18:21 UTC (permalink / raw)
  To: kvm, P J P; +Cc: Andrew Honig, Jim Mattson

From: Andrew Honig <ahonig@google.com>

This fixes CVE-2017-1000407.

KVM allows guests to directly access I/O port 0x80 on Intel hosts.  If
the guest floods this port with writes it generates exceptions and
instability in the host kernel, leading to a crash.  With this change
guest writes to port 0x80 on Intel will behave the same as they
currently behave on AMD systems.

Prevent the flooding by removing the code that sets port 0x80 as a
passthrough port.  This is essentially the same as upstream patch
99f85a28a78e96d28907fe036e1671a218fee597, except that patch was
for AMD chipsets and this patch is for Intel.

Signed-off-by: Andrew Honig <ahonig@google.com>
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/vmx.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index d2b452d66363..d16abd1808eb 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -6753,12 +6753,7 @@ static __init int hardware_setup(void)
 	memset(vmx_vmread_bitmap, 0xff, PAGE_SIZE);
 	memset(vmx_vmwrite_bitmap, 0xff, PAGE_SIZE);
 
-	/*
-	 * Allow direct access to the PC debug port (it is often used for I/O
-	 * delays, but the vmexits simply slow things down).
-	 */
 	memset(vmx_io_bitmap_a, 0xff, PAGE_SIZE);
-	clear_bit(0x80, vmx_io_bitmap_a);
 
 	memset(vmx_io_bitmap_b, 0xff, PAGE_SIZE);
 
-- 
2.15.0.531.g2ccb3012c9-goog

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/2] KVM: VMX: Use just one page for I/O permission bitmaps
  2017-12-01 18:21 [PATCH 1/2] KVM: VMX: remove I/O port 0x80 bypass on Intel hosts Jim Mattson
@ 2017-12-01 18:21 ` Jim Mattson
  2017-12-04 18:30   ` kbuild test robot
  2017-12-05 21:26   ` Radim Krčmář
  2017-12-02  0:34 ` [PATCH 1/2] KVM: VMX: remove I/O port 0x80 bypass on Intel hosts Krish Sadhukhan
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 20+ messages in thread
From: Jim Mattson @ 2017-12-01 18:21 UTC (permalink / raw)
  To: kvm, P J P; +Cc: Jim Mattson

Since we no longer allow any I/O ports to be passed through to the guest,
we can use the same page for I/O bitmap A and I/O bitmap B.

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/vmx.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index d16abd1808eb..0c216a137b02 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -943,8 +943,7 @@ static DEFINE_PER_CPU(struct list_head, blocked_vcpu_on_cpu);
 static DEFINE_PER_CPU(spinlock_t, blocked_vcpu_on_cpu_lock);
 
 enum {
-	VMX_IO_BITMAP_A,
-	VMX_IO_BITMAP_B,
+	VMX_IO_BITMAP,
 	VMX_MSR_BITMAP_LEGACY,
 	VMX_MSR_BITMAP_LONGMODE,
 	VMX_MSR_BITMAP_LEGACY_X2APIC_APICV,
@@ -958,8 +957,7 @@ enum {
 
 static unsigned long *vmx_bitmap[VMX_BITMAP_NR];
 
-#define vmx_io_bitmap_a                      (vmx_bitmap[VMX_IO_BITMAP_A])
-#define vmx_io_bitmap_b                      (vmx_bitmap[VMX_IO_BITMAP_B])
+#define vmx_io_bitmap                        (vmx_bitmap[VMX_IO_BITMAP])
 #define vmx_msr_bitmap_legacy                (vmx_bitmap[VMX_MSR_BITMAP_LEGACY])
 #define vmx_msr_bitmap_longmode              (vmx_bitmap[VMX_MSR_BITMAP_LONGMODE])
 #define vmx_msr_bitmap_legacy_x2apic_apicv   (vmx_bitmap[VMX_MSR_BITMAP_LEGACY_X2APIC_APICV])
@@ -5444,8 +5442,8 @@ static void vmx_vcpu_setup(struct vcpu_vmx *vmx)
 	int i;
 
 	/* I/O */
-	vmcs_write64(IO_BITMAP_A, __pa(vmx_io_bitmap_a));
-	vmcs_write64(IO_BITMAP_B, __pa(vmx_io_bitmap_b));
+	vmcs_write64(IO_BITMAP_A, __pa(vmx_io_bitmap));
+	vmcs_write64(IO_BITMAP_B, __pa(vmx_io_bitmap));
 
 	if (enable_shadow_vmcs) {
 		vmcs_write64(VMREAD_BITMAP, __pa(vmx_vmread_bitmap));
@@ -6753,9 +6751,7 @@ static __init int hardware_setup(void)
 	memset(vmx_vmread_bitmap, 0xff, PAGE_SIZE);
 	memset(vmx_vmwrite_bitmap, 0xff, PAGE_SIZE);
 
-	memset(vmx_io_bitmap_a, 0xff, PAGE_SIZE);
-
-	memset(vmx_io_bitmap_b, 0xff, PAGE_SIZE);
+	memset(vmx_io_bitmap, 0xff, PAGE_SIZE);
 
 	memset(vmx_msr_bitmap_legacy, 0xff, PAGE_SIZE);
 	memset(vmx_msr_bitmap_longmode, 0xff, PAGE_SIZE);
-- 
2.15.0.531.g2ccb3012c9-goog

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] KVM: VMX: Use just one page for I/O permission bitmaps
  2017-12-01 18:21 ` [PATCH 2/2] KVM: VMX: Use just one page for I/O permission bitmaps Jim Mattson
@ 2017-12-04 18:30   ` kbuild test robot
  2017-12-04 18:34     ` Jim Mattson
  2017-12-05 21:26   ` Radim Krčmář
  1 sibling, 1 reply; 20+ messages in thread
From: kbuild test robot @ 2017-12-04 18:30 UTC (permalink / raw)
  To: Jim Mattson; +Cc: kbuild-all, kvm, P J P, Jim Mattson

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

Hi Jim,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on kvm/linux-next]
[also build test ERROR on v4.15-rc2 next-20171204]
[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-VMX-remove-I-O-port-0x80-bypass-on-Intel-hosts/20171205-013621
base:   https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   arch/x86/kvm/vmx.c: In function 'hardware_setup':
>> arch/x86/kvm/vmx.c:6751:2: error: 'vmx_io_bitmap_b' undeclared (first use in this function); did you mean 'vmx_io_bitmap'?
     vmx_io_bitmap_b = (unsigned long *)__get_free_page(GFP_KERNEL);
     ^~~~~~~~~~~~~~~
     vmx_io_bitmap
   arch/x86/kvm/vmx.c:6751:2: note: each undeclared identifier is reported only once for each function it appears in

vim +6751 arch/x86/kvm/vmx.c

f160c7b7bb Junaid Shahid     2016-12-06  6735  
f2c7648d91 Tiejun Chen       2014-10-28  6736  static __init int hardware_setup(void)
f2c7648d91 Tiejun Chen       2014-10-28  6737  {
34a1cd60d1 Tiejun Chen       2014-10-28  6738  	int r = -ENOMEM, i, msr;
34a1cd60d1 Tiejun Chen       2014-10-28  6739  
34a1cd60d1 Tiejun Chen       2014-10-28  6740  	rdmsrl_safe(MSR_EFER, &host_efer);
34a1cd60d1 Tiejun Chen       2014-10-28  6741  
34a1cd60d1 Tiejun Chen       2014-10-28  6742  	for (i = 0; i < ARRAY_SIZE(vmx_msr_index); ++i)
34a1cd60d1 Tiejun Chen       2014-10-28  6743  		kvm_define_shared_msr(i, vmx_msr_index[i]);
34a1cd60d1 Tiejun Chen       2014-10-28  6744  
2361133293 Radim Krčmář      2016-09-29  6745  	for (i = 0; i < VMX_BITMAP_NR; i++) {
2361133293 Radim Krčmář      2016-09-29  6746  		vmx_bitmap[i] = (unsigned long *)__get_free_page(GFP_KERNEL);
2361133293 Radim Krčmář      2016-09-29  6747  		if (!vmx_bitmap[i])
34a1cd60d1 Tiejun Chen       2014-10-28  6748  			goto out;
2361133293 Radim Krčmář      2016-09-29  6749  	}
34a1cd60d1 Tiejun Chen       2014-10-28  6750  
2361133293 Radim Krčmář      2016-09-29 @6751  	vmx_io_bitmap_b = (unsigned long *)__get_free_page(GFP_KERNEL);
34a1cd60d1 Tiejun Chen       2014-10-28  6752  	memset(vmx_vmread_bitmap, 0xff, PAGE_SIZE);
34a1cd60d1 Tiejun Chen       2014-10-28  6753  	memset(vmx_vmwrite_bitmap, 0xff, PAGE_SIZE);
34a1cd60d1 Tiejun Chen       2014-10-28  6754  
4a290f4464 Jim Mattson       2017-12-01  6755  	memset(vmx_io_bitmap, 0xff, PAGE_SIZE);
34a1cd60d1 Tiejun Chen       2014-10-28  6756  
34a1cd60d1 Tiejun Chen       2014-10-28  6757  	memset(vmx_msr_bitmap_legacy, 0xff, PAGE_SIZE);
34a1cd60d1 Tiejun Chen       2014-10-28  6758  	memset(vmx_msr_bitmap_longmode, 0xff, PAGE_SIZE);
34a1cd60d1 Tiejun Chen       2014-10-28  6759  
34a1cd60d1 Tiejun Chen       2014-10-28  6760  	if (setup_vmcs_config(&vmcs_config) < 0) {
34a1cd60d1 Tiejun Chen       2014-10-28  6761  		r = -EIO;
2361133293 Radim Krčmář      2016-09-29  6762  		goto out;
34a1cd60d1 Tiejun Chen       2014-10-28  6763  	}
f2c7648d91 Tiejun Chen       2014-10-28  6764  
f2c7648d91 Tiejun Chen       2014-10-28  6765  	if (boot_cpu_has(X86_FEATURE_NX))
f2c7648d91 Tiejun Chen       2014-10-28  6766  		kvm_enable_efer_bits(EFER_NX);
f2c7648d91 Tiejun Chen       2014-10-28  6767  
08d839c4b1 Wanpeng Li        2017-03-23  6768  	if (!cpu_has_vmx_vpid() || !cpu_has_vmx_invvpid() ||
08d839c4b1 Wanpeng Li        2017-03-23  6769  		!(cpu_has_vmx_invvpid_single() || cpu_has_vmx_invvpid_global()))
f2c7648d91 Tiejun Chen       2014-10-28  6770  		enable_vpid = 0;
08d839c4b1 Wanpeng Li        2017-03-23  6771  
f2c7648d91 Tiejun Chen       2014-10-28  6772  	if (!cpu_has_vmx_shadow_vmcs())
f2c7648d91 Tiejun Chen       2014-10-28  6773  		enable_shadow_vmcs = 0;
f2c7648d91 Tiejun Chen       2014-10-28  6774  	if (enable_shadow_vmcs)
f2c7648d91 Tiejun Chen       2014-10-28  6775  		init_vmcs_shadow_fields();
f2c7648d91 Tiejun Chen       2014-10-28  6776  
f2c7648d91 Tiejun Chen       2014-10-28  6777  	if (!cpu_has_vmx_ept() ||
42aa53b4e1 David Hildenbrand 2017-08-10  6778  	    !cpu_has_vmx_ept_4levels() ||
f5f51586db David Hildenbrand 2017-08-24  6779  	    !cpu_has_vmx_ept_mt_wb() ||
8ad8182e93 Wanpeng Li        2017-10-09  6780  	    !cpu_has_vmx_invept_global())
f2c7648d91 Tiejun Chen       2014-10-28  6781  		enable_ept = 0;
f2c7648d91 Tiejun Chen       2014-10-28  6782  
fce6ac4c05 Wanpeng Li        2017-05-11  6783  	if (!cpu_has_vmx_ept_ad_bits() || !enable_ept)
f2c7648d91 Tiejun Chen       2014-10-28  6784  		enable_ept_ad_bits = 0;
f2c7648d91 Tiejun Chen       2014-10-28  6785  
8ad8182e93 Wanpeng Li        2017-10-09  6786  	if (!cpu_has_vmx_unrestricted_guest() || !enable_ept)
f2c7648d91 Tiejun Chen       2014-10-28  6787  		enable_unrestricted_guest = 0;
f2c7648d91 Tiejun Chen       2014-10-28  6788  
ad15a29647 Paolo Bonzini     2015-01-30  6789  	if (!cpu_has_vmx_flexpriority())
f2c7648d91 Tiejun Chen       2014-10-28  6790  		flexpriority_enabled = 0;
f2c7648d91 Tiejun Chen       2014-10-28  6791  
d02fcf5077 Paolo Bonzini     2017-11-06  6792  	if (!cpu_has_virtual_nmis())
d02fcf5077 Paolo Bonzini     2017-11-06  6793  		enable_vnmi = 0;
d02fcf5077 Paolo Bonzini     2017-11-06  6794  
f2c7648d91 Tiejun Chen       2014-10-28  6795  	/*
f2c7648d91 Tiejun Chen       2014-10-28  6796  	 * set_apic_access_page_addr() is used to reload apic access
ad15a29647 Paolo Bonzini     2015-01-30  6797  	 * page upon invalidation.  No need to do anything if not
ad15a29647 Paolo Bonzini     2015-01-30  6798  	 * using the APIC_ACCESS_ADDR VMCS field.
f2c7648d91 Tiejun Chen       2014-10-28  6799  	 */
ad15a29647 Paolo Bonzini     2015-01-30  6800  	if (!flexpriority_enabled)
f2c7648d91 Tiejun Chen       2014-10-28  6801  		kvm_x86_ops->set_apic_access_page_addr = NULL;
f2c7648d91 Tiejun Chen       2014-10-28  6802  
f2c7648d91 Tiejun Chen       2014-10-28  6803  	if (!cpu_has_vmx_tpr_shadow())
f2c7648d91 Tiejun Chen       2014-10-28  6804  		kvm_x86_ops->update_cr8_intercept = NULL;
f2c7648d91 Tiejun Chen       2014-10-28  6805  
f2c7648d91 Tiejun Chen       2014-10-28  6806  	if (enable_ept && !cpu_has_vmx_ept_2m_page())
f2c7648d91 Tiejun Chen       2014-10-28  6807  		kvm_disable_largepages();
f2c7648d91 Tiejun Chen       2014-10-28  6808  
0f107682cb Wanpeng Li        2017-09-28  6809  	if (!cpu_has_vmx_ple()) {
f2c7648d91 Tiejun Chen       2014-10-28  6810  		ple_gap = 0;
0f107682cb Wanpeng Li        2017-09-28  6811  		ple_window = 0;
0f107682cb Wanpeng Li        2017-09-28  6812  		ple_window_grow = 0;
0f107682cb Wanpeng Li        2017-09-28  6813  		ple_window_max = 0;
0f107682cb Wanpeng Li        2017-09-28  6814  		ple_window_shrink = 0;
0f107682cb Wanpeng Li        2017-09-28  6815  	}
f2c7648d91 Tiejun Chen       2014-10-28  6816  
76dfafd536 Paolo Bonzini     2016-12-19  6817  	if (!cpu_has_vmx_apicv()) {
f2c7648d91 Tiejun Chen       2014-10-28  6818  		enable_apicv = 0;
76dfafd536 Paolo Bonzini     2016-12-19  6819  		kvm_x86_ops->sync_pir_to_irr = NULL;
76dfafd536 Paolo Bonzini     2016-12-19  6820  	}
f2c7648d91 Tiejun Chen       2014-10-28  6821  
64903d6195 Haozhong Zhang    2015-10-20  6822  	if (cpu_has_vmx_tsc_scaling()) {
64903d6195 Haozhong Zhang    2015-10-20  6823  		kvm_has_tsc_control = true;
64903d6195 Haozhong Zhang    2015-10-20  6824  		kvm_max_tsc_scaling_ratio = KVM_VMX_TSC_MULTIPLIER_MAX;
64903d6195 Haozhong Zhang    2015-10-20  6825  		kvm_tsc_scaling_ratio_frac_bits = 48;
64903d6195 Haozhong Zhang    2015-10-20  6826  	}
64903d6195 Haozhong Zhang    2015-10-20  6827  
baa035227b Tiejun Chen       2014-12-23  6828  	vmx_disable_intercept_for_msr(MSR_FS_BASE, false);
baa035227b Tiejun Chen       2014-12-23  6829  	vmx_disable_intercept_for_msr(MSR_GS_BASE, false);
baa035227b Tiejun Chen       2014-12-23  6830  	vmx_disable_intercept_for_msr(MSR_KERNEL_GS_BASE, true);
baa035227b Tiejun Chen       2014-12-23  6831  	vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_CS, false);
baa035227b Tiejun Chen       2014-12-23  6832  	vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_ESP, false);
baa035227b Tiejun Chen       2014-12-23  6833  	vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_EIP, false);
baa035227b Tiejun Chen       2014-12-23  6834  
c63e45635b Wanpeng Li        2016-09-23  6835  	memcpy(vmx_msr_bitmap_legacy_x2apic_apicv,
baa035227b Tiejun Chen       2014-12-23  6836  			vmx_msr_bitmap_legacy, PAGE_SIZE);
c63e45635b Wanpeng Li        2016-09-23  6837  	memcpy(vmx_msr_bitmap_longmode_x2apic_apicv,
baa035227b Tiejun Chen       2014-12-23  6838  			vmx_msr_bitmap_longmode, PAGE_SIZE);
c63e45635b Wanpeng Li        2016-09-23  6839  	memcpy(vmx_msr_bitmap_legacy_x2apic,
f6e90f9e0e Wanpeng Li        2016-09-22  6840  			vmx_msr_bitmap_legacy, PAGE_SIZE);
c63e45635b Wanpeng Li        2016-09-23  6841  	memcpy(vmx_msr_bitmap_longmode_x2apic,
f6e90f9e0e Wanpeng Li        2016-09-22  6842  			vmx_msr_bitmap_longmode, PAGE_SIZE);
baa035227b Tiejun Chen       2014-12-23  6843  
04bb92e4b4 Wanpeng Li        2015-09-16  6844  	set_bit(0, vmx_vpid_bitmap); /* 0 is reserved for host */
04bb92e4b4 Wanpeng Li        2015-09-16  6845  
40d8338d09 Radim Krčmář      2016-09-29  6846  	for (msr = 0x800; msr <= 0x8ff; msr++) {
40d8338d09 Radim Krčmář      2016-09-29  6847  		if (msr == 0x839 /* TMCCT */)
40d8338d09 Radim Krčmář      2016-09-29  6848  			continue;
2e69f86561 Radim Krčmář      2016-09-29  6849  		vmx_disable_intercept_msr_x2apic(msr, MSR_TYPE_R, true);
40d8338d09 Radim Krčmář      2016-09-29  6850  	}
baa035227b Tiejun Chen       2014-12-23  6851  
f6e90f9e0e Wanpeng Li        2016-09-22  6852  	/*
2e69f86561 Radim Krčmář      2016-09-29  6853  	 * TPR reads and writes can be virtualized even if virtual interrupt
2e69f86561 Radim Krčmář      2016-09-29  6854  	 * delivery is not in use.
f6e90f9e0e Wanpeng Li        2016-09-22  6855  	 */
2e69f86561 Radim Krčmář      2016-09-29  6856  	vmx_disable_intercept_msr_x2apic(0x808, MSR_TYPE_W, true);
2e69f86561 Radim Krčmář      2016-09-29  6857  	vmx_disable_intercept_msr_x2apic(0x808, MSR_TYPE_R | MSR_TYPE_W, false);
baa035227b Tiejun Chen       2014-12-23  6858  
baa035227b Tiejun Chen       2014-12-23  6859  	/* EOI */
2e69f86561 Radim Krčmář      2016-09-29  6860  	vmx_disable_intercept_msr_x2apic(0x80b, MSR_TYPE_W, true);
baa035227b Tiejun Chen       2014-12-23  6861  	/* SELF-IPI */
2e69f86561 Radim Krčmář      2016-09-29  6862  	vmx_disable_intercept_msr_x2apic(0x83f, MSR_TYPE_W, true);
baa035227b Tiejun Chen       2014-12-23  6863  
f160c7b7bb Junaid Shahid     2016-12-06  6864  	if (enable_ept)
f160c7b7bb Junaid Shahid     2016-12-06  6865  		vmx_enable_tdp();
f160c7b7bb Junaid Shahid     2016-12-06  6866  	else
baa035227b Tiejun Chen       2014-12-23  6867  		kvm_disable_tdp();
baa035227b Tiejun Chen       2014-12-23  6868  
baa035227b Tiejun Chen       2014-12-23  6869  	update_ple_window_actual_max();
baa035227b Tiejun Chen       2014-12-23  6870  
843e433057 Kai Huang         2015-01-28  6871  	/*
843e433057 Kai Huang         2015-01-28  6872  	 * Only enable PML when hardware supports PML feature, and both EPT
843e433057 Kai Huang         2015-01-28  6873  	 * and EPT A/D bit features are enabled -- PML depends on them to work.
843e433057 Kai Huang         2015-01-28  6874  	 */
843e433057 Kai Huang         2015-01-28  6875  	if (!enable_ept || !enable_ept_ad_bits || !cpu_has_vmx_pml())
843e433057 Kai Huang         2015-01-28  6876  		enable_pml = 0;
843e433057 Kai Huang         2015-01-28  6877  
843e433057 Kai Huang         2015-01-28  6878  	if (!enable_pml) {
843e433057 Kai Huang         2015-01-28  6879  		kvm_x86_ops->slot_enable_log_dirty = NULL;
843e433057 Kai Huang         2015-01-28  6880  		kvm_x86_ops->slot_disable_log_dirty = NULL;
843e433057 Kai Huang         2015-01-28  6881  		kvm_x86_ops->flush_log_dirty = NULL;
843e433057 Kai Huang         2015-01-28  6882  		kvm_x86_ops->enable_log_dirty_pt_masked = NULL;
843e433057 Kai Huang         2015-01-28  6883  	}
843e433057 Kai Huang         2015-01-28  6884  
64672c95ea Yunhong Jiang     2016-06-13  6885  	if (cpu_has_vmx_preemption_timer() && enable_preemption_timer) {
64672c95ea Yunhong Jiang     2016-06-13  6886  		u64 vmx_msr;
64672c95ea Yunhong Jiang     2016-06-13  6887  
64672c95ea Yunhong Jiang     2016-06-13  6888  		rdmsrl(MSR_IA32_VMX_MISC, vmx_msr);
64672c95ea Yunhong Jiang     2016-06-13  6889  		cpu_preemption_timer_multi =
64672c95ea Yunhong Jiang     2016-06-13  6890  			 vmx_msr & VMX_MISC_PREEMPTION_TIMER_RATE_MASK;
64672c95ea Yunhong Jiang     2016-06-13  6891  	} else {
64672c95ea Yunhong Jiang     2016-06-13  6892  		kvm_x86_ops->set_hv_timer = NULL;
64672c95ea Yunhong Jiang     2016-06-13  6893  		kvm_x86_ops->cancel_hv_timer = NULL;
64672c95ea Yunhong Jiang     2016-06-13  6894  	}
64672c95ea Yunhong Jiang     2016-06-13  6895  
bf9f6ac8d7 Feng Wu           2015-09-18  6896  	kvm_set_posted_intr_wakeup_handler(wakeup_handler);
bf9f6ac8d7 Feng Wu           2015-09-18  6897  
c45dcc71b7 Ashok Raj         2016-06-22  6898  	kvm_mce_cap_supported |= MCG_LMCE_P;
c45dcc71b7 Ashok Raj         2016-06-22  6899  
f2c7648d91 Tiejun Chen       2014-10-28  6900  	return alloc_kvm_area();
34a1cd60d1 Tiejun Chen       2014-10-28  6901  
34a1cd60d1 Tiejun Chen       2014-10-28  6902  out:
2361133293 Radim Krčmář      2016-09-29  6903  	for (i = 0; i < VMX_BITMAP_NR; i++)
2361133293 Radim Krčmář      2016-09-29  6904  		free_page((unsigned long)vmx_bitmap[i]);
34a1cd60d1 Tiejun Chen       2014-10-28  6905  
34a1cd60d1 Tiejun Chen       2014-10-28  6906      return r;
f2c7648d91 Tiejun Chen       2014-10-28  6907  }
f2c7648d91 Tiejun Chen       2014-10-28  6908  

:::::: The code at line 6751 was first introduced by commit
:::::: 23611332938d8c7e87f7fbe6d3bc37f9c50d688f KVM: VMX: refactor setup of global page-sized bitmaps

:::::: TO: Radim Krčmář <rkrcmar@redhat.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: 62397 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] KVM: VMX: Use just one page for I/O permission bitmaps
  2017-12-04 18:30   ` kbuild test robot
@ 2017-12-04 18:34     ` Jim Mattson
  0 siblings, 0 replies; 20+ messages in thread
From: Jim Mattson @ 2017-12-04 18:34 UTC (permalink / raw)
  To: kbuild test robot; +Cc: kbuild-all, kvm list, P J P

The line in question was removed in a previously submitted patch.

On Mon, Dec 4, 2017 at 10:30 AM, kbuild test robot <lkp@intel.com> wrote:
> Hi Jim,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on kvm/linux-next]
> [also build test ERROR on v4.15-rc2 next-20171204]
> [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-VMX-remove-I-O-port-0x80-bypass-on-Intel-hosts/20171205-013621
> base:   https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next
> config: x86_64-allmodconfig (attached as .config)
> compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64
>
> All errors (new ones prefixed by >>):
>
>    arch/x86/kvm/vmx.c: In function 'hardware_setup':
>>> arch/x86/kvm/vmx.c:6751:2: error: 'vmx_io_bitmap_b' undeclared (first use in this function); did you mean 'vmx_io_bitmap'?
>      vmx_io_bitmap_b = (unsigned long *)__get_free_page(GFP_KERNEL);
>      ^~~~~~~~~~~~~~~
>      vmx_io_bitmap
>    arch/x86/kvm/vmx.c:6751:2: note: each undeclared identifier is reported only once for each function it appears in
>
> vim +6751 arch/x86/kvm/vmx.c
>
> f160c7b7bb Junaid Shahid     2016-12-06  6735
> f2c7648d91 Tiejun Chen       2014-10-28  6736  static __init int hardware_setup(void)
> f2c7648d91 Tiejun Chen       2014-10-28  6737  {
> 34a1cd60d1 Tiejun Chen       2014-10-28  6738   int r = -ENOMEM, i, msr;
> 34a1cd60d1 Tiejun Chen       2014-10-28  6739
> 34a1cd60d1 Tiejun Chen       2014-10-28  6740   rdmsrl_safe(MSR_EFER, &host_efer);
> 34a1cd60d1 Tiejun Chen       2014-10-28  6741
> 34a1cd60d1 Tiejun Chen       2014-10-28  6742   for (i = 0; i < ARRAY_SIZE(vmx_msr_index); ++i)
> 34a1cd60d1 Tiejun Chen       2014-10-28  6743           kvm_define_shared_msr(i, vmx_msr_index[i]);
> 34a1cd60d1 Tiejun Chen       2014-10-28  6744
> 2361133293 Radim Krčmář      2016-09-29  6745   for (i = 0; i < VMX_BITMAP_NR; i++) {
> 2361133293 Radim Krčmář      2016-09-29  6746           vmx_bitmap[i] = (unsigned long *)__get_free_page(GFP_KERNEL);
> 2361133293 Radim Krčmář      2016-09-29  6747           if (!vmx_bitmap[i])
> 34a1cd60d1 Tiejun Chen       2014-10-28  6748                   goto out;
> 2361133293 Radim Krčmář      2016-09-29  6749   }
> 34a1cd60d1 Tiejun Chen       2014-10-28  6750
> 2361133293 Radim Krčmář      2016-09-29 @6751   vmx_io_bitmap_b = (unsigned long *)__get_free_page(GFP_KERNEL);
> 34a1cd60d1 Tiejun Chen       2014-10-28  6752   memset(vmx_vmread_bitmap, 0xff, PAGE_SIZE);
> 34a1cd60d1 Tiejun Chen       2014-10-28  6753   memset(vmx_vmwrite_bitmap, 0xff, PAGE_SIZE);
> 34a1cd60d1 Tiejun Chen       2014-10-28  6754
> 4a290f4464 Jim Mattson       2017-12-01  6755   memset(vmx_io_bitmap, 0xff, PAGE_SIZE);
> 34a1cd60d1 Tiejun Chen       2014-10-28  6756
> 34a1cd60d1 Tiejun Chen       2014-10-28  6757   memset(vmx_msr_bitmap_legacy, 0xff, PAGE_SIZE);
> 34a1cd60d1 Tiejun Chen       2014-10-28  6758   memset(vmx_msr_bitmap_longmode, 0xff, PAGE_SIZE);
> 34a1cd60d1 Tiejun Chen       2014-10-28  6759
> 34a1cd60d1 Tiejun Chen       2014-10-28  6760   if (setup_vmcs_config(&vmcs_config) < 0) {
> 34a1cd60d1 Tiejun Chen       2014-10-28  6761           r = -EIO;
> 2361133293 Radim Krčmář      2016-09-29  6762           goto out;
> 34a1cd60d1 Tiejun Chen       2014-10-28  6763   }
> f2c7648d91 Tiejun Chen       2014-10-28  6764
> f2c7648d91 Tiejun Chen       2014-10-28  6765   if (boot_cpu_has(X86_FEATURE_NX))
> f2c7648d91 Tiejun Chen       2014-10-28  6766           kvm_enable_efer_bits(EFER_NX);
> f2c7648d91 Tiejun Chen       2014-10-28  6767
> 08d839c4b1 Wanpeng Li        2017-03-23  6768   if (!cpu_has_vmx_vpid() || !cpu_has_vmx_invvpid() ||
> 08d839c4b1 Wanpeng Li        2017-03-23  6769           !(cpu_has_vmx_invvpid_single() || cpu_has_vmx_invvpid_global()))
> f2c7648d91 Tiejun Chen       2014-10-28  6770           enable_vpid = 0;
> 08d839c4b1 Wanpeng Li        2017-03-23  6771
> f2c7648d91 Tiejun Chen       2014-10-28  6772   if (!cpu_has_vmx_shadow_vmcs())
> f2c7648d91 Tiejun Chen       2014-10-28  6773           enable_shadow_vmcs = 0;
> f2c7648d91 Tiejun Chen       2014-10-28  6774   if (enable_shadow_vmcs)
> f2c7648d91 Tiejun Chen       2014-10-28  6775           init_vmcs_shadow_fields();
> f2c7648d91 Tiejun Chen       2014-10-28  6776
> f2c7648d91 Tiejun Chen       2014-10-28  6777   if (!cpu_has_vmx_ept() ||
> 42aa53b4e1 David Hildenbrand 2017-08-10  6778       !cpu_has_vmx_ept_4levels() ||
> f5f51586db David Hildenbrand 2017-08-24  6779       !cpu_has_vmx_ept_mt_wb() ||
> 8ad8182e93 Wanpeng Li        2017-10-09  6780       !cpu_has_vmx_invept_global())
> f2c7648d91 Tiejun Chen       2014-10-28  6781           enable_ept = 0;
> f2c7648d91 Tiejun Chen       2014-10-28  6782
> fce6ac4c05 Wanpeng Li        2017-05-11  6783   if (!cpu_has_vmx_ept_ad_bits() || !enable_ept)
> f2c7648d91 Tiejun Chen       2014-10-28  6784           enable_ept_ad_bits = 0;
> f2c7648d91 Tiejun Chen       2014-10-28  6785
> 8ad8182e93 Wanpeng Li        2017-10-09  6786   if (!cpu_has_vmx_unrestricted_guest() || !enable_ept)
> f2c7648d91 Tiejun Chen       2014-10-28  6787           enable_unrestricted_guest = 0;
> f2c7648d91 Tiejun Chen       2014-10-28  6788
> ad15a29647 Paolo Bonzini     2015-01-30  6789   if (!cpu_has_vmx_flexpriority())
> f2c7648d91 Tiejun Chen       2014-10-28  6790           flexpriority_enabled = 0;
> f2c7648d91 Tiejun Chen       2014-10-28  6791
> d02fcf5077 Paolo Bonzini     2017-11-06  6792   if (!cpu_has_virtual_nmis())
> d02fcf5077 Paolo Bonzini     2017-11-06  6793           enable_vnmi = 0;
> d02fcf5077 Paolo Bonzini     2017-11-06  6794
> f2c7648d91 Tiejun Chen       2014-10-28  6795   /*
> f2c7648d91 Tiejun Chen       2014-10-28  6796    * set_apic_access_page_addr() is used to reload apic access
> ad15a29647 Paolo Bonzini     2015-01-30  6797    * page upon invalidation.  No need to do anything if not
> ad15a29647 Paolo Bonzini     2015-01-30  6798    * using the APIC_ACCESS_ADDR VMCS field.
> f2c7648d91 Tiejun Chen       2014-10-28  6799    */
> ad15a29647 Paolo Bonzini     2015-01-30  6800   if (!flexpriority_enabled)
> f2c7648d91 Tiejun Chen       2014-10-28  6801           kvm_x86_ops->set_apic_access_page_addr = NULL;
> f2c7648d91 Tiejun Chen       2014-10-28  6802
> f2c7648d91 Tiejun Chen       2014-10-28  6803   if (!cpu_has_vmx_tpr_shadow())
> f2c7648d91 Tiejun Chen       2014-10-28  6804           kvm_x86_ops->update_cr8_intercept = NULL;
> f2c7648d91 Tiejun Chen       2014-10-28  6805
> f2c7648d91 Tiejun Chen       2014-10-28  6806   if (enable_ept && !cpu_has_vmx_ept_2m_page())
> f2c7648d91 Tiejun Chen       2014-10-28  6807           kvm_disable_largepages();
> f2c7648d91 Tiejun Chen       2014-10-28  6808
> 0f107682cb Wanpeng Li        2017-09-28  6809   if (!cpu_has_vmx_ple()) {
> f2c7648d91 Tiejun Chen       2014-10-28  6810           ple_gap = 0;
> 0f107682cb Wanpeng Li        2017-09-28  6811           ple_window = 0;
> 0f107682cb Wanpeng Li        2017-09-28  6812           ple_window_grow = 0;
> 0f107682cb Wanpeng Li        2017-09-28  6813           ple_window_max = 0;
> 0f107682cb Wanpeng Li        2017-09-28  6814           ple_window_shrink = 0;
> 0f107682cb Wanpeng Li        2017-09-28  6815   }
> f2c7648d91 Tiejun Chen       2014-10-28  6816
> 76dfafd536 Paolo Bonzini     2016-12-19  6817   if (!cpu_has_vmx_apicv()) {
> f2c7648d91 Tiejun Chen       2014-10-28  6818           enable_apicv = 0;
> 76dfafd536 Paolo Bonzini     2016-12-19  6819           kvm_x86_ops->sync_pir_to_irr = NULL;
> 76dfafd536 Paolo Bonzini     2016-12-19  6820   }
> f2c7648d91 Tiejun Chen       2014-10-28  6821
> 64903d6195 Haozhong Zhang    2015-10-20  6822   if (cpu_has_vmx_tsc_scaling()) {
> 64903d6195 Haozhong Zhang    2015-10-20  6823           kvm_has_tsc_control = true;
> 64903d6195 Haozhong Zhang    2015-10-20  6824           kvm_max_tsc_scaling_ratio = KVM_VMX_TSC_MULTIPLIER_MAX;
> 64903d6195 Haozhong Zhang    2015-10-20  6825           kvm_tsc_scaling_ratio_frac_bits = 48;
> 64903d6195 Haozhong Zhang    2015-10-20  6826   }
> 64903d6195 Haozhong Zhang    2015-10-20  6827
> baa035227b Tiejun Chen       2014-12-23  6828   vmx_disable_intercept_for_msr(MSR_FS_BASE, false);
> baa035227b Tiejun Chen       2014-12-23  6829   vmx_disable_intercept_for_msr(MSR_GS_BASE, false);
> baa035227b Tiejun Chen       2014-12-23  6830   vmx_disable_intercept_for_msr(MSR_KERNEL_GS_BASE, true);
> baa035227b Tiejun Chen       2014-12-23  6831   vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_CS, false);
> baa035227b Tiejun Chen       2014-12-23  6832   vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_ESP, false);
> baa035227b Tiejun Chen       2014-12-23  6833   vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_EIP, false);
> baa035227b Tiejun Chen       2014-12-23  6834
> c63e45635b Wanpeng Li        2016-09-23  6835   memcpy(vmx_msr_bitmap_legacy_x2apic_apicv,
> baa035227b Tiejun Chen       2014-12-23  6836                   vmx_msr_bitmap_legacy, PAGE_SIZE);
> c63e45635b Wanpeng Li        2016-09-23  6837   memcpy(vmx_msr_bitmap_longmode_x2apic_apicv,
> baa035227b Tiejun Chen       2014-12-23  6838                   vmx_msr_bitmap_longmode, PAGE_SIZE);
> c63e45635b Wanpeng Li        2016-09-23  6839   memcpy(vmx_msr_bitmap_legacy_x2apic,
> f6e90f9e0e Wanpeng Li        2016-09-22  6840                   vmx_msr_bitmap_legacy, PAGE_SIZE);
> c63e45635b Wanpeng Li        2016-09-23  6841   memcpy(vmx_msr_bitmap_longmode_x2apic,
> f6e90f9e0e Wanpeng Li        2016-09-22  6842                   vmx_msr_bitmap_longmode, PAGE_SIZE);
> baa035227b Tiejun Chen       2014-12-23  6843
> 04bb92e4b4 Wanpeng Li        2015-09-16  6844   set_bit(0, vmx_vpid_bitmap); /* 0 is reserved for host */
> 04bb92e4b4 Wanpeng Li        2015-09-16  6845
> 40d8338d09 Radim Krčmář      2016-09-29  6846   for (msr = 0x800; msr <= 0x8ff; msr++) {
> 40d8338d09 Radim Krčmář      2016-09-29  6847           if (msr == 0x839 /* TMCCT */)
> 40d8338d09 Radim Krčmář      2016-09-29  6848                   continue;
> 2e69f86561 Radim Krčmář      2016-09-29  6849           vmx_disable_intercept_msr_x2apic(msr, MSR_TYPE_R, true);
> 40d8338d09 Radim Krčmář      2016-09-29  6850   }
> baa035227b Tiejun Chen       2014-12-23  6851
> f6e90f9e0e Wanpeng Li        2016-09-22  6852   /*
> 2e69f86561 Radim Krčmář      2016-09-29  6853    * TPR reads and writes can be virtualized even if virtual interrupt
> 2e69f86561 Radim Krčmář      2016-09-29  6854    * delivery is not in use.
> f6e90f9e0e Wanpeng Li        2016-09-22  6855    */
> 2e69f86561 Radim Krčmář      2016-09-29  6856   vmx_disable_intercept_msr_x2apic(0x808, MSR_TYPE_W, true);
> 2e69f86561 Radim Krčmář      2016-09-29  6857   vmx_disable_intercept_msr_x2apic(0x808, MSR_TYPE_R | MSR_TYPE_W, false);
> baa035227b Tiejun Chen       2014-12-23  6858
> baa035227b Tiejun Chen       2014-12-23  6859   /* EOI */
> 2e69f86561 Radim Krčmář      2016-09-29  6860   vmx_disable_intercept_msr_x2apic(0x80b, MSR_TYPE_W, true);
> baa035227b Tiejun Chen       2014-12-23  6861   /* SELF-IPI */
> 2e69f86561 Radim Krčmář      2016-09-29  6862   vmx_disable_intercept_msr_x2apic(0x83f, MSR_TYPE_W, true);
> baa035227b Tiejun Chen       2014-12-23  6863
> f160c7b7bb Junaid Shahid     2016-12-06  6864   if (enable_ept)
> f160c7b7bb Junaid Shahid     2016-12-06  6865           vmx_enable_tdp();
> f160c7b7bb Junaid Shahid     2016-12-06  6866   else
> baa035227b Tiejun Chen       2014-12-23  6867           kvm_disable_tdp();
> baa035227b Tiejun Chen       2014-12-23  6868
> baa035227b Tiejun Chen       2014-12-23  6869   update_ple_window_actual_max();
> baa035227b Tiejun Chen       2014-12-23  6870
> 843e433057 Kai Huang         2015-01-28  6871   /*
> 843e433057 Kai Huang         2015-01-28  6872    * Only enable PML when hardware supports PML feature, and both EPT
> 843e433057 Kai Huang         2015-01-28  6873    * and EPT A/D bit features are enabled -- PML depends on them to work.
> 843e433057 Kai Huang         2015-01-28  6874    */
> 843e433057 Kai Huang         2015-01-28  6875   if (!enable_ept || !enable_ept_ad_bits || !cpu_has_vmx_pml())
> 843e433057 Kai Huang         2015-01-28  6876           enable_pml = 0;
> 843e433057 Kai Huang         2015-01-28  6877
> 843e433057 Kai Huang         2015-01-28  6878   if (!enable_pml) {
> 843e433057 Kai Huang         2015-01-28  6879           kvm_x86_ops->slot_enable_log_dirty = NULL;
> 843e433057 Kai Huang         2015-01-28  6880           kvm_x86_ops->slot_disable_log_dirty = NULL;
> 843e433057 Kai Huang         2015-01-28  6881           kvm_x86_ops->flush_log_dirty = NULL;
> 843e433057 Kai Huang         2015-01-28  6882           kvm_x86_ops->enable_log_dirty_pt_masked = NULL;
> 843e433057 Kai Huang         2015-01-28  6883   }
> 843e433057 Kai Huang         2015-01-28  6884
> 64672c95ea Yunhong Jiang     2016-06-13  6885   if (cpu_has_vmx_preemption_timer() && enable_preemption_timer) {
> 64672c95ea Yunhong Jiang     2016-06-13  6886           u64 vmx_msr;
> 64672c95ea Yunhong Jiang     2016-06-13  6887
> 64672c95ea Yunhong Jiang     2016-06-13  6888           rdmsrl(MSR_IA32_VMX_MISC, vmx_msr);
> 64672c95ea Yunhong Jiang     2016-06-13  6889           cpu_preemption_timer_multi =
> 64672c95ea Yunhong Jiang     2016-06-13  6890                    vmx_msr & VMX_MISC_PREEMPTION_TIMER_RATE_MASK;
> 64672c95ea Yunhong Jiang     2016-06-13  6891   } else {
> 64672c95ea Yunhong Jiang     2016-06-13  6892           kvm_x86_ops->set_hv_timer = NULL;
> 64672c95ea Yunhong Jiang     2016-06-13  6893           kvm_x86_ops->cancel_hv_timer = NULL;
> 64672c95ea Yunhong Jiang     2016-06-13  6894   }
> 64672c95ea Yunhong Jiang     2016-06-13  6895
> bf9f6ac8d7 Feng Wu           2015-09-18  6896   kvm_set_posted_intr_wakeup_handler(wakeup_handler);
> bf9f6ac8d7 Feng Wu           2015-09-18  6897
> c45dcc71b7 Ashok Raj         2016-06-22  6898   kvm_mce_cap_supported |= MCG_LMCE_P;
> c45dcc71b7 Ashok Raj         2016-06-22  6899
> f2c7648d91 Tiejun Chen       2014-10-28  6900   return alloc_kvm_area();
> 34a1cd60d1 Tiejun Chen       2014-10-28  6901
> 34a1cd60d1 Tiejun Chen       2014-10-28  6902  out:
> 2361133293 Radim Krčmář      2016-09-29  6903   for (i = 0; i < VMX_BITMAP_NR; i++)
> 2361133293 Radim Krčmář      2016-09-29  6904           free_page((unsigned long)vmx_bitmap[i]);
> 34a1cd60d1 Tiejun Chen       2014-10-28  6905
> 34a1cd60d1 Tiejun Chen       2014-10-28  6906      return r;
> f2c7648d91 Tiejun Chen       2014-10-28  6907  }
> f2c7648d91 Tiejun Chen       2014-10-28  6908
>
> :::::: The code at line 6751 was first introduced by commit
> :::::: 23611332938d8c7e87f7fbe6d3bc37f9c50d688f KVM: VMX: refactor setup of global page-sized bitmaps
>
> :::::: TO: Radim Krčmář <rkrcmar@redhat.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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] KVM: VMX: Use just one page for I/O permission bitmaps
  2017-12-01 18:21 ` [PATCH 2/2] KVM: VMX: Use just one page for I/O permission bitmaps Jim Mattson
  2017-12-04 18:30   ` kbuild test robot
@ 2017-12-05 21:26   ` Radim Krčmář
  2017-12-06  0:16     ` Jim Mattson
  2017-12-06 11:17     ` Quan Xu
  1 sibling, 2 replies; 20+ messages in thread
From: Radim Krčmář @ 2017-12-05 21:26 UTC (permalink / raw)
  To: Jim Mattson; +Cc: kvm, P J P

2017-12-01 10:21-0800, Jim Mattson:
> Since we no longer allow any I/O ports to be passed through to the guest,
> we can use the same page for I/O bitmap A and I/O bitmap B.

I think we can disable the feature and save the second page as well:

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index e25c55ea2eb7..80859a7cdf6d 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3624,7 +3624,7 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
 #endif
 	      CPU_BASED_CR3_LOAD_EXITING |
 	      CPU_BASED_CR3_STORE_EXITING |
-	      CPU_BASED_USE_IO_BITMAPS |
+	      CPU_BASED_UNCOND_IO_EXITING |
 	      CPU_BASED_MOV_DR_EXITING |
 	      CPU_BASED_USE_TSC_OFFSETING |
 	      CPU_BASED_INVLPG_EXITING |

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] KVM: VMX: Use just one page for I/O permission bitmaps
  2017-12-05 21:26   ` Radim Krčmář
@ 2017-12-06  0:16     ` Jim Mattson
  2017-12-06 11:17     ` Quan Xu
  1 sibling, 0 replies; 20+ messages in thread
From: Jim Mattson @ 2017-12-06  0:16 UTC (permalink / raw)
  To: Radim Krčmář; +Cc: kvm list, P J P

Yes, of course.

Thanks!

On Tue, Dec 5, 2017 at 1:26 PM, Radim Krčmář <rkrcmar@redhat.com> wrote:
> 2017-12-01 10:21-0800, Jim Mattson:
>> Since we no longer allow any I/O ports to be passed through to the guest,
>> we can use the same page for I/O bitmap A and I/O bitmap B.
>
> I think we can disable the feature and save the second page as well:
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index e25c55ea2eb7..80859a7cdf6d 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -3624,7 +3624,7 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
>  #endif
>               CPU_BASED_CR3_LOAD_EXITING |
>               CPU_BASED_CR3_STORE_EXITING |
> -             CPU_BASED_USE_IO_BITMAPS |
> +             CPU_BASED_UNCOND_IO_EXITING |
>               CPU_BASED_MOV_DR_EXITING |
>               CPU_BASED_USE_TSC_OFFSETING |
>               CPU_BASED_INVLPG_EXITING |

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] KVM: VMX: Use just one page for I/O permission bitmaps
  2017-12-05 21:26   ` Radim Krčmář
  2017-12-06  0:16     ` Jim Mattson
@ 2017-12-06 11:17     ` Quan Xu
  2017-12-06 18:19       ` Jim Mattson
  1 sibling, 1 reply; 20+ messages in thread
From: Quan Xu @ 2017-12-06 11:17 UTC (permalink / raw)
  To: Radim Krčmář, Jim Mattson; +Cc: kvm, P J P



On 2017/12/06 05:26, Radim Krčmář wrote:
> 2017-12-01 10:21-0800, Jim Mattson:
>> Since we no longer allow any I/O ports to be passed through to the guest,
>> we can use the same page for I/O bitmap A and I/O bitmap B.
> I think we can disable the feature and save the second page as well:
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index e25c55ea2eb7..80859a7cdf6d 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -3624,7 +3624,7 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
>   #endif
>   	      CPU_BASED_CR3_LOAD_EXITING |
>   	      CPU_BASED_CR3_STORE_EXITING |
> -	      CPU_BASED_USE_IO_BITMAPS |
> +	      CPU_BASED_UNCOND_IO_EXITING |
>   	      CPU_BASED_MOV_DR_EXITING |
>   	      CPU_BASED_USE_TSC_OFFSETING |
>   	      CPU_BASED_INVLPG_EXITING |
>

Jim / Radim,


As a logical processor uses these bitmaps if and only if the “use I/O 
bitmaps” control is 1.

Since we drop 'CPU_BASED_USE_IOBITMAPS' in vmcs configuration.. We could 
also drop
'IO_BITMAP_A'/'IO_BITMAP_B'/'vmx_io_bitmap_a'/'vmx_io_bitmap_b' for a 
furthermore
cleanup as below:

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 04b4dbc..3e4f760 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -771,8 +771,6 @@ static struct pi_desc *vcpu_to_pi_desc(struct 
kvm_vcpu *vcpu)
         FIELD(HOST_FS_SELECTOR, host_fs_selector),
         FIELD(HOST_GS_SELECTOR, host_gs_selector),
         FIELD(HOST_TR_SELECTOR, host_tr_selector),
-       FIELD64(IO_BITMAP_A, io_bitmap_a),
-       FIELD64(IO_BITMAP_B, io_bitmap_b),
         FIELD64(MSR_BITMAP, msr_bitmap),
         FIELD64(VM_EXIT_MSR_STORE_ADDR, vm_exit_msr_store_addr),
         FIELD64(VM_EXIT_MSR_LOAD_ADDR, vm_exit_msr_load_addr),
@@ -943,8 +941,6 @@ static bool nested_vmx_is_page_fault_vmexit(struct 
vmcs12 *vmcs12,
  static DEFINE_PER_CPU(spinlock_t, blocked_vcpu_on_cpu_lock);

  enum {
-       VMX_IO_BITMAP_A,
-       VMX_IO_BITMAP_B,
         VMX_MSR_BITMAP_LEGACY,
         VMX_MSR_BITMAP_LONGMODE,
         VMX_MSR_BITMAP_LEGACY_X2APIC_APICV,
@@ -958,8 +954,6 @@ enum {

  static unsigned long *vmx_bitmap[VMX_BITMAP_NR];

-#define vmx_io_bitmap_a (vmx_bitmap[VMX_IO_BITMAP_A])
-#define vmx_io_bitmap_b (vmx_bitmap[VMX_IO_BITMAP_B])
  #define vmx_msr_bitmap_legacy (vmx_bitmap[VMX_MSR_BITMAP_LEGACY])
  #define vmx_msr_bitmap_longmode (vmx_bitmap[VMX_MSR_BITMAP_LONGMODE])
  #define vmx_msr_bitmap_legacy_x2apic_apicv 
(vmx_bitmap[VMX_MSR_BITMAP_LEGACY_X2APIC_APICV])
@@ -3632,7 +3626,7 @@ static __init int setup_vmcs_config(struct 
vmcs_config *vmcs_conf)
  #endif
               CPU_BASED_CR3_LOAD_EXITING |
               CPU_BASED_CR3_STORE_EXITING |
-             CPU_BASED_USE_IO_BITMAPS |
+             CPU_BASED_UNCOND_IO_EXITING |
               CPU_BASED_MOV_DR_EXITING |
               CPU_BASED_USE_TSC_OFFSETING |
               CPU_BASED_INVLPG_EXITING |
@@ -5445,10 +5439,6 @@ static void vmx_vcpu_setup(struct vcpu_vmx *vmx)
  #endif
         int i;

-       /* I/O */
-       vmcs_write64(IO_BITMAP_A, __pa(vmx_io_bitmap_a));
-       vmcs_write64(IO_BITMAP_B, __pa(vmx_io_bitmap_b));
-
         if (enable_shadow_vmcs) {
                 vmcs_write64(VMREAD_BITMAP, __pa(vmx_vmread_bitmap));
                 vmcs_write64(VMWRITE_BITMAP, __pa(vmx_vmwrite_bitmap));
@@ -6751,13 +6741,9 @@ static __init int hardware_setup(void)
                         goto out;
         }

-       vmx_io_bitmap_b = (unsigned long *)__get_free_page(GFP_KERNEL);
         memset(vmx_vmread_bitmap, 0xff, PAGE_SIZE);
         memset(vmx_vmwrite_bitmap, 0xff, PAGE_SIZE);

-       memset(vmx_io_bitmap_a, 0xff, PAGE_SIZE);
-       memset(vmx_io_bitmap_b, 0xff, PAGE_SIZE);
-
         memset(vmx_msr_bitmap_legacy, 0xff, PAGE_SIZE);
         memset(vmx_msr_bitmap_longmode, 0xff, PAGE_SIZE);





Quan
Alibaba Cloud

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] KVM: VMX: Use just one page for I/O permission bitmaps
  2017-12-06 11:17     ` Quan Xu
@ 2017-12-06 18:19       ` Jim Mattson
  2017-12-07  2:33         ` Quan Xu
  0 siblings, 1 reply; 20+ messages in thread
From: Jim Mattson @ 2017-12-06 18:19 UTC (permalink / raw)
  To: Quan Xu; +Cc: Radim Krčmář, kvm list, P J P

On Wed, Dec 6, 2017 at 3:17 AM, Quan Xu <quan.xu0@gmail.com> wrote:
>
>
> On 2017/12/06 05:26, Radim Krčmář wrote:
>>
>> 2017-12-01 10:21-0800, Jim Mattson:
>>>
>>> Since we no longer allow any I/O ports to be passed through to the guest,
>>> we can use the same page for I/O bitmap A and I/O bitmap B.
>>
>> I think we can disable the feature and save the second page as well:
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index e25c55ea2eb7..80859a7cdf6d 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -3624,7 +3624,7 @@ static __init int setup_vmcs_config(struct
>> vmcs_config *vmcs_conf)
>>   #endif
>>               CPU_BASED_CR3_LOAD_EXITING |
>>               CPU_BASED_CR3_STORE_EXITING |
>> -             CPU_BASED_USE_IO_BITMAPS |
>> +             CPU_BASED_UNCOND_IO_EXITING |
>>               CPU_BASED_MOV_DR_EXITING |
>>               CPU_BASED_USE_TSC_OFFSETING |
>>               CPU_BASED_INVLPG_EXITING |
>>
>
> Jim / Radim,
>
>
> As a logical processor uses these bitmaps if and only if the “use I/O
> bitmaps” control is 1.
>
> Since we drop 'CPU_BASED_USE_IOBITMAPS' in vmcs configuration.. We could
> also drop
> 'IO_BITMAP_A'/'IO_BITMAP_B'/'vmx_io_bitmap_a'/'vmx_io_bitmap_b' for a
> furthermore
> cleanup as below:
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 04b4dbc..3e4f760 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -771,8 +771,6 @@ static struct pi_desc *vcpu_to_pi_desc(struct kvm_vcpu
> *vcpu)
>         FIELD(HOST_FS_SELECTOR, host_fs_selector),
>         FIELD(HOST_GS_SELECTOR, host_gs_selector),
>         FIELD(HOST_TR_SELECTOR, host_tr_selector),
> -       FIELD64(IO_BITMAP_A, io_bitmap_a),
> -       FIELD64(IO_BITMAP_B, io_bitmap_b),

Stet.

>         FIELD64(MSR_BITMAP, msr_bitmap),
>         FIELD64(VM_EXIT_MSR_STORE_ADDR, vm_exit_msr_store_addr),
>         FIELD64(VM_EXIT_MSR_LOAD_ADDR, vm_exit_msr_load_addr),
> @@ -943,8 +941,6 @@ static bool nested_vmx_is_page_fault_vmexit(struct
> vmcs12 *vmcs12,
>  static DEFINE_PER_CPU(spinlock_t, blocked_vcpu_on_cpu_lock);
>
>  enum {
> -       VMX_IO_BITMAP_A,
> -       VMX_IO_BITMAP_B,
>         VMX_MSR_BITMAP_LEGACY,
>         VMX_MSR_BITMAP_LONGMODE,
>         VMX_MSR_BITMAP_LEGACY_X2APIC_APICV,
> @@ -958,8 +954,6 @@ enum {
>
>  static unsigned long *vmx_bitmap[VMX_BITMAP_NR];
>
> -#define vmx_io_bitmap_a (vmx_bitmap[VMX_IO_BITMAP_A])
> -#define vmx_io_bitmap_b (vmx_bitmap[VMX_IO_BITMAP_B])
>  #define vmx_msr_bitmap_legacy (vmx_bitmap[VMX_MSR_BITMAP_LEGACY])
>  #define vmx_msr_bitmap_longmode (vmx_bitmap[VMX_MSR_BITMAP_LONGMODE])
>  #define vmx_msr_bitmap_legacy_x2apic_apicv
> (vmx_bitmap[VMX_MSR_BITMAP_LEGACY_X2APIC_APICV])
> @@ -3632,7 +3626,7 @@ static __init int setup_vmcs_config(struct vmcs_config
> *vmcs_conf)
>  #endif
>               CPU_BASED_CR3_LOAD_EXITING |
>               CPU_BASED_CR3_STORE_EXITING |
> -             CPU_BASED_USE_IO_BITMAPS |
> +             CPU_BASED_UNCOND_IO_EXITING |

Is it safe to assume that all CPUs (both physical and virtual)
currently running kvm will support this control bit?

>               CPU_BASED_MOV_DR_EXITING |
>               CPU_BASED_USE_TSC_OFFSETING |
>               CPU_BASED_INVLPG_EXITING |
> @@ -5445,10 +5439,6 @@ static void vmx_vcpu_setup(struct vcpu_vmx *vmx)
>  #endif
>         int i;
>
> -       /* I/O */
> -       vmcs_write64(IO_BITMAP_A, __pa(vmx_io_bitmap_a));
> -       vmcs_write64(IO_BITMAP_B, __pa(vmx_io_bitmap_b));
> -
>         if (enable_shadow_vmcs) {
>                 vmcs_write64(VMREAD_BITMAP, __pa(vmx_vmread_bitmap));
>                 vmcs_write64(VMWRITE_BITMAP, __pa(vmx_vmwrite_bitmap));
> @@ -6751,13 +6741,9 @@ static __init int hardware_setup(void)
>                         goto out;
>         }
>
> -       vmx_io_bitmap_b = (unsigned long *)__get_free_page(GFP_KERNEL);
>         memset(vmx_vmread_bitmap, 0xff, PAGE_SIZE);
>         memset(vmx_vmwrite_bitmap, 0xff, PAGE_SIZE);
>
> -       memset(vmx_io_bitmap_a, 0xff, PAGE_SIZE);
> -       memset(vmx_io_bitmap_b, 0xff, PAGE_SIZE);
> -
>         memset(vmx_msr_bitmap_legacy, 0xff, PAGE_SIZE);
>         memset(vmx_msr_bitmap_longmode, 0xff, PAGE_SIZE);
>
>
>
>
>
> Quan
> Alibaba Cloud

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] KVM: VMX: Use just one page for I/O permission bitmaps
  2017-12-06 18:19       ` Jim Mattson
@ 2017-12-07  2:33         ` Quan Xu
  2017-12-07 17:06           ` Radim Krčmář
  0 siblings, 1 reply; 20+ messages in thread
From: Quan Xu @ 2017-12-07  2:33 UTC (permalink / raw)
  To: Jim Mattson, Radim Krčmář; +Cc: kvm list, P J P



On 2017/12/07 02:19, Jim Mattson wrote:
> On Wed, Dec 6, 2017 at 3:17 AM, Quan Xu <quan.xu0@gmail.com> wrote:
>>
>> On 2017/12/06 05:26, Radim Krčmář wrote:
>>> 2017-12-01 10:21-0800, Jim Mattson:
>>>> Since we no longer allow any I/O ports to be passed through to the guest,
>>>> we can use the same page for I/O bitmap A and I/O bitmap B.
>>> I think we can disable the feature and save the second page as well:
>>>
>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>> index e25c55ea2eb7..80859a7cdf6d 100644
>>> --- a/arch/x86/kvm/vmx.c
>>> +++ b/arch/x86/kvm/vmx.c
>>> @@ -3624,7 +3624,7 @@ static __init int setup_vmcs_config(struct
>>> vmcs_config *vmcs_conf)
>>>    #endif
>>>                CPU_BASED_CR3_LOAD_EXITING |
>>>                CPU_BASED_CR3_STORE_EXITING |
>>> -             CPU_BASED_USE_IO_BITMAPS |
>>> +             CPU_BASED_UNCOND_IO_EXITING |
>>>                CPU_BASED_MOV_DR_EXITING |
>>>                CPU_BASED_USE_TSC_OFFSETING |
>>>                CPU_BASED_INVLPG_EXITING |
>>>
>> Jim / Radim,
>>
>>
>> As a logical processor uses these bitmaps if and only if the “use I/O
>> bitmaps” control is 1.
>>
>> Since we drop 'CPU_BASED_USE_IOBITMAPS' in vmcs configuration.. We could
>> also drop
>> 'IO_BITMAP_A'/'IO_BITMAP_B'/'vmx_io_bitmap_a'/'vmx_io_bitmap_b' for a
>> furthermore
>> cleanup as below:
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 04b4dbc..3e4f760 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -771,8 +771,6 @@ static struct pi_desc *vcpu_to_pi_desc(struct kvm_vcpu
>> *vcpu)
>>          FIELD(HOST_FS_SELECTOR, host_fs_selector),
>>          FIELD(HOST_GS_SELECTOR, host_gs_selector),
>>          FIELD(HOST_TR_SELECTOR, host_tr_selector),
>> -       FIELD64(IO_BITMAP_A, io_bitmap_a),
>> -       FIELD64(IO_BITMAP_B, io_bitmap_b),
> Stet.
>
>>          FIELD64(MSR_BITMAP, msr_bitmap),
>>          FIELD64(VM_EXIT_MSR_STORE_ADDR, vm_exit_msr_store_addr),
>>          FIELD64(VM_EXIT_MSR_LOAD_ADDR, vm_exit_msr_load_addr),
>> @@ -943,8 +941,6 @@ static bool nested_vmx_is_page_fault_vmexit(struct
>> vmcs12 *vmcs12,
>>   static DEFINE_PER_CPU(spinlock_t, blocked_vcpu_on_cpu_lock);
>>
>>   enum {
>> -       VMX_IO_BITMAP_A,
>> -       VMX_IO_BITMAP_B,
>>          VMX_MSR_BITMAP_LEGACY,
>>          VMX_MSR_BITMAP_LONGMODE,
>>          VMX_MSR_BITMAP_LEGACY_X2APIC_APICV,
>> @@ -958,8 +954,6 @@ enum {
>>
>>   static unsigned long *vmx_bitmap[VMX_BITMAP_NR];
>>
>> -#define vmx_io_bitmap_a (vmx_bitmap[VMX_IO_BITMAP_A])
>> -#define vmx_io_bitmap_b (vmx_bitmap[VMX_IO_BITMAP_B])
>>   #define vmx_msr_bitmap_legacy (vmx_bitmap[VMX_MSR_BITMAP_LEGACY])
>>   #define vmx_msr_bitmap_longmode (vmx_bitmap[VMX_MSR_BITMAP_LONGMODE])
>>   #define vmx_msr_bitmap_legacy_x2apic_apicv
>> (vmx_bitmap[VMX_MSR_BITMAP_LEGACY_X2APIC_APICV])
>> @@ -3632,7 +3626,7 @@ static __init int setup_vmcs_config(struct vmcs_config
>> *vmcs_conf)
>>   #endif
>>                CPU_BASED_CR3_LOAD_EXITING |
>>                CPU_BASED_CR3_STORE_EXITING |
>> -             CPU_BASED_USE_IO_BITMAPS |
>> +             CPU_BASED_UNCOND_IO_EXITING |
> Is it safe to assume that all CPUs (both physical and virtual)
> currently running kvm will support this control bit?

Jim, I don't see some conditions to "Use I/O bitmaps" or "Unconditional 
I/O exiting" in Intel SDM.
and in prepare_vmcs02(),

...
         /*
          * Merging of IO bitmap not currently supported.
          * Rather, exit every time.
          */
         exec_control &= ~CPU_BASED_USE_IO_BITMAPS;
         exec_control |= CPU_BASED_UNCOND_IO_EXITING;
...


So I think it is safe for cpu and vcpu.

as sdm said, """ "Unconditional I/O exiting" -- This control determines 
whether executions of I/O
instructions (IN, INS/INSB/INSW/INSD, OUT, and OUTS/OUTSB/OUTSW/OUTSD) 
cause VM exits.
"Use I/O bitmaps" -- For this control, “0” means “do not use I/O 
bitmaps” and “1” means
“use I/O bitmaps.” """"

I think it is safe for all CPU, if we clear the "Use I/O bitmaps". It 
doesn't matter whether
the "Unconditional I/O exiting" is set or clear.. Of Course both bits 
are clear, which may
make host/guest to the incorrect field.

is it?  Radim, could you help me double check it?


Quan
Alibaba Cloud

>>                CPU_BASED_MOV_DR_EXITING |
>>                CPU_BASED_USE_TSC_OFFSETING |
>>                CPU_BASED_INVLPG_EXITING |
>> @@ -5445,10 +5439,6 @@ static void vmx_vcpu_setup(struct vcpu_vmx *vmx)
>>   #endif
>>          int i;
>>
>> -       /* I/O */
>> -       vmcs_write64(IO_BITMAP_A, __pa(vmx_io_bitmap_a));
>> -       vmcs_write64(IO_BITMAP_B, __pa(vmx_io_bitmap_b));
>> -
>>          if (enable_shadow_vmcs) {
>>                  vmcs_write64(VMREAD_BITMAP, __pa(vmx_vmread_bitmap));
>>                  vmcs_write64(VMWRITE_BITMAP, __pa(vmx_vmwrite_bitmap));
>> @@ -6751,13 +6741,9 @@ static __init int hardware_setup(void)
>>                          goto out;
>>          }
>>
>> -       vmx_io_bitmap_b = (unsigned long *)__get_free_page(GFP_KERNEL);
>>          memset(vmx_vmread_bitmap, 0xff, PAGE_SIZE);
>>          memset(vmx_vmwrite_bitmap, 0xff, PAGE_SIZE);
>>
>> -       memset(vmx_io_bitmap_a, 0xff, PAGE_SIZE);
>> -       memset(vmx_io_bitmap_b, 0xff, PAGE_SIZE);
>> -
>>          memset(vmx_msr_bitmap_legacy, 0xff, PAGE_SIZE);
>>          memset(vmx_msr_bitmap_longmode, 0xff, PAGE_SIZE);
>>
>>
>>
>>
>>
>> Quan
>> Alibaba Cloud

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] KVM: VMX: Use just one page for I/O permission bitmaps
  2017-12-07  2:33         ` Quan Xu
@ 2017-12-07 17:06           ` Radim Krčmář
  2017-12-08  2:04             ` Quan Xu
  0 siblings, 1 reply; 20+ messages in thread
From: Radim Krčmář @ 2017-12-07 17:06 UTC (permalink / raw)
  To: Quan Xu; +Cc: Jim Mattson, kvm list, P J P

2017-12-07 10:33+0800, Quan Xu:
> 
> 
> On 2017/12/07 02:19, Jim Mattson wrote:
> > On Wed, Dec 6, 2017 at 3:17 AM, Quan Xu <quan.xu0@gmail.com> wrote:
> > > 
> > > On 2017/12/06 05:26, Radim Krčmář wrote:
> > > > 2017-12-01 10:21-0800, Jim Mattson:
> > > > > Since we no longer allow any I/O ports to be passed through to the guest,
> > > > > we can use the same page for I/O bitmap A and I/O bitmap B.
> > > > I think we can disable the feature and save the second page as well:
> > > > 
> > > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > > > index e25c55ea2eb7..80859a7cdf6d 100644
> > > > --- a/arch/x86/kvm/vmx.c
> > > > +++ b/arch/x86/kvm/vmx.c
> > > > @@ -3624,7 +3624,7 @@ static __init int setup_vmcs_config(struct
> > > > vmcs_config *vmcs_conf)
> > > >    #endif
> > > >                CPU_BASED_CR3_LOAD_EXITING |
> > > >                CPU_BASED_CR3_STORE_EXITING |
> > > > -             CPU_BASED_USE_IO_BITMAPS |
> > > > +             CPU_BASED_UNCOND_IO_EXITING |
> > > >                CPU_BASED_MOV_DR_EXITING |
> > > >                CPU_BASED_USE_TSC_OFFSETING |
> > > >                CPU_BASED_INVLPG_EXITING |
> > > > 
> > > Jim / Radim,
> > > 
> > > 
> > > As a logical processor uses these bitmaps if and only if the “use I/O
> > > bitmaps” control is 1.
> > > 
> > > Since we drop 'CPU_BASED_USE_IOBITMAPS' in vmcs configuration.. We could
> > > also drop
> > > 'IO_BITMAP_A'/'IO_BITMAP_B'/'vmx_io_bitmap_a'/'vmx_io_bitmap_b' for a
> > > furthermore
> > > cleanup as below:
> > > 
> > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > > index 04b4dbc..3e4f760 100644
> > > --- a/arch/x86/kvm/vmx.c
> > > +++ b/arch/x86/kvm/vmx.c
> > > @@ -771,8 +771,6 @@ static struct pi_desc *vcpu_to_pi_desc(struct kvm_vcpu
> > > *vcpu)
> > >          FIELD(HOST_FS_SELECTOR, host_fs_selector),
> > >          FIELD(HOST_GS_SELECTOR, host_gs_selector),
> > >          FIELD(HOST_TR_SELECTOR, host_tr_selector),
> > > -       FIELD64(IO_BITMAP_A, io_bitmap_a),
> > > -       FIELD64(IO_BITMAP_B, io_bitmap_b),
> > Stet.
> > 
> > >          FIELD64(MSR_BITMAP, msr_bitmap),
> > >          FIELD64(VM_EXIT_MSR_STORE_ADDR, vm_exit_msr_store_addr),
> > >          FIELD64(VM_EXIT_MSR_LOAD_ADDR, vm_exit_msr_load_addr),
> > > @@ -943,8 +941,6 @@ static bool nested_vmx_is_page_fault_vmexit(struct
> > > vmcs12 *vmcs12,
> > >   static DEFINE_PER_CPU(spinlock_t, blocked_vcpu_on_cpu_lock);
> > > 
> > >   enum {
> > > -       VMX_IO_BITMAP_A,
> > > -       VMX_IO_BITMAP_B,
> > >          VMX_MSR_BITMAP_LEGACY,
> > >          VMX_MSR_BITMAP_LONGMODE,
> > >          VMX_MSR_BITMAP_LEGACY_X2APIC_APICV,
> > > @@ -958,8 +954,6 @@ enum {
> > > 
> > >   static unsigned long *vmx_bitmap[VMX_BITMAP_NR];
> > > 
> > > -#define vmx_io_bitmap_a (vmx_bitmap[VMX_IO_BITMAP_A])
> > > -#define vmx_io_bitmap_b (vmx_bitmap[VMX_IO_BITMAP_B])
> > >   #define vmx_msr_bitmap_legacy (vmx_bitmap[VMX_MSR_BITMAP_LEGACY])
> > >   #define vmx_msr_bitmap_longmode (vmx_bitmap[VMX_MSR_BITMAP_LONGMODE])
> > >   #define vmx_msr_bitmap_legacy_x2apic_apicv
> > > (vmx_bitmap[VMX_MSR_BITMAP_LEGACY_X2APIC_APICV])
> > > @@ -3632,7 +3626,7 @@ static __init int setup_vmcs_config(struct vmcs_config
> > > *vmcs_conf)
> > >   #endif
> > >                CPU_BASED_CR3_LOAD_EXITING |
> > >                CPU_BASED_CR3_STORE_EXITING |
> > > -             CPU_BASED_USE_IO_BITMAPS |
> > > +             CPU_BASED_UNCOND_IO_EXITING |
> > Is it safe to assume that all CPUs (both physical and virtual)
> > currently running kvm will support this control bit?
> 
> Jim, I don't see some conditions to "Use I/O bitmaps" or "Unconditional I/O
> exiting" in Intel SDM.
> and in prepare_vmcs02(),
> 
> ...
>         /*
>          * Merging of IO bitmap not currently supported.
>          * Rather, exit every time.
>          */
>         exec_control &= ~CPU_BASED_USE_IO_BITMAPS;
>         exec_control |= CPU_BASED_UNCOND_IO_EXITING;
> ...
> 
> 
> So I think it is safe for cpu and vcpu.

I agree that it is safe.

> 
> as sdm said, """ "Unconditional I/O exiting" -- This control determines
> whether executions of I/O
> instructions (IN, INS/INSB/INSW/INSD, OUT, and OUTS/OUTSB/OUTSW/OUTSD) cause
> VM exits.
> "Use I/O bitmaps" -- For this control, “0” means “do not use I/O bitmaps”
> and “1” means
> “use I/O bitmaps.” """"
> 
> I think it is safe for all CPU, if we clear the "Use I/O bitmaps". It
> doesn't matter whether
> the "Unconditional I/O exiting" is set or clear.. Of Course both bits are
> clear, which may
> make host/guest to the incorrect field.
> 
> is it?  Radim, could you help me double check it?

I don't think it is correct to have neither, though, SMD describes them
as an alternative:

33.3.2.1 PIC Virtualization
  If the VMM is not supporting direct access to any I/O ports from a
  guest, it can set the unconditional-I/O-exiting in the VM-execution
  control field instead of activating I/O bitmaps.

And a quick test shows that we get a VM entry failure if both of them
are clear, although I can't find that explicitly in SDM.

Thanks.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] KVM: VMX: Use just one page for I/O permission bitmaps
  2017-12-07 17:06           ` Radim Krčmář
@ 2017-12-08  2:04             ` Quan Xu
  0 siblings, 0 replies; 20+ messages in thread
From: Quan Xu @ 2017-12-08  2:04 UTC (permalink / raw)
  To: Radim Krčmář; +Cc: Jim Mattson, kvm list, P J P



On 2017/12/08 01:06, Radim Krčmář wrote:
> 2017-12-07 10:33+0800, Quan Xu:
>>
>> On 2017/12/07 02:19, Jim Mattson wrote:
>>> On Wed, Dec 6, 2017 at 3:17 AM, Quan Xu <quan.xu0@gmail.com> wrote:
>>>> On 2017/12/06 05:26, Radim Krčmář wrote:
>>>>> 2017-12-01 10:21-0800, Jim Mattson:
>>>>>> Since we no longer allow any I/O ports to be passed through to the guest,
>>>>>> we can use the same page for I/O bitmap A and I/O bitmap B.
>>>>> I think we can disable the feature and save the second page as well:
>>>>>
>>>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>>>> index e25c55ea2eb7..80859a7cdf6d 100644
>>>>> --- a/arch/x86/kvm/vmx.c
>>>>> +++ b/arch/x86/kvm/vmx.c
>>>>> @@ -3624,7 +3624,7 @@ static __init int setup_vmcs_config(struct
>>>>> vmcs_config *vmcs_conf)
>>>>>     #endif
>>>>>                 CPU_BASED_CR3_LOAD_EXITING |
>>>>>                 CPU_BASED_CR3_STORE_EXITING |
>>>>> -             CPU_BASED_USE_IO_BITMAPS |
>>>>> +             CPU_BASED_UNCOND_IO_EXITING |
>>>>>                 CPU_BASED_MOV_DR_EXITING |
>>>>>                 CPU_BASED_USE_TSC_OFFSETING |
>>>>>                 CPU_BASED_INVLPG_EXITING |
>>>>>
>>>> Jim / Radim,
>>>>
>>>>
>>>> As a logical processor uses these bitmaps if and only if the “use I/O
>>>> bitmaps” control is 1.
>>>>
>>>> Since we drop 'CPU_BASED_USE_IOBITMAPS' in vmcs configuration.. We could
>>>> also drop
>>>> 'IO_BITMAP_A'/'IO_BITMAP_B'/'vmx_io_bitmap_a'/'vmx_io_bitmap_b' for a
>>>> furthermore
>>>> cleanup as below:
>>>>
>>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>>> index 04b4dbc..3e4f760 100644
>>>> --- a/arch/x86/kvm/vmx.c
>>>> +++ b/arch/x86/kvm/vmx.c
>>>> @@ -771,8 +771,6 @@ static struct pi_desc *vcpu_to_pi_desc(struct kvm_vcpu
>>>> *vcpu)
>>>>           FIELD(HOST_FS_SELECTOR, host_fs_selector),
>>>>           FIELD(HOST_GS_SELECTOR, host_gs_selector),
>>>>           FIELD(HOST_TR_SELECTOR, host_tr_selector),
>>>> -       FIELD64(IO_BITMAP_A, io_bitmap_a),
>>>> -       FIELD64(IO_BITMAP_B, io_bitmap_b),
>>> Stet.
>>>
>>>>           FIELD64(MSR_BITMAP, msr_bitmap),
>>>>           FIELD64(VM_EXIT_MSR_STORE_ADDR, vm_exit_msr_store_addr),
>>>>           FIELD64(VM_EXIT_MSR_LOAD_ADDR, vm_exit_msr_load_addr),
>>>> @@ -943,8 +941,6 @@ static bool nested_vmx_is_page_fault_vmexit(struct
>>>> vmcs12 *vmcs12,
>>>>    static DEFINE_PER_CPU(spinlock_t, blocked_vcpu_on_cpu_lock);
>>>>
>>>>    enum {
>>>> -       VMX_IO_BITMAP_A,
>>>> -       VMX_IO_BITMAP_B,
>>>>           VMX_MSR_BITMAP_LEGACY,
>>>>           VMX_MSR_BITMAP_LONGMODE,
>>>>           VMX_MSR_BITMAP_LEGACY_X2APIC_APICV,
>>>> @@ -958,8 +954,6 @@ enum {
>>>>
>>>>    static unsigned long *vmx_bitmap[VMX_BITMAP_NR];
>>>>
>>>> -#define vmx_io_bitmap_a (vmx_bitmap[VMX_IO_BITMAP_A])
>>>> -#define vmx_io_bitmap_b (vmx_bitmap[VMX_IO_BITMAP_B])
>>>>    #define vmx_msr_bitmap_legacy (vmx_bitmap[VMX_MSR_BITMAP_LEGACY])
>>>>    #define vmx_msr_bitmap_longmode (vmx_bitmap[VMX_MSR_BITMAP_LONGMODE])
>>>>    #define vmx_msr_bitmap_legacy_x2apic_apicv
>>>> (vmx_bitmap[VMX_MSR_BITMAP_LEGACY_X2APIC_APICV])
>>>> @@ -3632,7 +3626,7 @@ static __init int setup_vmcs_config(struct vmcs_config
>>>> *vmcs_conf)
>>>>    #endif
>>>>                 CPU_BASED_CR3_LOAD_EXITING |
>>>>                 CPU_BASED_CR3_STORE_EXITING |
>>>> -             CPU_BASED_USE_IO_BITMAPS |
>>>> +             CPU_BASED_UNCOND_IO_EXITING |
>>> Is it safe to assume that all CPUs (both physical and virtual)
>>> currently running kvm will support this control bit?
>> Jim, I don't see some conditions to "Use I/O bitmaps" or "Unconditional I/O
>> exiting" in Intel SDM.
>> and in prepare_vmcs02(),
>>
>> ...
>>          /*
>>           * Merging of IO bitmap not currently supported.
>>           * Rather, exit every time.
>>           */
>>          exec_control &= ~CPU_BASED_USE_IO_BITMAPS;
>>          exec_control |= CPU_BASED_UNCOND_IO_EXITING;
>> ...
>>
>>
>> So I think it is safe for cpu and vcpu.
> I agree that it is safe.
>
>> as sdm said, """ "Unconditional I/O exiting" -- This control determines
>> whether executions of I/O
>> instructions (IN, INS/INSB/INSW/INSD, OUT, and OUTS/OUTSB/OUTSW/OUTSD) cause
>> VM exits.
>> "Use I/O bitmaps" -- For this control, “0” means “do not use I/O bitmaps”
>> and “1” means
>> “use I/O bitmaps.” """"
>>
>> I think it is safe for all CPU, if we clear the "Use I/O bitmaps". It
>> doesn't matter whether
>> the "Unconditional I/O exiting" is set or clear.. Of Course both bits are
>> clear, which may
>> make host/guest to the incorrect field.
       [...]
>>
>> is it?  Radim, could you help me double check it?
> I don't think it is correct to have neither,

   yes, I also mentioned that  this may make host/guest to the 
_incorrect_ field. :)
   but the fix is to set CPU_BASED_UNCOND_IO_EXITING and clear 
CPU_BASED_USE_IO_BITMAPS..

-             CPU_BASED_USE_IO_BITMAPS |
+             CPU_BASED_UNCOND_IO_EXITING |


also drop io_bitmap_a/io_bitmap_b that are not used at all(as 
CPU_BASED_USE_IO_BITMAPS is clear).

>   though, SMD describes them
> as an alternative:
>
> 33.3.2.1 PIC Virtualization
>    If the VMM is not supporting direct access to any I/O ports from a
>    guest, it can set the unconditional-I/O-exiting in the VM-execution
>    control field instead of activating I/O bitmaps.
>
> And a quick test shows that we get a VM entry failure if both of them
> are clear, although I can't find that explicitly in SDM.
I think.. if both of them are clear, guests/host share host's I/O 
resource, as mentioned, which may make host/guest to the _incorrect_ field..
CPU may have some check to prevent this happening.

Quan
Alibaba Cloud

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] KVM: VMX: remove I/O port 0x80 bypass on Intel hosts
  2017-12-01 18:21 [PATCH 1/2] KVM: VMX: remove I/O port 0x80 bypass on Intel hosts Jim Mattson
  2017-12-01 18:21 ` [PATCH 2/2] KVM: VMX: Use just one page for I/O permission bitmaps Jim Mattson
@ 2017-12-02  0:34 ` Krish Sadhukhan
  2017-12-04 12:44 ` Wanpeng Li
  2017-12-05 21:32 ` Radim Krčmář
  3 siblings, 0 replies; 20+ messages in thread
From: Krish Sadhukhan @ 2017-12-02  0:34 UTC (permalink / raw)
  To: Jim Mattson, kvm, P J P; +Cc: Andrew Honig



On 12/01/2017 10:21 AM, Jim Mattson wrote:
> From: Andrew Honig <ahonig@google.com>
>
> This fixes CVE-2017-1000407.
>
> KVM allows guests to directly access I/O port 0x80 on Intel hosts.  If
> the guest floods this port with writes it generates exceptions and
> instability in the host kernel, leading to a crash.  With this change
> guest writes to port 0x80 on Intel will behave the same as they
> currently behave on AMD systems.
>
> Prevent the flooding by removing the code that sets port 0x80 as a
> passthrough port.  This is essentially the same as upstream patch
> 99f85a28a78e96d28907fe036e1671a218fee597, except that patch was
> for AMD chipsets and this patch is for Intel.
>
> Signed-off-by: Andrew Honig <ahonig@google.com>
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>   arch/x86/kvm/vmx.c | 5 -----
>   1 file changed, 5 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index d2b452d66363..d16abd1808eb 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -6753,12 +6753,7 @@ static __init int hardware_setup(void)
>   	memset(vmx_vmread_bitmap, 0xff, PAGE_SIZE);
>   	memset(vmx_vmwrite_bitmap, 0xff, PAGE_SIZE);
>   
> -	/*
> -	 * Allow direct access to the PC debug port (it is often used for I/O
> -	 * delays, but the vmexits simply slow things down).
> -	 */
>   	memset(vmx_io_bitmap_a, 0xff, PAGE_SIZE);
> -	clear_bit(0x80, vmx_io_bitmap_a);
>   
>   	memset(vmx_io_bitmap_b, 0xff, PAGE_SIZE);
>   
Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] KVM: VMX: remove I/O port 0x80 bypass on Intel hosts
  2017-12-01 18:21 [PATCH 1/2] KVM: VMX: remove I/O port 0x80 bypass on Intel hosts Jim Mattson
  2017-12-01 18:21 ` [PATCH 2/2] KVM: VMX: Use just one page for I/O permission bitmaps Jim Mattson
  2017-12-02  0:34 ` [PATCH 1/2] KVM: VMX: remove I/O port 0x80 bypass on Intel hosts Krish Sadhukhan
@ 2017-12-04 12:44 ` Wanpeng Li
  2017-12-04 17:10   ` Jim Mattson
  2017-12-05 21:32 ` Radim Krčmář
  3 siblings, 1 reply; 20+ messages in thread
From: Wanpeng Li @ 2017-12-04 12:44 UTC (permalink / raw)
  To: Jim Mattson; +Cc: kvm, P J P, Andrew Honig

Hi Jim,
2017-12-02 2:21 GMT+08:00 Jim Mattson <jmattson@google.com>:
> From: Andrew Honig <ahonig@google.com>
>
> This fixes CVE-2017-1000407.

Do you observe a real issue on recent Intel boxes? In addition, how to
reproduce? Actually there is a testcase in kvm-unit-tests which can
run 10 million times ioport 0x80 write and I didn't observe any issue
before. :)

Regards,
Wanpeng Li

>
> KVM allows guests to directly access I/O port 0x80 on Intel hosts.  If
> the guest floods this port with writes it generates exceptions and
> instability in the host kernel, leading to a crash.  With this change
> guest writes to port 0x80 on Intel will behave the same as they
> currently behave on AMD systems.
>
> Prevent the flooding by removing the code that sets port 0x80 as a
> passthrough port.  This is essentially the same as upstream patch
> 99f85a28a78e96d28907fe036e1671a218fee597, except that patch was
> for AMD chipsets and this patch is for Intel.
>
> Signed-off-by: Andrew Honig <ahonig@google.com>
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/kvm/vmx.c | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index d2b452d66363..d16abd1808eb 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -6753,12 +6753,7 @@ static __init int hardware_setup(void)
>         memset(vmx_vmread_bitmap, 0xff, PAGE_SIZE);
>         memset(vmx_vmwrite_bitmap, 0xff, PAGE_SIZE);
>
> -       /*
> -        * Allow direct access to the PC debug port (it is often used for I/O
> -        * delays, but the vmexits simply slow things down).
> -        */
>         memset(vmx_io_bitmap_a, 0xff, PAGE_SIZE);
> -       clear_bit(0x80, vmx_io_bitmap_a);
>
>         memset(vmx_io_bitmap_b, 0xff, PAGE_SIZE);
>
> --
> 2.15.0.531.g2ccb3012c9-goog
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] KVM: VMX: remove I/O port 0x80 bypass on Intel hosts
  2017-12-04 12:44 ` Wanpeng Li
@ 2017-12-04 17:10   ` Jim Mattson
  2017-12-05  7:19     ` Wanpeng Li
  2017-12-05 11:24     ` Quan Xu
  0 siblings, 2 replies; 20+ messages in thread
From: Jim Mattson @ 2017-12-04 17:10 UTC (permalink / raw)
  To: Wanpeng Li; +Cc: kvm, P J P, Andrew Honig

Google has carried this patch since long before my time. I would
suggest modifying the kvm-unit-test to (a) unroll the loop ~1000
times, and (b) execute out to port 0x80 from ~64 vcpu threads in
parallel.

On Mon, Dec 4, 2017 at 4:44 AM, Wanpeng Li <kernellwp@gmail.com> wrote:
> Hi Jim,
> 2017-12-02 2:21 GMT+08:00 Jim Mattson <jmattson@google.com>:
>> From: Andrew Honig <ahonig@google.com>
>>
>> This fixes CVE-2017-1000407.
>
> Do you observe a real issue on recent Intel boxes? In addition, how to
> reproduce? Actually there is a testcase in kvm-unit-tests which can
> run 10 million times ioport 0x80 write and I didn't observe any issue
> before. :)
>
> Regards,
> Wanpeng Li
>
>>
>> KVM allows guests to directly access I/O port 0x80 on Intel hosts.  If
>> the guest floods this port with writes it generates exceptions and
>> instability in the host kernel, leading to a crash.  With this change
>> guest writes to port 0x80 on Intel will behave the same as they
>> currently behave on AMD systems.
>>
>> Prevent the flooding by removing the code that sets port 0x80 as a
>> passthrough port.  This is essentially the same as upstream patch
>> 99f85a28a78e96d28907fe036e1671a218fee597, except that patch was
>> for AMD chipsets and this patch is for Intel.
>>
>> Signed-off-by: Andrew Honig <ahonig@google.com>
>> Signed-off-by: Jim Mattson <jmattson@google.com>
>> ---
>>  arch/x86/kvm/vmx.c | 5 -----
>>  1 file changed, 5 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index d2b452d66363..d16abd1808eb 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -6753,12 +6753,7 @@ static __init int hardware_setup(void)
>>         memset(vmx_vmread_bitmap, 0xff, PAGE_SIZE);
>>         memset(vmx_vmwrite_bitmap, 0xff, PAGE_SIZE);
>>
>> -       /*
>> -        * Allow direct access to the PC debug port (it is often used for I/O
>> -        * delays, but the vmexits simply slow things down).
>> -        */
>>         memset(vmx_io_bitmap_a, 0xff, PAGE_SIZE);
>> -       clear_bit(0x80, vmx_io_bitmap_a);
>>
>>         memset(vmx_io_bitmap_b, 0xff, PAGE_SIZE);
>>
>> --
>> 2.15.0.531.g2ccb3012c9-goog
>>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] KVM: VMX: remove I/O port 0x80 bypass on Intel hosts
  2017-12-04 17:10   ` Jim Mattson
@ 2017-12-05  7:19     ` Wanpeng Li
  2017-12-05 11:24     ` Quan Xu
  1 sibling, 0 replies; 20+ messages in thread
From: Wanpeng Li @ 2017-12-05  7:19 UTC (permalink / raw)
  To: Jim Mattson; +Cc: kvm, P J P, Andrew Honig

2017-12-05 1:10 GMT+08:00 Jim Mattson <jmattson@google.com>:
> Google has carried this patch since long before my time. I would
> suggest modifying the kvm-unit-test to (a) unroll the loop ~1000
> times, and (b) execute out to port 0x80 from ~64 vcpu threads in
> parallel.

Thanks for the information. :)

Regards,
Wanpeng Li

>
> On Mon, Dec 4, 2017 at 4:44 AM, Wanpeng Li <kernellwp@gmail.com> wrote:
>> Hi Jim,
>> 2017-12-02 2:21 GMT+08:00 Jim Mattson <jmattson@google.com>:
>>> From: Andrew Honig <ahonig@google.com>
>>>
>>> This fixes CVE-2017-1000407.
>>
>> Do you observe a real issue on recent Intel boxes? In addition, how to
>> reproduce? Actually there is a testcase in kvm-unit-tests which can
>> run 10 million times ioport 0x80 write and I didn't observe any issue
>> before. :)
>>
>> Regards,
>> Wanpeng Li
>>
>>>
>>> KVM allows guests to directly access I/O port 0x80 on Intel hosts.  If
>>> the guest floods this port with writes it generates exceptions and
>>> instability in the host kernel, leading to a crash.  With this change
>>> guest writes to port 0x80 on Intel will behave the same as they
>>> currently behave on AMD systems.
>>>
>>> Prevent the flooding by removing the code that sets port 0x80 as a
>>> passthrough port.  This is essentially the same as upstream patch
>>> 99f85a28a78e96d28907fe036e1671a218fee597, except that patch was
>>> for AMD chipsets and this patch is for Intel.
>>>
>>> Signed-off-by: Andrew Honig <ahonig@google.com>
>>> Signed-off-by: Jim Mattson <jmattson@google.com>
>>> ---
>>>  arch/x86/kvm/vmx.c | 5 -----
>>>  1 file changed, 5 deletions(-)
>>>
>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>> index d2b452d66363..d16abd1808eb 100644
>>> --- a/arch/x86/kvm/vmx.c
>>> +++ b/arch/x86/kvm/vmx.c
>>> @@ -6753,12 +6753,7 @@ static __init int hardware_setup(void)
>>>         memset(vmx_vmread_bitmap, 0xff, PAGE_SIZE);
>>>         memset(vmx_vmwrite_bitmap, 0xff, PAGE_SIZE);
>>>
>>> -       /*
>>> -        * Allow direct access to the PC debug port (it is often used for I/O
>>> -        * delays, but the vmexits simply slow things down).
>>> -        */
>>>         memset(vmx_io_bitmap_a, 0xff, PAGE_SIZE);
>>> -       clear_bit(0x80, vmx_io_bitmap_a);
>>>
>>>         memset(vmx_io_bitmap_b, 0xff, PAGE_SIZE);
>>>
>>> --
>>> 2.15.0.531.g2ccb3012c9-goog
>>>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] KVM: VMX: remove I/O port 0x80 bypass on Intel hosts
  2017-12-04 17:10   ` Jim Mattson
  2017-12-05  7:19     ` Wanpeng Li
@ 2017-12-05 11:24     ` Quan Xu
  2017-12-05 12:15       ` Wanpeng Li
  1 sibling, 1 reply; 20+ messages in thread
From: Quan Xu @ 2017-12-05 11:24 UTC (permalink / raw)
  To: Jim Mattson, Wanpeng Li; +Cc: kvm, P J P, Andrew Honig



On 2017/12/05 01:10, Jim Mattson wrote:
> Google has carried this patch since long before my time. I would
> suggest modifying the kvm-unit-test to (a) unroll the loop ~1000
> times, and (b) execute out to port 0x80 from ~64 vcpu threads in
> parallel.

Jim,  could you reproduce it on all of your machine types?
I can't reproduce it on 2 types of my machine.. btw, it is not a good 
idle to
open reproduction here:(..

I do believe you can reproduce on you machine. could you have a try:
without guest, flood 80 port with writes in host kernel..  does it lead 
to a crash?
if host kernel crash, does it a machine hardware issue, kernel issue, or 
both?


Quan
Alibaba Cloud


> On Mon, Dec 4, 2017 at 4:44 AM, Wanpeng Li <kernellwp@gmail.com> wrote:
>> Hi Jim,
>> 2017-12-02 2:21 GMT+08:00 Jim Mattson <jmattson@google.com>:
>>> From: Andrew Honig <ahonig@google.com>
>>>
>>> This fixes CVE-2017-1000407.
>> Do you observe a real issue on recent Intel boxes? In addition, how to
>> reproduce? Actually there is a testcase in kvm-unit-tests which can
>> run 10 million times ioport 0x80 write and I didn't observe any issue
>> before. :)
>>
>> Regards,
>> Wanpeng Li
>>
>>> KVM allows guests to directly access I/O port 0x80 on Intel hosts.  If
>>> the guest floods this port with writes it generates exceptions and
>>> instability in the host kernel, leading to a crash.  With this change
>>> guest writes to port 0x80 on Intel will behave the same as they
>>> currently behave on AMD systems.
>>>
>>> Prevent the flooding by removing the code that sets port 0x80 as a
>>> passthrough port.  This is essentially the same as upstream patch
>>> 99f85a28a78e96d28907fe036e1671a218fee597, except that patch was
>>> for AMD chipsets and this patch is for Intel.
>>>
>>> Signed-off-by: Andrew Honig <ahonig@google.com>
>>> Signed-off-by: Jim Mattson <jmattson@google.com>
>>> ---
>>>   arch/x86/kvm/vmx.c | 5 -----
>>>   1 file changed, 5 deletions(-)
>>>
>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>> index d2b452d66363..d16abd1808eb 100644
>>> --- a/arch/x86/kvm/vmx.c
>>> +++ b/arch/x86/kvm/vmx.c
>>> @@ -6753,12 +6753,7 @@ static __init int hardware_setup(void)
>>>          memset(vmx_vmread_bitmap, 0xff, PAGE_SIZE);
>>>          memset(vmx_vmwrite_bitmap, 0xff, PAGE_SIZE);
>>>
>>> -       /*
>>> -        * Allow direct access to the PC debug port (it is often used for I/O
>>> -        * delays, but the vmexits simply slow things down).
>>> -        */
>>>          memset(vmx_io_bitmap_a, 0xff, PAGE_SIZE);
>>> -       clear_bit(0x80, vmx_io_bitmap_a);
>>>
>>>          memset(vmx_io_bitmap_b, 0xff, PAGE_SIZE);
>>>
>>> --
>>> 2.15.0.531.g2ccb3012c9-goog
>>>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] KVM: VMX: remove I/O port 0x80 bypass on Intel hosts
  2017-12-05 11:24     ` Quan Xu
@ 2017-12-05 12:15       ` Wanpeng Li
  0 siblings, 0 replies; 20+ messages in thread
From: Wanpeng Li @ 2017-12-05 12:15 UTC (permalink / raw)
  To: Quan Xu; +Cc: Jim Mattson, kvm, P J P, Andrew Honig

2017-12-05 19:24 GMT+08:00 Quan Xu <quan.xu0@gmail.com>:
>
>
> On 2017/12/05 01:10, Jim Mattson wrote:
>>
>> Google has carried this patch since long before my time. I would
>> suggest modifying the kvm-unit-test to (a) unroll the loop ~1000
>> times, and (b) execute out to port 0x80 from ~64 vcpu threads in
>> parallel.
>
>
> Jim,  could you reproduce it on all of your machine types?
> I can't reproduce it on 2 types of my machine.. btw, it is not a good idle
> to
> open reproduction here:(..

My fault, however, luckily, we still can't reproduce by the method
which Jim pointed out. :)

Regards,
Wanpeng Li

>
> I do believe you can reproduce on you machine. could you have a try:
> without guest, flood 80 port with writes in host kernel..  does it lead to a
> crash?
> if host kernel crash, does it a machine hardware issue, kernel issue, or
> both?
>
>
> Quan
> Alibaba Cloud
>
>
>
>> On Mon, Dec 4, 2017 at 4:44 AM, Wanpeng Li <kernellwp@gmail.com> wrote:
>>>
>>> Hi Jim,
>>> 2017-12-02 2:21 GMT+08:00 Jim Mattson <jmattson@google.com>:
>>>>
>>>> From: Andrew Honig <ahonig@google.com>
>>>>
>>>> This fixes CVE-2017-1000407.
>>>
>>> Do you observe a real issue on recent Intel boxes? In addition, how to
>>> reproduce? Actually there is a testcase in kvm-unit-tests which can
>>> run 10 million times ioport 0x80 write and I didn't observe any issue
>>> before. :)
>>>
>>> Regards,
>>> Wanpeng Li
>>>
>>>> KVM allows guests to directly access I/O port 0x80 on Intel hosts.  If
>>>> the guest floods this port with writes it generates exceptions and
>>>> instability in the host kernel, leading to a crash.  With this change
>>>> guest writes to port 0x80 on Intel will behave the same as they
>>>> currently behave on AMD systems.
>>>>
>>>> Prevent the flooding by removing the code that sets port 0x80 as a
>>>> passthrough port.  This is essentially the same as upstream patch
>>>> 99f85a28a78e96d28907fe036e1671a218fee597, except that patch was
>>>> for AMD chipsets and this patch is for Intel.
>>>>
>>>> Signed-off-by: Andrew Honig <ahonig@google.com>
>>>> Signed-off-by: Jim Mattson <jmattson@google.com>
>>>> ---
>>>>   arch/x86/kvm/vmx.c | 5 -----
>>>>   1 file changed, 5 deletions(-)
>>>>
>>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>>> index d2b452d66363..d16abd1808eb 100644
>>>> --- a/arch/x86/kvm/vmx.c
>>>> +++ b/arch/x86/kvm/vmx.c
>>>> @@ -6753,12 +6753,7 @@ static __init int hardware_setup(void)
>>>>          memset(vmx_vmread_bitmap, 0xff, PAGE_SIZE);
>>>>          memset(vmx_vmwrite_bitmap, 0xff, PAGE_SIZE);
>>>>
>>>> -       /*
>>>> -        * Allow direct access to the PC debug port (it is often used
>>>> for I/O
>>>> -        * delays, but the vmexits simply slow things down).
>>>> -        */
>>>>          memset(vmx_io_bitmap_a, 0xff, PAGE_SIZE);
>>>> -       clear_bit(0x80, vmx_io_bitmap_a);
>>>>
>>>>          memset(vmx_io_bitmap_b, 0xff, PAGE_SIZE);
>>>>
>>>> --
>>>> 2.15.0.531.g2ccb3012c9-goog
>>>>
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] KVM: VMX: remove I/O port 0x80 bypass on Intel hosts
  2017-12-01 18:21 [PATCH 1/2] KVM: VMX: remove I/O port 0x80 bypass on Intel hosts Jim Mattson
                   ` (2 preceding siblings ...)
  2017-12-04 12:44 ` Wanpeng Li
@ 2017-12-05 21:32 ` Radim Krčmář
  2017-12-06  0:36   ` Jim Mattson
  2017-12-06  1:38   ` Quan Xu
  3 siblings, 2 replies; 20+ messages in thread
From: Radim Krčmář @ 2017-12-05 21:32 UTC (permalink / raw)
  To: Jim Mattson; +Cc: kvm, P J P, Andrew Honig

2017-12-01 10:21-0800, Jim Mattson:
> From: Andrew Honig <ahonig@google.com>
> 
> This fixes CVE-2017-1000407.
> 
> KVM allows guests to directly access I/O port 0x80 on Intel hosts.  If
> the guest floods this port with writes it generates exceptions and
> instability in the host kernel, leading to a crash.  With this change
> guest writes to port 0x80 on Intel will behave the same as they
> currently behave on AMD systems.
> 
> Prevent the flooding by removing the code that sets port 0x80 as a
> passthrough port.  This is essentially the same as upstream patch
> 99f85a28a78e96d28907fe036e1671a218fee597, except that patch was
> for AMD chipsets and this patch is for Intel.
> 
> Signed-off-by: Andrew Honig <ahonig@google.com>
> Signed-off-by: Jim Mattson <jmattson@google.com>

Fixes: fdef3ad1b386 ("KVM: VMX: Enable io bitmaps to avoid IO port 0x80 VMEXITs")
Cc: <stable@vger.kernel.org>

Applied, thanks.  The commit that introduced it boasted 3-5% performance
improvements when compiling the kernel -- have you noticed regressions?

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] KVM: VMX: remove I/O port 0x80 bypass on Intel hosts
  2017-12-05 21:32 ` Radim Krčmář
@ 2017-12-06  0:36   ` Jim Mattson
  2017-12-06  1:38   ` Quan Xu
  1 sibling, 0 replies; 20+ messages in thread
From: Jim Mattson @ 2017-12-06  0:36 UTC (permalink / raw)
  To: Radim Krčmář; +Cc: kvm list, P J P, Andrew Honig

I don't think I believe the performance claim of the original
commit...unless the kernel build test was spewing its output onto a
serial port, in which case the performance claim is mischaracterized.

On Tue, Dec 5, 2017 at 1:32 PM, Radim Krčmář <rkrcmar@redhat.com> wrote:
> 2017-12-01 10:21-0800, Jim Mattson:
>> From: Andrew Honig <ahonig@google.com>
>>
>> This fixes CVE-2017-1000407.
>>
>> KVM allows guests to directly access I/O port 0x80 on Intel hosts.  If
>> the guest floods this port with writes it generates exceptions and
>> instability in the host kernel, leading to a crash.  With this change
>> guest writes to port 0x80 on Intel will behave the same as they
>> currently behave on AMD systems.
>>
>> Prevent the flooding by removing the code that sets port 0x80 as a
>> passthrough port.  This is essentially the same as upstream patch
>> 99f85a28a78e96d28907fe036e1671a218fee597, except that patch was
>> for AMD chipsets and this patch is for Intel.
>>
>> Signed-off-by: Andrew Honig <ahonig@google.com>
>> Signed-off-by: Jim Mattson <jmattson@google.com>
>
> Fixes: fdef3ad1b386 ("KVM: VMX: Enable io bitmaps to avoid IO port 0x80 VMEXITs")
> Cc: <stable@vger.kernel.org>
>
> Applied, thanks.  The commit that introduced it boasted 3-5% performance
> improvements when compiling the kernel -- have you noticed regressions?

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] KVM: VMX: remove I/O port 0x80 bypass on Intel hosts
  2017-12-05 21:32 ` Radim Krčmář
  2017-12-06  0:36   ` Jim Mattson
@ 2017-12-06  1:38   ` Quan Xu
  1 sibling, 0 replies; 20+ messages in thread
From: Quan Xu @ 2017-12-06  1:38 UTC (permalink / raw)
  To: Radim Krčmář, Jim Mattson; +Cc: kvm, P J P, Andrew Honig



On 2017/12/06 05:32, Radim Krčmář wrote:
> 2017-12-01 10:21-0800, Jim Mattson:
>> From: Andrew Honig <ahonig@google.com>
>>
>> This fixes CVE-2017-1000407.
>>
>> KVM allows guests to directly access I/O port 0x80 on Intel hosts.  If
>> the guest floods this port with writes it generates exceptions and
>> instability in the host kernel, leading to a crash.  With this change
>> guest writes to port 0x80 on Intel will behave the same as they
>> currently behave on AMD systems.
>>
>> Prevent the flooding by removing the code that sets port 0x80 as a
>> passthrough port.  This is essentially the same as upstream patch
>> 99f85a28a78e96d28907fe036e1671a218fee597, except that patch was
>> for AMD chipsets and this patch is for Intel.
>>
>> Signed-off-by: Andrew Honig <ahonig@google.com>
>> Signed-off-by: Jim Mattson <jmattson@google.com>
Reviewed-by: Quan Xu <quan.xu0@gmail.com>


Quan
Alibaba Cloud
> Fixes: fdef3ad1b386 ("KVM: VMX: Enable io bitmaps to avoid IO port 0x80 VMEXITs")
> Cc: <stable@vger.kernel.org>
>
> Applied, thanks.  The commit that introduced it boasted 3-5% performance
> improvements when compiling the kernel -- have you noticed regressions?
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2017-12-08  2:04 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-01 18:21 [PATCH 1/2] KVM: VMX: remove I/O port 0x80 bypass on Intel hosts Jim Mattson
2017-12-01 18:21 ` [PATCH 2/2] KVM: VMX: Use just one page for I/O permission bitmaps Jim Mattson
2017-12-04 18:30   ` kbuild test robot
2017-12-04 18:34     ` Jim Mattson
2017-12-05 21:26   ` Radim Krčmář
2017-12-06  0:16     ` Jim Mattson
2017-12-06 11:17     ` Quan Xu
2017-12-06 18:19       ` Jim Mattson
2017-12-07  2:33         ` Quan Xu
2017-12-07 17:06           ` Radim Krčmář
2017-12-08  2:04             ` Quan Xu
2017-12-02  0:34 ` [PATCH 1/2] KVM: VMX: remove I/O port 0x80 bypass on Intel hosts Krish Sadhukhan
2017-12-04 12:44 ` Wanpeng Li
2017-12-04 17:10   ` Jim Mattson
2017-12-05  7:19     ` Wanpeng Li
2017-12-05 11:24     ` Quan Xu
2017-12-05 12:15       ` Wanpeng Li
2017-12-05 21:32 ` Radim Krčmář
2017-12-06  0:36   ` Jim Mattson
2017-12-06  1:38   ` Quan Xu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox