On March 19, 2018 3:31:24 PM PDT, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> wrote:
>On March 19, 2018 11:29:45 AM EDT, Tim Shearer
><TShearer@xxxxxxxxxxxxxxx> wrote:
>>From: Tim Shearer <tshearer@xxxxxxxxxxxxxxx>
>>
>>This reverts commit 8eb73e2d410f00d383023fe41c0c25c6195b7389.
>>This reverts commit d59d51f088014f25c2562de59b9abff4f42a7468.
>>
>>Reintroducing the I/O port 0x80 bypass, the removal of which incurred
>a
>>significant performance penalty for NFV guests due to excessive and
>>time-consuming VMEXITs.
>
>
>.. and adds back a security issue.
>
>Perhaps a better approach would be to add an QEMU flag to
>enable/disable access it port 80?
>
>Similar to how spec_ctrl is done?
>>
>>Signed-off-by: Tim Shearer <tshearer@xxxxxxxxxxxxxxx>
>>---
>> arch/x86/kvm/vmx.c | 23 ++++++++++++++++++++---
>> 1 file changed, 20 insertions(+), 3 deletions(-)
>>
>>diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>index 051dab74e4e9..a2194fe02f2a 100644
>>--- a/arch/x86/kvm/vmx.c
>>+++ b/arch/x86/kvm/vmx.c
>>@@ -928,6 +928,8 @@ 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_VMREAD_BITMAP,
>> VMX_VMWRITE_BITMAP,
>> VMX_BITMAP_NR
>>@@ -935,6 +937,8 @@ 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_vmread_bitmap
>>(vmx_bitmap[VMX_VMREAD_BITMAP])
>>#define vmx_vmwrite_bitmap
>>(vmx_bitmap[VMX_VMWRITE_BITMAP])
>>
>>@@ -3701,7 +3705,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_UNCOND_IO_EXITING |
>>+ CPU_BASED_USE_IO_BITMAPS |
>> CPU_BASED_MOV_DR_EXITING |
>> CPU_BASED_USE_TSC_OFFSETING |
>> CPU_BASED_INVLPG_EXITING |
>>@@ -5657,6 +5661,10 @@ 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));
>>@@ -6985,6 +6993,15 @@ 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);
>>+
>> if (setup_vmcs_config(&vmcs_config) < 0) {
>> r = -EIO;
>> goto out;
>>@@ -10830,8 +10847,8 @@ static int prepare_vmcs02(struct kvm_vcpu
>>*vcpu, struct vmcs12 *vmcs12,
>> }
>>
>> /*
>>- * A vmexit (to either L1 hypervisor or L0 userspace) is always
>>needed
>>- * for I/O port accesses.
>>+ * 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;
What is the security issue? Port 0x80 used for other purposes on real hardware? In that case, the host kernel would need to know about it, and could disable this hack, no?
(Such a machine would have a hard time running Linux, too. That being said, I don't think it would be a bad idea to induce something like X86_FEATURE_NOIODELAY which would patch out those writes; KVM guests could set it.)
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
