Re: [PATCH v6 2/2] x86/xen: Allow per-domain usage of hardware virtualized APIC

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

On Tue, Mar 08, 2022 at 05:36:43PM +0000, Jane Malalane wrote:
> Introduce a new per-domain creation x86 specific flag to
> select whether hardware assisted virtualization should be used for
> x{2}APIC.
> 
> A per-domain option is added to xl in order to select the usage of
> x{2}APIC hardware assisted virtualization, as well as a global
> configuration option.
> 
> Having all APIC interaction exit to Xen for emulation is slow and can
> induce much overhead. Hardware can speed up x{2}APIC by decoding the
> APIC access and providing a VM exit with a more specific exit reason
> than a regular EPT fault or by altogether avoiding a VM exit.
> 
> On the other hand, being able to disable x{2}APIC hardware assisted
> virtualization can be useful for testing and debugging purposes.
> 
> Note: vmx_install_vlapic_mapping doesn't require modifications
> regardless of whether the guest has "Virtualize APIC accesses" enabled
> or not, i.e., setting the APIC_ACCESS_ADDR VMCS field is fine so long
> as virtualize_apic_accesses is supported by the CPU.
> 
> Suggested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Signed-off-by: Jane Malalane <jane.malalane@xxxxxxxxxx>
> ---
> CC: Wei Liu <wl@xxxxxxx>
> CC: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> CC: Juergen Gross <jgross@xxxxxxxx>
> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> CC: George Dunlap <george.dunlap@xxxxxxxxxx>
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> CC: Julien Grall <julien@xxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Christian Lindig <christian.lindig@xxxxxxxxxx>
> CC: David Scott <dave@xxxxxxxxxx>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
> CC: "Roger Pau Monné" <roger.pau@xxxxxxxxxx>
> 
> v6:
>  * Use ENODEV instead of EINVAL when rejecting assisted_x{2}apic
>    for PV guests
>  * Move has_assisted_x{2}apic macros out of an Intel specific header
>  * Remove references to Intel specific features in documentation
> 
> v5:
>  * Revert v4 changes in vmx_vlapic_msr_changed(), preserving the use of
>    the has_assisted_x{2}apic macros
>  * Following changes in assisted_x{2}apic_available definitions in
>    patch 1, retighten conditionals for setting
>    XEN_HVM_CPUID_APIC_ACCESS_VIRT and XEN_HVM_CPUID_X2APIC_VIRT in
>    cpuid_hypervisor_leaves()
> 
> v4:
>  * Add has_assisted_x{2}apic macros and use them where appropriate
>  * Replace CPU checks with per-domain assisted_x{2}apic control
>    options in vmx_vlapic_msr_changed() and cpuid_hypervisor_leaves(),
>    following edits to assisted_x{2}apic_available definitions in
>    patch 1
>    Note: new assisted_x{2}apic_available definitions make later
>    cpu_has_vmx_apic_reg_virt and cpu_has_vmx_virtual_intr_delivery
>    checks redundant in vmx_vlapic_msr_changed()
> 
> v3:
>  * Change info in xl.cfg to better express reality and fix
>    capitalization of x{2}apic
>  * Move "physinfo" variable definition to the beggining of
>    libxl__domain_build_info_setdefault()
>  * Reposition brackets in if statement to match libxl coding style
>  * Shorten logic in libxl__arch_domain_build_info_setdefault()
>  * Correct dprintk message in arch_sanitise_domain_config()
>  * Make appropriate changes in vmx_vlapic_msr_changed() and
>    cpuid_hypervisor_leaves() for amended "assisted_x2apic" bit
>  * Remove unneeded parantheses
> 
> v2:
>  * Add a LIBXL_HAVE_ASSISTED_APIC macro
>  * Pass xcpyshinfo as a pointer in libxl__arch_get_physinfo
>  * Add a return statement in now "int"
>    libxl__arch_domain_build_info_setdefault()
>  * Preserve libxl__arch_domain_build_info_setdefault 's location in
>    libxl_create.c
>  * Correct x{2}apic default setting logic in
>    libxl__arch_domain_prepare_config()
>  * Correct logic for parsing assisted_x{2}apic host/guest options in
>    xl_parse.c and initialize them to -1 in xl.c
>  * Use guest options directly in vmx_vlapic_msr_changed
>  * Fix indentation of bool assisted_x{2}apic in struct hvm_domain
>  * Add a change in xenctrl_stubs.c to pass xenctrl ABI checks
> ---
>  docs/man/xl.cfg.5.pod.in              | 15 +++++++++++++++
>  docs/man/xl.conf.5.pod.in             | 12 ++++++++++++
>  tools/golang/xenlight/helpers.gen.go  | 12 ++++++++++++
>  tools/golang/xenlight/types.gen.go    |  2 ++
>  tools/include/libxl.h                 |  7 +++++++
>  tools/libs/light/libxl_arch.h         |  5 +++--
>  tools/libs/light/libxl_arm.c          |  7 +++++--
>  tools/libs/light/libxl_create.c       | 22 +++++++++++++---------
>  tools/libs/light/libxl_types.idl      |  2 ++
>  tools/libs/light/libxl_x86.c          | 27 +++++++++++++++++++++++++--
>  tools/ocaml/libs/xc/xenctrl.ml        |  2 ++
>  tools/ocaml/libs/xc/xenctrl.mli       |  2 ++
>  tools/ocaml/libs/xc/xenctrl_stubs.c   |  2 +-
>  tools/xl/xl.c                         |  8 ++++++++
>  tools/xl/xl.h                         |  2 ++
>  tools/xl/xl_parse.c                   | 16 ++++++++++++++++
>  xen/arch/x86/domain.c                 | 28 +++++++++++++++++++++++++++-
>  xen/arch/x86/hvm/vmx/vmcs.c           |  4 ++++
>  xen/arch/x86/hvm/vmx/vmx.c            | 13 ++++---------
>  xen/arch/x86/include/asm/hvm/domain.h |  6 ++++++
>  xen/arch/x86/include/asm/hvm/hvm.h    |  6 ++++++
>  xen/arch/x86/traps.c                  |  5 +++--
>  xen/include/public/arch-x86/xen.h     |  2 ++
>  23 files changed, 179 insertions(+), 28 deletions(-)
> 
> diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
> index b98d161398..b4239fcc5e 100644
> --- a/docs/man/xl.cfg.5.pod.in
> +++ b/docs/man/xl.cfg.5.pod.in
> @@ -1862,6 +1862,21 @@ firmware tables when using certain older guest 
> Operating
>  Systems. These tables have been superseded by newer constructs within
>  the ACPI tables.
>  
> +=item B<assisted_xapic=BOOLEAN>
> +
> +B<(x86 only)> Enables or disables hardware assisted virtualization for
> +xAPIC. With this option enabled, a memory-mapped APIC access will be
> +decoded by hardware and either issue a more specific VM exit than just
> +an EPT fault, or altogether avoid a VM exit. The
> +default is settable via L<xl.conf(5)>.

Sorry, replied to a v2 version of the patch instead of this one. Could
we use 'p2m fault' or maybe 'guest physmap fault' intead of EPT (as
that's Intel specific).

Thanks, Roger.