Re: [PATCH] x86/vmx: Remove IO bitmap from minimal VMX requirements

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Jan 15, 2021 at 03:30:50PM +0100, Hubert Jasudowicz wrote:
> This patch is a result of a downstream bug report[1]. Xen fails to
> create a HVM domain while running under VMware Fusion 12.1.0 on
> a modern Intel Core i9 CPU:
> 
> (XEN) VMX: CPU0 has insufficient CPU-Based Exec Control (b5b9fffe; requires 
> 2299968c)
> (XEN) VMX: failed to initialise.
> 
> It seems that Apple hypervisor API doesn't support this feature[2].
> 
> Move this bit from minimal required features to optional.
> 
> [1] https://github.com/CERT-Polska/drakvuf-sandbox/issues/418
> [2] https://developer.apple.com/documentation/hypervisor/cpu_based_io_bitmaps
> 
> Signed-off-by: Hubert Jasudowicz <hubert.jasudowicz@xxxxxxx>
> ---
>  xen/arch/x86/hvm/vmx/vmcs.c        | 8 +++++---
>  xen/include/asm-x86/hvm/vmx/vmcs.h | 2 ++
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
> index 164535f8f0..bad4d6e206 100644
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -276,10 +276,10 @@ static int vmx_init_vmcs_config(void)
>             CPU_BASED_MONITOR_EXITING |
>             CPU_BASED_MWAIT_EXITING |
>             CPU_BASED_MOV_DR_EXITING |
> -           CPU_BASED_ACTIVATE_IO_BITMAP |
>             CPU_BASED_USE_TSC_OFFSETING |
>             CPU_BASED_RDTSC_EXITING);
>      opt = (CPU_BASED_ACTIVATE_MSR_BITMAP |
> +           CPU_BASED_ACTIVATE_IO_BITMAP |
>             CPU_BASED_TPR_SHADOW |
>             CPU_BASED_MONITOR_TRAP_FLAG |
>             CPU_BASED_ACTIVATE_SECONDARY_CONTROLS);
> @@ -1168,8 +1168,10 @@ static int construct_vmcs(struct vcpu *v)
>      }
>  
>      /* I/O access bitmap. */
> -    __vmwrite(IO_BITMAP_A, __pa(d->arch.hvm.io_bitmap));
> -    __vmwrite(IO_BITMAP_B, __pa(d->arch.hvm.io_bitmap) + PAGE_SIZE);
> +    if ( cpu_has_vmx_io_bitmap ) {
> +        __vmwrite(IO_BITMAP_A, __pa(d->arch.hvm.io_bitmap));
> +        __vmwrite(IO_BITMAP_B, __pa(d->arch.hvm.io_bitmap) + PAGE_SIZE);
> +    }

Maybe I'm missing something, but don't you need to expand
EXIT_REASON_IO_INSTRUCTION in vmx_vmexit_handler when there's no IO
bitmap support so that all the emulation is bypassed and the IO port
access is replayed by Xen?

I think you don't strictly need to modify EXIT_REASON_IO_INSTRUCTION
and can use the existing g2m_ioport_list infrastructure to add the
allowed ports present on the bitmap and prevent them from being
forwarded to the IOREQ server.

Thanks, Roger.