On Mon, Feb 07, 2022 at 06:21:00PM +0000, Jane Malalane wrote:
> Add XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_xapic and
> XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_x2apic to report accelerated xapic
> and x2apic, on x86 hardware.
> No such features are currently implemented on AMD hardware.
>
> For that purpose, also add an arch-specific "capabilities" parameter
> to struct xen_sysctl_physinfo.
>
> Signed-off-by: Jane Malalane <jane.malal...@citrix.com>
> Suggested-by: Andrew Cooper <andrew.coop...@citrix.com>
Tag order should be inverted, first Suggested-by, then SoB.
> ---
> CC: Wei Liu <w...@xen.org>
> CC: Anthony PERARD <anthony.per...@citrix.com>
> CC: Juergen Gross <jgr...@suse.com>
> CC: Andrew Cooper <andrew.coop...@citrix.com>
> CC: George Dunlap <george.dun...@citrix.com>
> CC: Jan Beulich <jbeul...@suse.com>
> CC: Julien Grall <jul...@xen.org>
> CC: Stefano Stabellini <sstabell...@kernel.org>
> CC: Volodymyr Babchuk <volodymyr_babc...@epam.com>
> CC: Bertrand Marquis <bertrand.marq...@arm.com>
> CC: Jun Nakajima <jun.nakaj...@intel.com>
> CC: Kevin Tian <kevin.t...@intel.com>
> CC: "Roger Pau Monné" <roger....@citrix.com>
>
> v2:
> * Use one macro LIBXL_HAVE_PHYSINFO_ASSISTED_APIC instead of two
> * Pass xcpyshinfo as a pointer in libxl__arch_get_physinfo
> * Set assisted_x{2}apic_available to be conditional upon "bsp" and
> annotate it with __ro_after_init
> * Change XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_X{2}APIC to
> .._X86_ASSISTED_X{2}APIC
> * Keep XEN_SYSCTL_PHYSCAP_X86_ASSISTED_X{2}APIC contained within
> sysctl.h
> * Fix padding introduced in struct xen_sysctl_physinfo and bump
> XEN_SYSCTL_INTERFACE_VERSION
> ---
> tools/golang/xenlight/helpers.gen.go | 4 ++++
> tools/golang/xenlight/types.gen.go | 6 ++++++
> tools/include/libxl.h | 7 +++++++
> tools/libs/light/libxl.c | 3 +++
> tools/libs/light/libxl_arch.h | 4 ++++
> tools/libs/light/libxl_arm.c | 5 +++++
> tools/libs/light/libxl_types.idl | 2 ++
> tools/libs/light/libxl_x86.c | 11 +++++++++++
> tools/ocaml/libs/xc/xenctrl.ml | 5 +++++
> tools/ocaml/libs/xc/xenctrl.mli | 5 +++++
> tools/xl/xl_info.c | 6 ++++--
> xen/arch/x86/hvm/vmx/vmcs.c | 9 +++++++++
> xen/arch/x86/include/asm/domain.h | 3 +++
> xen/arch/x86/sysctl.c | 7 +++++++
> xen/include/public/sysctl.h | 8 +++++++-
> 15 files changed, 82 insertions(+), 3 deletions(-)
>
> diff --git a/tools/golang/xenlight/helpers.gen.go
> b/tools/golang/xenlight/helpers.gen.go
> index b746ff1081..dd4e6c9f14 100644
> --- a/tools/golang/xenlight/helpers.gen.go
> +++ b/tools/golang/xenlight/helpers.gen.go
> @@ -3373,6 +3373,8 @@ x.CapVmtrace = bool(xc.cap_vmtrace)
> x.CapVpmu = bool(xc.cap_vpmu)
> x.CapGnttabV1 = bool(xc.cap_gnttab_v1)
> x.CapGnttabV2 = bool(xc.cap_gnttab_v2)
> +x.CapAssistedXapic = bool(xc.cap_assisted_xapic)
> +x.CapAssistedX2Apic = bool(xc.cap_assisted_x2apic)
>
> return nil}
>
> @@ -3407,6 +3409,8 @@ xc.cap_vmtrace = C.bool(x.CapVmtrace)
> xc.cap_vpmu = C.bool(x.CapVpmu)
> xc.cap_gnttab_v1 = C.bool(x.CapGnttabV1)
> xc.cap_gnttab_v2 = C.bool(x.CapGnttabV2)
> +xc.cap_assisted_xapic = C.bool(x.CapAssistedXapic)
> +xc.cap_assisted_x2apic = C.bool(x.CapAssistedX2Apic)
>
> return nil
> }
> diff --git a/tools/golang/xenlight/types.gen.go
> b/tools/golang/xenlight/types.gen.go
> index b1e84d5258..5f384b767c 100644
> --- a/tools/golang/xenlight/types.gen.go
> +++ b/tools/golang/xenlight/types.gen.go
> @@ -389,6 +389,10 @@ RunHotplugScripts Defbool
> DriverDomain Defbool
> Passthrough Passthrough
> XendSuspendEvtchnCompat Defbool
> +ArchX86 struct {
> +AssistedXapic Defbool
> +AssistedX2Apic Defbool
Don't you need some indentation here?
Also name would better be Assistedx{2}APIC IMO if possible. Having a
capital 'X' and lowercase 'apic' looks really strange.
> +}
> }
>
> type DomainRestoreParams struct {
> @@ -1014,6 +1018,8 @@ CapVmtrace bool
> CapVpmu bool
> CapGnttabV1 bool
> CapGnttabV2 bool
> +CapAssistedXApic bool
> +CapAssistedX2apic bool
> }
>
> type Connectorinfo struct {
> diff --git a/tools/include/libxl.h b/tools/include/libxl.h
> index 2bbbd21f0b..924e142628 100644
> --- a/tools/include/libxl.h
> +++ b/tools/include/libxl.h
> @@ -528,6 +528,13 @@
> #define LIBXL_HAVE_MAX_GRANT_VERSION 1
>
> /*
> + * LIBXL_HAVE_PHYSINFO_ASSISTED_APIC indicates that libxl_physinfo has
> + * cap_assisted_x{2}apic fields, which indicates the availability of x{2}APIC
> + * hardware assisted virtualization.
> + */
> +#define LIBXL_HAVE_PHYSINFO_ASSISTED_APIC 1
> +
> +/*
> * libxl ABI compatibility
> *
> * The only guarantee which libxl makes regarding ABI compatibility
> diff --git a/tools/libs/light/libxl.c b/tools/libs/light/libxl.c
> index 667ae6409b..fabb474221 100644
> --- a/tools/libs/light/libxl.c
> +++ b/tools/libs/light/libxl.c
> @@ -15,6 +15,7 @@
> #include "libxl_osdeps.h"
>
> #include "libxl_internal.h"
> +#include "libxl_arch.h"
>
> int libxl_ctx_alloc(libxl_ctx **pctx, int version,
> unsigned flags, xentoollog_logger * lg)
> @@ -410,6 +411,8 @@ int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo
> *physinfo)
> physinfo->cap_gnttab_v2 =
> !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_gnttab_v2);
>
> + libxl__arch_get_physinfo(physinfo, &xcphysinfo);
> +
> GC_FREE;
> return 0;
> }
> diff --git a/tools/libs/light/libxl_arch.h b/tools/libs/light/libxl_arch.h
> index 1522ecb97f..207ceac6a1 100644
> --- a/tools/libs/light/libxl_arch.h
> +++ b/tools/libs/light/libxl_arch.h
> @@ -86,6 +86,10 @@ int libxl__arch_extra_memory(libxl__gc *gc,
> uint64_t *out);
>
> _hidden
> +void libxl__arch_get_physinfo(libxl_physinfo *physinfo,
> + const xc_physinfo_t *xcphysinfo);
> +
> +_hidden
> void libxl__arch_update_domain_config(libxl__gc *gc,
> libxl_domain_config *dst,
> const libxl_domain_config *src);
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index eef1de0939..39fdca1b49 100644
> --- a/tools/libs/light/libxl_arm.c
> +++ b/tools/libs/light/libxl_arm.c
> @@ -1431,6 +1431,11 @@ int libxl__arch_passthrough_mode_setdefault(libxl__gc
> *gc,
> return rc;
> }
>
> +void libxl__arch_get_physinfo(libxl_physinfo *physinfo,
> + const xc_physinfo_t *xcphysinfo)
> +{
> +}
> +
> void libxl__arch_update_domain_config(libxl__gc *gc,
> libxl_domain_config *dst,
> const libxl_domain_config *src)
> diff --git a/tools/libs/light/libxl_types.idl
> b/tools/libs/light/libxl_types.idl
> index 2a42da2f7d..42ac6c357b 100644
> --- a/tools/libs/light/libxl_types.idl
> +++ b/tools/libs/light/libxl_types.idl
> @@ -1068,6 +1068,8 @@ libxl_physinfo = Struct("physinfo", [
> ("cap_vpmu", bool),
> ("cap_gnttab_v1", bool),
> ("cap_gnttab_v2", bool),
> + ("cap_assisted_xapic", bool),
> + ("cap_assisted_x2apic", bool),
> ], dir=DIR_OUT)
>
> libxl_connectorinfo = Struct("connectorinfo", [
> diff --git a/tools/libs/light/libxl_x86.c b/tools/libs/light/libxl_x86.c
> index 1feadebb18..e0a06ecfe3 100644
> --- a/tools/libs/light/libxl_x86.c
> +++ b/tools/libs/light/libxl_x86.c
> @@ -866,6 +866,17 @@ int libxl__arch_passthrough_mode_setdefault(libxl__gc
> *gc,
> return rc;
> }
>
> +void libxl__arch_get_physinfo(libxl_physinfo *physinfo,
> + const xc_physinfo_t *xcphysinfo)
> +{
> + physinfo->cap_assisted_xapic =
> + !!(xcphysinfo->arch_capabilities &
> + XEN_SYSCTL_PHYSCAP_X86_ASSISTED_XAPIC);
> + physinfo->cap_assisted_x2apic =
> + !!(xcphysinfo->arch_capabilities &
> + XEN_SYSCTL_PHYSCAP_X86_ASSISTED_X2APIC);
> +}
> +
> void libxl__arch_update_domain_config(libxl__gc *gc,
> libxl_domain_config *dst,
> const libxl_domain_config *src)
> diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
> index 7503031d8f..7ce832d605 100644
> --- a/tools/ocaml/libs/xc/xenctrl.ml
> +++ b/tools/ocaml/libs/xc/xenctrl.ml
> @@ -127,6 +127,10 @@ type physinfo_cap_flag =
> | CAP_Gnttab_v1
> | CAP_Gnttab_v2
>
> +type physinfo_cap_arch_flag =
> + | CAP_ARCH_ASSISTED_XAPIC
> + | CAP_ARCH_ASSISTED_X2APIC
> +
> type physinfo =
> {
> threads_per_core : int;
> @@ -139,6 +143,7 @@ type physinfo =
> scrub_pages : nativeint;
> (* XXX hw_cap *)
> capabilities : physinfo_cap_flag list;
> + arch_capabilities : physinfo_cap_arch_flag list;
I know very little about Ocaml, but I think you are not setting this
field anywhere? I would expect a call to ocaml_list_to_c_bitmap and
then you will likely need to define XEN_SYSCTL_PHYSCAP_X86_MAX so you
can check the options. See XEN_SYSCTL_PHYSCAP_MAX for example.
> max_nr_cpus : int;
> }
>
> diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
> index d1d9c9247a..a2b15130ee 100644
> --- a/tools/ocaml/libs/xc/xenctrl.mli
> +++ b/tools/ocaml/libs/xc/xenctrl.mli
> @@ -112,6 +112,10 @@ type physinfo_cap_flag =
> | CAP_Gnttab_v1
> | CAP_Gnttab_v2
>
> +type physinfo_cap_arch_flag =
> + | CAP_ARCH_ASSISTED_XAPIC
> + | CAP_ARCH_ASSISTED_X2APIC
> +
> type physinfo = {
> threads_per_core : int;
> cores_per_socket : int;
> @@ -122,6 +126,7 @@ type physinfo = {
> free_pages : nativeint;
> scrub_pages : nativeint;
> capabilities : physinfo_cap_flag list;
> + arch_capabilities : physinfo_cap_arch_flag list;
> max_nr_cpus : int; (** compile-time max possible number of nr_cpus *)
> }
> type version = { major : int; minor : int; extra : string; }
> diff --git a/tools/xl/xl_info.c b/tools/xl/xl_info.c
> index 712b7638b0..3205270754 100644
> --- a/tools/xl/xl_info.c
> +++ b/tools/xl/xl_info.c
> @@ -210,7 +210,7 @@ static void output_physinfo(void)
> info.hw_cap[4], info.hw_cap[5], info.hw_cap[6], info.hw_cap[7]
> );
>
> - maybe_printf("virt_caps :%s%s%s%s%s%s%s%s%s%s%s\n",
> + maybe_printf("virt_caps :%s%s%s%s%s%s%s%s%s%s%s%s%s\n",
> info.cap_pv ? " pv" : "",
> info.cap_hvm ? " hvm" : "",
> info.cap_hvm && info.cap_hvm_directio ? " hvm_directio" : "",
> @@ -221,7 +221,9 @@ static void output_physinfo(void)
> info.cap_vmtrace ? " vmtrace" : "",
> info.cap_vpmu ? " vpmu" : "",
> info.cap_gnttab_v1 ? " gnttab-v1" : "",
> - info.cap_gnttab_v2 ? " gnttab-v2" : ""
> + info.cap_gnttab_v2 ? " gnttab-v2" : "",
> + info.cap_assisted_xapic ? " assisted_xapic" : "",
> + info.cap_assisted_x2apic ? " assisted_x2apic" : ""
> );
>
> vinfo = libxl_get_version_info(ctx);
> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
> index 7ab15e07a0..4060aef1bd 100644
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -343,6 +343,15 @@ static int vmx_init_vmcs_config(bool bsp)
> MSR_IA32_VMX_PROCBASED_CTLS2, &mismatch);
> }
>
> + /* Check whether hardware supports accelerated xapic and x2apic. */
> + if ( bsp )
> + {
> + assisted_xapic_available = cpu_has_vmx_virtualize_apic_accesses;
> + assisted_x2apic_available = (cpu_has_vmx_apic_reg_virt ||
> + cpu_has_vmx_virtual_intr_delivery) &&
> + cpu_has_vmx_virtualize_x2apic_mode;
> + }
> +
> /* The IA32_VMX_EPT_VPID_CAP MSR exists only when EPT or VPID available
> */
> if ( _vmx_secondary_exec_control & (SECONDARY_EXEC_ENABLE_EPT |
> SECONDARY_EXEC_ENABLE_VPID) )
> diff --git a/xen/arch/x86/include/asm/domain.h
> b/xen/arch/x86/include/asm/domain.h
> index e62e109598..72431df26d 100644
> --- a/xen/arch/x86/include/asm/domain.h
> +++ b/xen/arch/x86/include/asm/domain.h
> @@ -756,6 +756,9 @@ static inline void pv_inject_sw_interrupt(unsigned int
> vector)
> : is_pv_32bit_domain(d) ? PV32_VM_ASSIST_MASK \
> : PV64_VM_ASSIST_MASK)
>
> +extern bool assisted_xapic_available;
> +extern bool assisted_x2apic_available;
> +
> #endif /* __ASM_DOMAIN_H__ */
>
> /*
> diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
> index aff52a13f3..642cc96985 100644
> --- a/xen/arch/x86/sysctl.c
> +++ b/xen/arch/x86/sysctl.c
> @@ -69,6 +69,9 @@ struct l3_cache_info {
> unsigned long size;
> };
>
> +bool __ro_after_init assisted_xapic_available;
> +bool __ro_after_init assisted_x2apic_available;
You could likely shorten this by dropping the _available suffix.
Thanks, Roger.
