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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 15/01/2021 14:30, 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>

For others reviewing, this was my suggestion to fix it.

The IO port bitmap is only used as a performance optimisation for legacy
BIOS code using port 0x80/0xed for IO delays, which isn't a good enough
reason for the feature to be mandatory.

Nested virt like this is primarily used for ease of development.  The
VMExit IO path should DTRT, even for a PVH dom0.

> 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
> @@ -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 ) {

Brace on newline.  Can be fixed on commit - no need to resend just for this.

Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

~Andrew

> +        __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_virtual_intr_delivery )
>      {
>