| Message ID | 2b8eff6f-4b35-ce3a-c716-eb8fb7461eb3@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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.
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
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),