| Message ID | 20220207182101.31941-3-jane.malalane@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen: Report and use hardware APIC virtualization capabilities | expand |
> On 7 Feb 2022, at 18:21, Jane Malalane <jane.malalane@citrix.com> wrote: > > Introduce a new per-domain creation x86 specific flag to > select whether hardware assisted virtualization should be used for > x{2}APIC. > > --- > docs/man/xl.cfg.5.pod.in | 10 ++++++++++ > docs/man/xl.conf.5.pod.in | 12 ++++++++++++ > tools/golang/xenlight/helpers.gen.go | 12 ++++++++++++ > 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 | 23 ++++++++++++++--------- > tools/libs/light/libxl_types.idl | 2 ++ > tools/libs/light/libxl_x86.c | 31 +++++++++++++++++++++++++++++-- > 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 | 14 +++++--------- > xen/arch/x86/include/asm/hvm/domain.h | 6 ++++++ > xen/arch/x86/traps.c | 8 ++++---- > xen/include/public/arch-x86/xen.h | 2 ++ > 21 files changed, 173 insertions(+), 30 deletions(-) The changes to the OCaml part are minimal. Acked-by: Christian Lindig <christian.lindig@citrix.com> — C
On Mon, Feb 07, 2022 at 06:21:01PM +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 vitualization, 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 running APIC > read/write accesses without taking a VM exit. > > Being able to disable x{2}APIC hardware assisted vitualization can be > useful for testing and debugging purposes. I think you have agreed with Jan some changes to the description regarding the purpose of the commit. > > Signed-off-by: Jane Malalane <jane.malalane@citrix.com> > Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > CC: Wei Liu <wl@xen.org> > CC: Anthony PERARD <anthony.perard@citrix.com> > CC: Juergen Gross <jgross@suse.com> > CC: Andrew Cooper <andrew.cooper3@citrix.com> > CC: George Dunlap <george.dunlap@citrix.com> > CC: Jan Beulich <jbeulich@suse.com> > CC: Julien Grall <julien@xen.org> > CC: Stefano Stabellini <sstabellini@kernel.org> > CC: Christian Lindig <christian.lindig@citrix.com> > CC: David Scott <dave@recoil.org> > CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> > CC: "Roger Pau Monné" <roger.pau@citrix.com> > > 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 | 10 ++++++++++ > docs/man/xl.conf.5.pod.in | 12 ++++++++++++ > tools/golang/xenlight/helpers.gen.go | 12 ++++++++++++ > 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 | 23 ++++++++++++++--------- > tools/libs/light/libxl_types.idl | 2 ++ > tools/libs/light/libxl_x86.c | 31 +++++++++++++++++++++++++++++-- > 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 | 14 +++++--------- > xen/arch/x86/include/asm/hvm/domain.h | 6 ++++++ > xen/arch/x86/traps.c | 8 ++++---- > xen/include/public/arch-x86/xen.h | 2 ++ > 21 files changed, 173 insertions(+), 30 deletions(-) > > diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in > index b98d161398..1d98bbd182 100644 > --- a/docs/man/xl.cfg.5.pod.in > +++ b/docs/man/xl.cfg.5.pod.in > @@ -1862,6 +1862,16 @@ 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. > +This allows accessing APIC registers without a VM-exit. > +The default is settable via L<xl.conf(5)>. > + > +=item B<assisted_x2APIC=BOOLEAN> > +B<(x86 only)> Enables or disables hardware assisted virtualization for x2apic. > +This allows accessing APIC registers without a VM-exit. > +The default is settable via L<xl.conf(5)>. > + > =item B<nx=BOOLEAN> > > B<(x86 only)> Hides or exposes the No-eXecute capability. This allows a guest > diff --git a/docs/man/xl.conf.5.pod.in b/docs/man/xl.conf.5.pod.in > index df20c08137..30993827e5 100644 > --- a/docs/man/xl.conf.5.pod.in > +++ b/docs/man/xl.conf.5.pod.in > @@ -107,6 +107,18 @@ Sets the default value for the C<max_grant_version> domain config value. > > Default: maximum grant version supported by the hypervisor. > > +=item B<assisted_xAPIC=BOOLEAN> > + > +If enabled, domains will use xAPIC hardware assisted virtualization by default. > + > +Default: enabled if supported. > + > +=item B<assisted_x2APIC=BOOLEAN> > + > +If enabled, domains will use x2APIC hardware assisted virtualization by default. > + > +Default: enabled if supported. We don't capitalize xl options, so I would suggest to lowercase APIC for all the option names. > + > =item B<vif.default.script="PATH"> > > Configures the default hotplug script used by virtual network devices. > diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go > index dd4e6c9f14..90e7b9b205 100644 > --- a/tools/golang/xenlight/helpers.gen.go > +++ b/tools/golang/xenlight/helpers.gen.go > @@ -636,6 +636,12 @@ x.Passthrough = Passthrough(xc.passthrough) > if err := x.XendSuspendEvtchnCompat.fromC(&xc.xend_suspend_evtchn_compat);err != nil { > return fmt.Errorf("converting field XendSuspendEvtchnCompat: %v", err) > } > +if err := x.ArchX86.AssistedXapic.fromC(&xc.arch_x86.assisted_xapic);err != nil { > +return fmt.Errorf("converting field ArchX86.AssistedXapic: %v", err) > +} > +if err := x.ArchX86.AssistedX2Apic.fromC(&xc.arch_x86.assisted_x2apic);err != nil { > +return fmt.Errorf("converting field ArchX86.AssistedX2Apic: %v", err) > +} > > return nil} > > @@ -679,6 +685,12 @@ xc.passthrough = C.libxl_passthrough(x.Passthrough) > if err := x.XendSuspendEvtchnCompat.toC(&xc.xend_suspend_evtchn_compat); err != nil { > return fmt.Errorf("converting field XendSuspendEvtchnCompat: %v", err) > } > +if err := x.ArchX86.AssistedXapic.toC(&xc.arch_x86.assisted_xapic); err != nil { > +return fmt.Errorf("converting field ArchX86.AssistedXapic: %v", err) > +} > +if err := x.ArchX86.AssistedX2Apic.toC(&xc.arch_x86.assisted_x2apic); err != nil { > +return fmt.Errorf("converting field ArchX86.AssistedX2Apic: %v", err) > +} > > return nil > } > diff --git a/tools/include/libxl.h b/tools/include/libxl.h > index 924e142628..83944c17ae 100644 > --- a/tools/include/libxl.h > +++ b/tools/include/libxl.h > @@ -535,6 +535,13 @@ > #define LIBXL_HAVE_PHYSINFO_ASSISTED_APIC 1 > > /* > + * LIBXL_HAVE_ASSISTED_APIC indicates that libxl_domain_build_info has > + * assisted_x{2}apic fields, for enabling hardware assisted virtualization for > + * x{2}apic per domain. > + */ > +#define LIBXL_HAVE_ASSISTED_APIC 1 > + > +/* > * libxl ABI compatibility > * > * The only guarantee which libxl makes regarding ABI compatibility > diff --git a/tools/libs/light/libxl_arch.h b/tools/libs/light/libxl_arch.h > index 207ceac6a1..03b89929e6 100644 > --- a/tools/libs/light/libxl_arch.h > +++ b/tools/libs/light/libxl_arch.h > @@ -71,8 +71,9 @@ void libxl__arch_domain_create_info_setdefault(libxl__gc *gc, > libxl_domain_create_info *c_info); > > _hidden > -void libxl__arch_domain_build_info_setdefault(libxl__gc *gc, > - libxl_domain_build_info *b_info); > +int libxl__arch_domain_build_info_setdefault(libxl__gc *gc, > + libxl_domain_build_info *b_info, > + const libxl_physinfo *physinfo); > > _hidden > int libxl__arch_passthrough_mode_setdefault(libxl__gc *gc, > diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c > index 39fdca1b49..ba5b8f433f 100644 > --- a/tools/libs/light/libxl_arm.c > +++ b/tools/libs/light/libxl_arm.c > @@ -1384,8 +1384,9 @@ void libxl__arch_domain_create_info_setdefault(libxl__gc *gc, > } > } > > -void libxl__arch_domain_build_info_setdefault(libxl__gc *gc, > - libxl_domain_build_info *b_info) > +int libxl__arch_domain_build_info_setdefault(libxl__gc *gc, > + libxl_domain_build_info *b_info, > + const libxl_physinfo *physinfo) > { > /* ACPI is disabled by default */ > libxl_defbool_setdefault(&b_info->acpi, false); > @@ -1399,6 +1400,8 @@ void libxl__arch_domain_build_info_setdefault(libxl__gc *gc, > memset(&b_info->u, '\0', sizeof(b_info->u)); > b_info->type = LIBXL_DOMAIN_TYPE_INVALID; > libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PVH); > + > + return 0; > } > > int libxl__arch_passthrough_mode_setdefault(libxl__gc *gc, > diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c > index d7a40d7550..843e523df9 100644 > --- a/tools/libs/light/libxl_create.c > +++ b/tools/libs/light/libxl_create.c > @@ -264,7 +264,20 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, > if (!b_info->event_channels) > b_info->event_channels = 1023; > > - libxl__arch_domain_build_info_setdefault(gc, b_info); > + libxl_physinfo info; The definition of info needs to be at the top of the function, together with the rest of the variable definitions. > + > + rc = libxl_get_physinfo(CTX, &info); > + if (rc) { > + LOG(ERROR, "failed to get hypervisor info"); > + return rc; > + } > + > + rc = libxl__arch_domain_build_info_setdefault(gc, b_info, &info); > + if (rc) { > + LOG(ERROR, "unable to set domain arch build info defaults"); > + return rc; > + } > + > libxl_defbool_setdefault(&b_info->dm_restrict, false); > > if (b_info->iommu_memkb == LIBXL_MEMKB_DEFAULT) > @@ -457,14 +470,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, > } > > if (b_info->max_grant_version == LIBXL_MAX_GRANT_DEFAULT) { > - libxl_physinfo info; > - > - rc = libxl_get_physinfo(CTX, &info); > - if (rc) { > - LOG(ERROR, "failed to get hypervisor info"); > - return rc; > - } > - > if (info.cap_gnttab_v2) > b_info->max_grant_version = 2; > else if (info.cap_gnttab_v1) > diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl > index 42ac6c357b..db5eb0a0b3 100644 > --- a/tools/libs/light/libxl_types.idl > +++ b/tools/libs/light/libxl_types.idl > @@ -648,6 +648,8 @@ libxl_domain_build_info = Struct("domain_build_info",[ > ("vuart", libxl_vuart_type), > ])), > ("arch_x86", Struct(None, [("msr_relaxed", libxl_defbool), > + ("assisted_xapic", libxl_defbool), > + ("assisted_x2apic", libxl_defbool), > ])), > # Alternate p2m is not bound to any architecture or guest type, as it is > # supported by x86 HVM and ARM support is planned. > diff --git a/tools/libs/light/libxl_x86.c b/tools/libs/light/libxl_x86.c > index e0a06ecfe3..f0fa0ceea2 100644 > --- a/tools/libs/light/libxl_x86.c > +++ b/tools/libs/light/libxl_x86.c > @@ -23,6 +23,14 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc, > if (libxl_defbool_val(d_config->b_info.arch_x86.msr_relaxed)) > config->arch.misc_flags |= XEN_X86_MSR_RELAXED; > > + if (d_config->c_info.type != LIBXL_DOMAIN_TYPE_PV) > + { Coding style for libxl is to place the bracket in the same line as the if. > + if (libxl_defbool_val(d_config->b_info.arch_x86.assisted_xapic)) > + config->arch.misc_flags |= XEN_X86_ASSISTED_XAPIC; > + > + if (libxl_defbool_val(d_config->b_info.arch_x86.assisted_x2apic)) > + config->arch.misc_flags |= XEN_X86_ASSISTED_X2APIC; > + } > return 0; > } > > @@ -819,11 +827,30 @@ void libxl__arch_domain_create_info_setdefault(libxl__gc *gc, > { > } > > -void libxl__arch_domain_build_info_setdefault(libxl__gc *gc, > - libxl_domain_build_info *b_info) > +int libxl__arch_domain_build_info_setdefault(libxl__gc *gc, > + libxl_domain_build_info *b_info, > + const libxl_physinfo *physinfo) > { > libxl_defbool_setdefault(&b_info->acpi, true); > libxl_defbool_setdefault(&b_info->arch_x86.msr_relaxed, false); > + > + if (b_info->type != LIBXL_DOMAIN_TYPE_PV) > + { > + libxl_defbool_setdefault(&b_info->arch_x86.assisted_xapic, > + physinfo->cap_assisted_xapic); > + libxl_defbool_setdefault(&b_info->arch_x86.assisted_x2apic, > + physinfo->cap_assisted_x2apic); > + } > + > + if (b_info->type == LIBXL_DOMAIN_TYPE_PV && > + (!libxl_defbool_is_default(b_info->arch_x86.assisted_xapic) || > + !libxl_defbool_is_default(b_info->arch_x86.assisted_x2apic))) You could just do: if (b_info->type != LIBXL_DOMAIN_TYPE_PV) { libxl_defbool_setdefault(&b_info->arch_x86.assisted_xapic, physinfo->cap_assisted_xapic); libxl_defbool_setdefault(&b_info->arch_x86.assisted_x2apic, physinfo->cap_assisted_x2apic); } else if (!libxl_defbool_is_default(b_info->arch_x86.assisted_xapic) || !libxl_defbool_is_default(b_info->arch_x86.assisted_x2apic)) /* ERROR */ > + { > + LOG(ERROR, "Interrupt Controller Virtualization not supported for PV"); > + return ERROR_INVAL; > + } > + > + return 0; > } > > int libxl__arch_passthrough_mode_setdefault(libxl__gc *gc, > diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml > index 7ce832d605..cce30d8731 100644 > --- a/tools/ocaml/libs/xc/xenctrl.ml > +++ b/tools/ocaml/libs/xc/xenctrl.ml > @@ -50,6 +50,8 @@ type x86_arch_emulation_flags = > > type x86_arch_misc_flags = > | X86_MSR_RELAXED > + | X86_ASSISTED_XAPIC > + | X86_ASSISTED_X2APIC > > type xen_x86_arch_domainconfig = > { > diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli > index a2b15130ee..67a22ec15c 100644 > --- a/tools/ocaml/libs/xc/xenctrl.mli > +++ b/tools/ocaml/libs/xc/xenctrl.mli > @@ -44,6 +44,8 @@ type x86_arch_emulation_flags = > > type x86_arch_misc_flags = > | X86_MSR_RELAXED > + | X86_ASSISTED_XAPIC > + | X86_ASSISTED_X2APIC > > type xen_x86_arch_domainconfig = { > emulation_flags: x86_arch_emulation_flags list; > diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c > index 5b4fe72c8d..0aa957d379 100644 > --- a/tools/ocaml/libs/xc/xenctrl_stubs.c > +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c > @@ -239,7 +239,7 @@ CAMLprim value stub_xc_domain_create(value xch, value wanted_domid, value config > > cfg.arch.misc_flags = ocaml_list_to_c_bitmap > /* ! x86_arch_misc_flags X86_ none */ > - /* ! XEN_X86_ XEN_X86_MSR_RELAXED all */ > + /* ! XEN_X86_ XEN_X86_ASSISTED_X2APIC max */ We would usually define an XEN_X86_MISC_MAX that would point to XEN_X86_ASSISTED_X2APIC currently. > (VAL_MISC_FLAGS); > > #undef VAL_MISC_FLAGS > diff --git a/tools/xl/xl.c b/tools/xl/xl.c > index 2d1ec18ea3..31eb223309 100644 > --- a/tools/xl/xl.c > +++ b/tools/xl/xl.c > @@ -57,6 +57,8 @@ int max_grant_frames = -1; > int max_maptrack_frames = -1; > int max_grant_version = LIBXL_MAX_GRANT_DEFAULT; > libxl_domid domid_policy = INVALID_DOMID; > +int assisted_xapic = -1; > +int assisted_x2apic = -1; > > xentoollog_level minmsglevel = minmsglevel_default; > > @@ -201,6 +203,12 @@ static void parse_global_config(const char *configfile, > if (!xlu_cfg_get_long (config, "claim_mode", &l, 0)) > claim_mode = l; > > + if (!xlu_cfg_get_long (config, "assisted_xapic", &l, 0)) > + assisted_xapic = l; > + > + if (!xlu_cfg_get_long (config, "assisted_x2apic", &l, 0)) > + assisted_x2apic = l; > + > xlu_cfg_replace_string (config, "remus.default.netbufscript", > &default_remus_netbufscript, 0); > xlu_cfg_replace_string (config, "colo.default.proxyscript", > diff --git a/tools/xl/xl.h b/tools/xl/xl.h > index c5c4bedbdd..528deb3feb 100644 > --- a/tools/xl/xl.h > +++ b/tools/xl/xl.h > @@ -286,6 +286,8 @@ extern libxl_bitmap global_vm_affinity_mask; > extern libxl_bitmap global_hvm_affinity_mask; > extern libxl_bitmap global_pv_affinity_mask; > extern libxl_domid domid_policy; > +extern int assisted_xapic; > +extern int assisted_x2apic; > > enum output_format { > OUTPUT_FORMAT_JSON, > diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c > index 117fcdcb2b..0ab9b145fe 100644 > --- a/tools/xl/xl_parse.c > +++ b/tools/xl/xl_parse.c > @@ -1681,6 +1681,22 @@ void parse_config_data(const char *config_source, > xlu_cfg_get_defbool(config, "vpt_align", &b_info->u.hvm.vpt_align, 0); > xlu_cfg_get_defbool(config, "apic", &b_info->apic, 0); > > + e = xlu_cfg_get_long(config, "assisted_xapic", &l , 0); > + if ((e == ESRCH && assisted_xapic != -1)) /* use global default if present */ ^ no need for the extra parentheses here and below. > + libxl_defbool_set(&b_info->arch_x86.assisted_xapic, assisted_xapic); > + else if (!e) > + libxl_defbool_set(&b_info->arch_x86.assisted_xapic, l); > + else > + exit(1); > + > + e = xlu_cfg_get_long(config, "assisted_x2apic", &l, 0); > + if ((e == ESRCH && assisted_x2apic != -1)) /* use global default if present */ > + libxl_defbool_set(&b_info->arch_x86.assisted_x2apic, assisted_x2apic); > + else if (!e) > + libxl_defbool_set(&b_info->arch_x86.assisted_x2apic, l); > + else > + exit(1); > + > switch (xlu_cfg_get_list(config, "viridian", > &viridian, &num_viridian, 1)) > { > diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c > index ef1812dc14..9033a0e181 100644 > --- a/xen/arch/x86/domain.c > +++ b/xen/arch/x86/domain.c > @@ -619,6 +619,8 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config) > bool hvm = config->flags & XEN_DOMCTL_CDF_hvm; > bool hap = config->flags & XEN_DOMCTL_CDF_hap; > bool nested_virt = config->flags & XEN_DOMCTL_CDF_nested_virt; > + bool assisted_xapic = config->arch.misc_flags & XEN_X86_ASSISTED_XAPIC; > + bool assisted_x2apic = config->arch.misc_flags & XEN_X86_ASSISTED_X2APIC; > unsigned int max_vcpus; > > if ( hvm ? !hvm_enabled : !IS_ENABLED(CONFIG_PV) ) > @@ -685,13 +687,31 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config) > } > } > > - if ( config->arch.misc_flags & ~XEN_X86_MSR_RELAXED ) > + if ( config->arch.misc_flags & ~(XEN_X86_MSR_RELAXED | > + XEN_X86_ASSISTED_XAPIC | > + XEN_X86_ASSISTED_X2APIC) ) > { > dprintk(XENLOG_INFO, "Invalid arch misc flags %#x\n", > config->arch.misc_flags); > return -EINVAL; > } > > + if ( (assisted_xapic || assisted_x2apic) && !hvm ) > + { > + dprintk(XENLOG_INFO, > + "Interrupt Controller Virtualization not supported for PV\n"); > + return -EINVAL; > + } > + > + if ( (assisted_xapic && !assisted_xapic_available) || > + (assisted_x2apic && !assisted_x2apic_available) ) > + { > + dprintk(XENLOG_INFO, > + "Hardware assisted x%sAPIC requested but not available\n", > + assisted_xapic && !assisted_xapic_available ? "" : "2"); > + return -EINVAL; > + } > + > return 0; > } > > @@ -863,6 +883,12 @@ int arch_domain_create(struct domain *d, > > d->arch.msr_relaxed = config->arch.misc_flags & XEN_X86_MSR_RELAXED; > > + d->arch.hvm.assisted_xapic = > + config->arch.misc_flags & XEN_X86_ASSISTED_XAPIC; > + > + d->arch.hvm.assisted_x2apic = > + config->arch.misc_flags & XEN_X86_ASSISTED_X2APIC; > + > return 0; > > fail: > diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c > index 4060aef1bd..614db5c4a4 100644 > --- a/xen/arch/x86/hvm/vmx/vmcs.c > +++ b/xen/arch/x86/hvm/vmx/vmcs.c > @@ -1157,6 +1157,10 @@ static int construct_vmcs(struct vcpu *v) > __vmwrite(PLE_WINDOW, ple_window); > } > > + if ( !v->domain->arch.hvm.assisted_xapic ) > + v->arch.hvm.vmx.secondary_exec_control &= > + ~SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES; > + > if ( cpu_has_vmx_secondary_exec_control ) > __vmwrite(SECONDARY_VM_EXEC_CONTROL, > v->arch.hvm.vmx.secondary_exec_control); > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > index 36c8a12cfe..3c9ff60154 100644 > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -3333,16 +3333,11 @@ static void vmx_install_vlapic_mapping(struct vcpu *v) > > void vmx_vlapic_msr_changed(struct vcpu *v) > { > - int virtualize_x2apic_mode; > struct vlapic *vlapic = vcpu_vlapic(v); > unsigned int msr; > > - virtualize_x2apic_mode = ( (cpu_has_vmx_apic_reg_virt || > - cpu_has_vmx_virtual_intr_delivery) && > - cpu_has_vmx_virtualize_x2apic_mode ); > - > - if ( !cpu_has_vmx_virtualize_apic_accesses && > - !virtualize_x2apic_mode ) > + if ( ! v->domain->arch.hvm.assisted_xapic && > + ! v->domain->arch.hvm.assisted_x2apic ) ^ extra space. > return; > > vmx_vmcs_enter(v); > @@ -3352,7 +3347,8 @@ void vmx_vlapic_msr_changed(struct vcpu *v) > if ( !vlapic_hw_disabled(vlapic) && > (vlapic_base_address(vlapic) == APIC_DEFAULT_PHYS_BASE) ) > { > - if ( virtualize_x2apic_mode && vlapic_x2apic_mode(vlapic) ) > + if ( v->domain->arch.hvm.assisted_x2apic > + && vlapic_x2apic_mode(vlapic) ) > { > v->arch.hvm.vmx.secondary_exec_control |= > SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE; > @@ -3373,7 +3369,7 @@ void vmx_vlapic_msr_changed(struct vcpu *v) > vmx_clear_msr_intercept(v, MSR_X2APIC_SELF, VMX_MSR_W); > } > } > - else > + else if ( v->domain->arch.hvm.assisted_xapic ) > v->arch.hvm.vmx.secondary_exec_control |= > SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES; > } > diff --git a/xen/arch/x86/include/asm/hvm/domain.h b/xen/arch/x86/include/asm/hvm/domain.h > index 698455444e..92bf53483c 100644 > --- a/xen/arch/x86/include/asm/hvm/domain.h > +++ b/xen/arch/x86/include/asm/hvm/domain.h > @@ -117,6 +117,12 @@ struct hvm_domain { > > bool is_s3_suspended; > > + /* xAPIC hardware assisted virtualization. */ > + bool assisted_xapic; > + > + /* x2APIC hardware assisted virtualization. */ > + bool assisted_x2apic; > + > /* hypervisor intercepted msix table */ > struct list_head msixtbl_list; > > diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c > index 485bd66971..33694acc99 100644 > --- a/xen/arch/x86/traps.c > +++ b/xen/arch/x86/traps.c > @@ -1115,7 +1115,8 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, uint32_t leaf, > if ( !is_hvm_domain(d) || subleaf != 0 ) > break; > > - if ( cpu_has_vmx_apic_reg_virt ) > + if ( cpu_has_vmx_apic_reg_virt && You can drop the cpu_has_vmx_apic_reg_virt check here, if cpu_has_vmx_apic_reg_virt is false assisted_xapic won't be set to true. > + v->domain->arch.hvm.assisted_xapic ) > res->a |= XEN_HVM_CPUID_APIC_ACCESS_VIRT; > > /* > @@ -1124,9 +1125,8 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, uint32_t leaf, > * and wrmsr in the guest will run without VMEXITs (see > * vmx_vlapic_msr_changed()). > */ > - if ( cpu_has_vmx_virtualize_x2apic_mode && > - cpu_has_vmx_apic_reg_virt && > - cpu_has_vmx_virtual_intr_delivery ) > + if ( (cpu_has_vmx_apic_reg_virt && cpu_has_vmx_virtual_intr_delivery) && ^ unneeded parentheses Thanks, Roger.
On 08/02/2022 16:17, Roger Pau Monné wrote: > On Mon, Feb 07, 2022 at 06:21:01PM +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 vitualization, 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 running APIC >> read/write accesses without taking a VM exit. >> >> Being able to disable x{2}APIC hardware assisted vitualization can be >> useful for testing and debugging purposes. > > I think you have agreed with Jan some changes to the description > regarding the purpose of the commit. Yes. > >> >> Signed-off-by: Jane Malalane <jane.malalane@citrix.com> >> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> Will swap the ordering, forgot to do this previously, apologies. >> --- >> CC: Wei Liu <wl@xen.org> >> CC: Anthony PERARD <anthony.perard@citrix.com> >> CC: Juergen Gross <jgross@suse.com> >> CC: Andrew Cooper <andrew.cooper3@citrix.com> >> CC: George Dunlap <george.dunlap@citrix.com> >> CC: Jan Beulich <jbeulich@suse.com> >> CC: Julien Grall <julien@xen.org> >> CC: Stefano Stabellini <sstabellini@kernel.org> >> CC: Christian Lindig <christian.lindig@citrix.com> >> CC: David Scott <dave@recoil.org> >> CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> >> CC: "Roger Pau Monné" <roger.pau@citrix.com> >> >> 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 | 10 ++++++++++ >> docs/man/xl.conf.5.pod.in | 12 ++++++++++++ >> tools/golang/xenlight/helpers.gen.go | 12 ++++++++++++ >> 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 | 23 ++++++++++++++--------- >> tools/libs/light/libxl_types.idl | 2 ++ >> tools/libs/light/libxl_x86.c | 31 +++++++++++++++++++++++++++++-- >> 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 | 14 +++++--------- >> xen/arch/x86/include/asm/hvm/domain.h | 6 ++++++ >> xen/arch/x86/traps.c | 8 ++++---- >> xen/include/public/arch-x86/xen.h | 2 ++ >> 21 files changed, 173 insertions(+), 30 deletions(-) >> >> diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in >> index b98d161398..1d98bbd182 100644 >> --- a/docs/man/xl.cfg.5.pod.in >> +++ b/docs/man/xl.cfg.5.pod.in >> @@ -1862,6 +1862,16 @@ 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. >> +This allows accessing APIC registers without a VM-exit. >> +The default is settable via L<xl.conf(5)>. >> + >> +=item B<assisted_x2APIC=BOOLEAN> >> +B<(x86 only)> Enables or disables hardware assisted virtualization for x2apic. >> +This allows accessing APIC registers without a VM-exit. >> +The default is settable via L<xl.conf(5)>. >> + >> =item B<nx=BOOLEAN> >> >> B<(x86 only)> Hides or exposes the No-eXecute capability. This allows a guest >> diff --git a/docs/man/xl.conf.5.pod.in b/docs/man/xl.conf.5.pod.in >> index df20c08137..30993827e5 100644 >> --- a/docs/man/xl.conf.5.pod.in >> +++ b/docs/man/xl.conf.5.pod.in >> @@ -107,6 +107,18 @@ Sets the default value for the C<max_grant_version> domain config value. >> >> Default: maximum grant version supported by the hypervisor. >> >> +=item B<assisted_xAPIC=BOOLEAN> >> + >> +If enabled, domains will use xAPIC hardware assisted virtualization by default. >> + >> +Default: enabled if supported. >> + >> +=item B<assisted_x2APIC=BOOLEAN> >> + >> +If enabled, domains will use x2APIC hardware assisted virtualization by default. >> + >> +Default: enabled if supported. > > We don't capitalize xl options, so I would suggest to lowercase APIC > for all the option names. Okay. > >> + >> =item B<vif.default.script="PATH"> >> >> Configures the default hotplug script used by virtual network devices. >> diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go >> index dd4e6c9f14..90e7b9b205 100644 >> --- a/tools/golang/xenlight/helpers.gen.go >> +++ b/tools/golang/xenlight/helpers.gen.go >> @@ -636,6 +636,12 @@ x.Passthrough = Passthrough(xc.passthrough) >> if err := x.XendSuspendEvtchnCompat.fromC(&xc.xend_suspend_evtchn_compat);err != nil { >> return fmt.Errorf("converting field XendSuspendEvtchnCompat: %v", err) >> } >> +if err := x.ArchX86.AssistedXapic.fromC(&xc.arch_x86.assisted_xapic);err != nil { >> +return fmt.Errorf("converting field ArchX86.AssistedXapic: %v", err) >> +} >> +if err := x.ArchX86.AssistedX2Apic.fromC(&xc.arch_x86.assisted_x2apic);err != nil { >> +return fmt.Errorf("converting field ArchX86.AssistedX2Apic: %v", err) >> +} >> >> return nil} >> >> @@ -679,6 +685,12 @@ xc.passthrough = C.libxl_passthrough(x.Passthrough) >> if err := x.XendSuspendEvtchnCompat.toC(&xc.xend_suspend_evtchn_compat); err != nil { >> return fmt.Errorf("converting field XendSuspendEvtchnCompat: %v", err) >> } >> +if err := x.ArchX86.AssistedXapic.toC(&xc.arch_x86.assisted_xapic); err != nil { >> +return fmt.Errorf("converting field ArchX86.AssistedXapic: %v", err) >> +} >> +if err := x.ArchX86.AssistedX2Apic.toC(&xc.arch_x86.assisted_x2apic); err != nil { >> +return fmt.Errorf("converting field ArchX86.AssistedX2Apic: %v", err) >> +} >> >> return nil >> } >> diff --git a/tools/include/libxl.h b/tools/include/libxl.h >> index 924e142628..83944c17ae 100644 >> --- a/tools/include/libxl.h >> +++ b/tools/include/libxl.h >> @@ -535,6 +535,13 @@ >> #define LIBXL_HAVE_PHYSINFO_ASSISTED_APIC 1 >> >> /* >> + * LIBXL_HAVE_ASSISTED_APIC indicates that libxl_domain_build_info has >> + * assisted_x{2}apic fields, for enabling hardware assisted virtualization for >> + * x{2}apic per domain. >> + */ >> +#define LIBXL_HAVE_ASSISTED_APIC 1 >> + >> +/* >> * libxl ABI compatibility >> * >> * The only guarantee which libxl makes regarding ABI compatibility >> diff --git a/tools/libs/light/libxl_arch.h b/tools/libs/light/libxl_arch.h >> index 207ceac6a1..03b89929e6 100644 >> --- a/tools/libs/light/libxl_arch.h >> +++ b/tools/libs/light/libxl_arch.h >> @@ -71,8 +71,9 @@ void libxl__arch_domain_create_info_setdefault(libxl__gc *gc, >> libxl_domain_create_info *c_info); >> >> _hidden >> -void libxl__arch_domain_build_info_setdefault(libxl__gc *gc, >> - libxl_domain_build_info *b_info); >> +int libxl__arch_domain_build_info_setdefault(libxl__gc *gc, >> + libxl_domain_build_info *b_info, >> + const libxl_physinfo *physinfo); >> >> _hidden >> int libxl__arch_passthrough_mode_setdefault(libxl__gc *gc, >> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c >> index 39fdca1b49..ba5b8f433f 100644 >> --- a/tools/libs/light/libxl_arm.c >> +++ b/tools/libs/light/libxl_arm.c >> @@ -1384,8 +1384,9 @@ void libxl__arch_domain_create_info_setdefault(libxl__gc *gc, >> } >> } >> >> -void libxl__arch_domain_build_info_setdefault(libxl__gc *gc, >> - libxl_domain_build_info *b_info) >> +int libxl__arch_domain_build_info_setdefault(libxl__gc *gc, >> + libxl_domain_build_info *b_info, >> + const libxl_physinfo *physinfo) >> { >> /* ACPI is disabled by default */ >> libxl_defbool_setdefault(&b_info->acpi, false); >> @@ -1399,6 +1400,8 @@ void libxl__arch_domain_build_info_setdefault(libxl__gc *gc, >> memset(&b_info->u, '\0', sizeof(b_info->u)); >> b_info->type = LIBXL_DOMAIN_TYPE_INVALID; >> libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PVH); >> + >> + return 0; >> } >> >> int libxl__arch_passthrough_mode_setdefault(libxl__gc *gc, >> diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c >> index d7a40d7550..843e523df9 100644 >> --- a/tools/libs/light/libxl_create.c >> +++ b/tools/libs/light/libxl_create.c >> @@ -264,7 +264,20 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, >> if (!b_info->event_channels) >> b_info->event_channels = 1023; >> >> - libxl__arch_domain_build_info_setdefault(gc, b_info); >> + libxl_physinfo info; > > The definition of info needs to be at the top of the function, > together with the rest of the variable definitions. Okay. > >> + >> + rc = libxl_get_physinfo(CTX, &info); >> + if (rc) { >> + LOG(ERROR, "failed to get hypervisor info"); >> + return rc; >> + } >> + >> + rc = libxl__arch_domain_build_info_setdefault(gc, b_info, &info); >> + if (rc) { >> + LOG(ERROR, "unable to set domain arch build info defaults"); >> + return rc; >> + } >> + >> libxl_defbool_setdefault(&b_info->dm_restrict, false); >> >> if (b_info->iommu_memkb == LIBXL_MEMKB_DEFAULT) >> @@ -457,14 +470,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, >> } >> >> if (b_info->max_grant_version == LIBXL_MAX_GRANT_DEFAULT) { >> - libxl_physinfo info; >> - >> - rc = libxl_get_physinfo(CTX, &info); >> - if (rc) { >> - LOG(ERROR, "failed to get hypervisor info"); >> - return rc; >> - } >> - >> if (info.cap_gnttab_v2) >> b_info->max_grant_version = 2; >> else if (info.cap_gnttab_v1) >> diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl >> index 42ac6c357b..db5eb0a0b3 100644 >> --- a/tools/libs/light/libxl_types.idl >> +++ b/tools/libs/light/libxl_types.idl >> @@ -648,6 +648,8 @@ libxl_domain_build_info = Struct("domain_build_info",[ >> ("vuart", libxl_vuart_type), >> ])), >> ("arch_x86", Struct(None, [("msr_relaxed", libxl_defbool), >> + ("assisted_xapic", libxl_defbool), >> + ("assisted_x2apic", libxl_defbool), >> ])), >> # Alternate p2m is not bound to any architecture or guest type, as it is >> # supported by x86 HVM and ARM support is planned. >> diff --git a/tools/libs/light/libxl_x86.c b/tools/libs/light/libxl_x86.c >> index e0a06ecfe3..f0fa0ceea2 100644 >> --- a/tools/libs/light/libxl_x86.c >> +++ b/tools/libs/light/libxl_x86.c >> @@ -23,6 +23,14 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc, >> if (libxl_defbool_val(d_config->b_info.arch_x86.msr_relaxed)) >> config->arch.misc_flags |= XEN_X86_MSR_RELAXED; >> >> + if (d_config->c_info.type != LIBXL_DOMAIN_TYPE_PV) >> + { > > Coding style for libxl is to place the bracket in the same line as the > if. Okay. > >> + if (libxl_defbool_val(d_config->b_info.arch_x86.assisted_xapic)) >> + config->arch.misc_flags |= XEN_X86_ASSISTED_XAPIC; >> + >> + if (libxl_defbool_val(d_config->b_info.arch_x86.assisted_x2apic)) >> + config->arch.misc_flags |= XEN_X86_ASSISTED_X2APIC; >> + } >> return 0; >> } >> >> @@ -819,11 +827,30 @@ void libxl__arch_domain_create_info_setdefault(libxl__gc *gc, >> { >> } >> >> -void libxl__arch_domain_build_info_setdefault(libxl__gc *gc, >> - libxl_domain_build_info *b_info) >> +int libxl__arch_domain_build_info_setdefault(libxl__gc *gc, >> + libxl_domain_build_info *b_info, >> + const libxl_physinfo *physinfo) >> { >> libxl_defbool_setdefault(&b_info->acpi, true); >> libxl_defbool_setdefault(&b_info->arch_x86.msr_relaxed, false); >> + >> + if (b_info->type != LIBXL_DOMAIN_TYPE_PV) >> + { >> + libxl_defbool_setdefault(&b_info->arch_x86.assisted_xapic, >> + physinfo->cap_assisted_xapic); >> + libxl_defbool_setdefault(&b_info->arch_x86.assisted_x2apic, >> + physinfo->cap_assisted_x2apic); >> + } >> + >> + if (b_info->type == LIBXL_DOMAIN_TYPE_PV && >> + (!libxl_defbool_is_default(b_info->arch_x86.assisted_xapic) || >> + !libxl_defbool_is_default(b_info->arch_x86.assisted_x2apic))) > > You could just do: > > if (b_info->type != LIBXL_DOMAIN_TYPE_PV) { > libxl_defbool_setdefault(&b_info->arch_x86.assisted_xapic, > physinfo->cap_assisted_xapic); > libxl_defbool_setdefault(&b_info->arch_x86.assisted_x2apic, > physinfo->cap_assisted_x2apic); > } else if (!libxl_defbool_is_default(b_info->arch_x86.assisted_xapic) || > !libxl_defbool_is_default(b_info->arch_x86.assisted_x2apic)) > /* ERROR */ > >> + { >> + LOG(ERROR, "Interrupt Controller Virtualization not supported for PV"); >> + return ERROR_INVAL; >> + } >> + >> + return 0; True. >> } >> >> int libxl__arch_passthrough_mode_setdefault(libxl__gc *gc, >> diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml >> index 7ce832d605..cce30d8731 100644 >> --- a/tools/ocaml/libs/xc/xenctrl.ml >> +++ b/tools/ocaml/libs/xc/xenctrl.ml >> @@ -50,6 +50,8 @@ type x86_arch_emulation_flags = >> >> type x86_arch_misc_flags = >> | X86_MSR_RELAXED >> + | X86_ASSISTED_XAPIC >> + | X86_ASSISTED_X2APIC >> >> type xen_x86_arch_domainconfig = >> { >> diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli >> index a2b15130ee..67a22ec15c 100644 >> --- a/tools/ocaml/libs/xc/xenctrl.mli >> +++ b/tools/ocaml/libs/xc/xenctrl.mli >> @@ -44,6 +44,8 @@ type x86_arch_emulation_flags = >> >> type x86_arch_misc_flags = >> | X86_MSR_RELAXED >> + | X86_ASSISTED_XAPIC >> + | X86_ASSISTED_X2APIC >> >> type xen_x86_arch_domainconfig = { >> emulation_flags: x86_arch_emulation_flags list; >> diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c >> index 5b4fe72c8d..0aa957d379 100644 >> --- a/tools/ocaml/libs/xc/xenctrl_stubs.c >> +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c >> @@ -239,7 +239,7 @@ CAMLprim value stub_xc_domain_create(value xch, value wanted_domid, value config >> >> cfg.arch.misc_flags = ocaml_list_to_c_bitmap >> /* ! x86_arch_misc_flags X86_ none */ >> - /* ! XEN_X86_ XEN_X86_MSR_RELAXED all */ >> + /* ! XEN_X86_ XEN_X86_ASSISTED_X2APIC max */ > > We would usually define an XEN_X86_MISC_MAX that would point to > XEN_X86_ASSISTED_X2APIC currently. > >> (VAL_MISC_FLAGS); >> >> #undef VAL_MISC_FLAGS >> diff --git a/tools/xl/xl.c b/tools/xl/xl.c >> index 2d1ec18ea3..31eb223309 100644 >> --- a/tools/xl/xl.c >> +++ b/tools/xl/xl.c >> @@ -57,6 +57,8 @@ int max_grant_frames = -1; >> int max_maptrack_frames = -1; >> int max_grant_version = LIBXL_MAX_GRANT_DEFAULT; >> libxl_domid domid_policy = INVALID_DOMID; >> +int assisted_xapic = -1; >> +int assisted_x2apic = -1; >> >> xentoollog_level minmsglevel = minmsglevel_default; >> >> @@ -201,6 +203,12 @@ static void parse_global_config(const char *configfile, >> if (!xlu_cfg_get_long (config, "claim_mode", &l, 0)) >> claim_mode = l; >> >> + if (!xlu_cfg_get_long (config, "assisted_xapic", &l, 0)) >> + assisted_xapic = l; >> + >> + if (!xlu_cfg_get_long (config, "assisted_x2apic", &l, 0)) >> + assisted_x2apic = l; >> + >> xlu_cfg_replace_string (config, "remus.default.netbufscript", >> &default_remus_netbufscript, 0); >> xlu_cfg_replace_string (config, "colo.default.proxyscript", >> diff --git a/tools/xl/xl.h b/tools/xl/xl.h >> index c5c4bedbdd..528deb3feb 100644 >> --- a/tools/xl/xl.h >> +++ b/tools/xl/xl.h >> @@ -286,6 +286,8 @@ extern libxl_bitmap global_vm_affinity_mask; >> extern libxl_bitmap global_hvm_affinity_mask; >> extern libxl_bitmap global_pv_affinity_mask; >> extern libxl_domid domid_policy; >> +extern int assisted_xapic; >> +extern int assisted_x2apic; >> >> enum output_format { >> OUTPUT_FORMAT_JSON, >> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c >> index 117fcdcb2b..0ab9b145fe 100644 >> --- a/tools/xl/xl_parse.c >> +++ b/tools/xl/xl_parse.c >> @@ -1681,6 +1681,22 @@ void parse_config_data(const char *config_source, >> xlu_cfg_get_defbool(config, "vpt_align", &b_info->u.hvm.vpt_align, 0); >> xlu_cfg_get_defbool(config, "apic", &b_info->apic, 0); >> >> + e = xlu_cfg_get_long(config, "assisted_xapic", &l , 0); >> + if ((e == ESRCH && assisted_xapic != -1)) /* use global default if present */ > ^ no need for the extra parentheses here and below. > >> + libxl_defbool_set(&b_info->arch_x86.assisted_xapic, assisted_xapic); >> + else if (!e) >> + libxl_defbool_set(&b_info->arch_x86.assisted_xapic, l); >> + else >> + exit(1); >> + >> + e = xlu_cfg_get_long(config, "assisted_x2apic", &l, 0); >> + if ((e == ESRCH && assisted_x2apic != -1)) /* use global default if present */ >> + libxl_defbool_set(&b_info->arch_x86.assisted_x2apic, assisted_x2apic); >> + else if (!e) >> + libxl_defbool_set(&b_info->arch_x86.assisted_x2apic, l); >> + else >> + exit(1); >> + >> switch (xlu_cfg_get_list(config, "viridian", >> &viridian, &num_viridian, 1)) >> { >> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c >> index ef1812dc14..9033a0e181 100644 >> --- a/xen/arch/x86/domain.c >> +++ b/xen/arch/x86/domain.c >> @@ -619,6 +619,8 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config) >> bool hvm = config->flags & XEN_DOMCTL_CDF_hvm; >> bool hap = config->flags & XEN_DOMCTL_CDF_hap; >> bool nested_virt = config->flags & XEN_DOMCTL_CDF_nested_virt; >> + bool assisted_xapic = config->arch.misc_flags & XEN_X86_ASSISTED_XAPIC; >> + bool assisted_x2apic = config->arch.misc_flags & XEN_X86_ASSISTED_X2APIC; >> unsigned int max_vcpus; >> >> if ( hvm ? !hvm_enabled : !IS_ENABLED(CONFIG_PV) ) >> @@ -685,13 +687,31 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config) >> } >> } >> >> - if ( config->arch.misc_flags & ~XEN_X86_MSR_RELAXED ) >> + if ( config->arch.misc_flags & ~(XEN_X86_MSR_RELAXED | >> + XEN_X86_ASSISTED_XAPIC | >> + XEN_X86_ASSISTED_X2APIC) ) >> { >> dprintk(XENLOG_INFO, "Invalid arch misc flags %#x\n", >> config->arch.misc_flags); >> return -EINVAL; >> } >> >> + if ( (assisted_xapic || assisted_x2apic) && !hvm ) >> + { >> + dprintk(XENLOG_INFO, >> + "Interrupt Controller Virtualization not supported for PV\n"); >> + return -EINVAL; >> + } >> + >> + if ( (assisted_xapic && !assisted_xapic_available) || >> + (assisted_x2apic && !assisted_x2apic_available) ) >> + { >> + dprintk(XENLOG_INFO, >> + "Hardware assisted x%sAPIC requested but not available\n", >> + assisted_xapic && !assisted_xapic_available ? "" : "2"); >> + return -EINVAL; >> + } >> + >> return 0; >> } >> >> @@ -863,6 +883,12 @@ int arch_domain_create(struct domain *d, >> >> d->arch.msr_relaxed = config->arch.misc_flags & XEN_X86_MSR_RELAXED; >> >> + d->arch.hvm.assisted_xapic = >> + config->arch.misc_flags & XEN_X86_ASSISTED_XAPIC; >> + >> + d->arch.hvm.assisted_x2apic = >> + config->arch.misc_flags & XEN_X86_ASSISTED_X2APIC; >> + >> return 0; >> >> fail: >> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c >> index 4060aef1bd..614db5c4a4 100644 >> --- a/xen/arch/x86/hvm/vmx/vmcs.c >> +++ b/xen/arch/x86/hvm/vmx/vmcs.c >> @@ -1157,6 +1157,10 @@ static int construct_vmcs(struct vcpu *v) >> __vmwrite(PLE_WINDOW, ple_window); >> } >> >> + if ( !v->domain->arch.hvm.assisted_xapic ) >> + v->arch.hvm.vmx.secondary_exec_control &= >> + ~SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES; >> + >> if ( cpu_has_vmx_secondary_exec_control ) >> __vmwrite(SECONDARY_VM_EXEC_CONTROL, >> v->arch.hvm.vmx.secondary_exec_control); >> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c >> index 36c8a12cfe..3c9ff60154 100644 >> --- a/xen/arch/x86/hvm/vmx/vmx.c >> +++ b/xen/arch/x86/hvm/vmx/vmx.c >> @@ -3333,16 +3333,11 @@ static void vmx_install_vlapic_mapping(struct vcpu *v) >> >> void vmx_vlapic_msr_changed(struct vcpu *v) >> { >> - int virtualize_x2apic_mode; >> struct vlapic *vlapic = vcpu_vlapic(v); >> unsigned int msr; >> >> - virtualize_x2apic_mode = ( (cpu_has_vmx_apic_reg_virt || >> - cpu_has_vmx_virtual_intr_delivery) && >> - cpu_has_vmx_virtualize_x2apic_mode ); >> - >> - if ( !cpu_has_vmx_virtualize_apic_accesses && >> - !virtualize_x2apic_mode ) >> + if ( ! v->domain->arch.hvm.assisted_xapic && >> + ! v->domain->arch.hvm.assisted_x2apic ) > ^ extra space. >> return; >> >> vmx_vmcs_enter(v); >> @@ -3352,7 +3347,8 @@ void vmx_vlapic_msr_changed(struct vcpu *v) >> if ( !vlapic_hw_disabled(vlapic) && >> (vlapic_base_address(vlapic) == APIC_DEFAULT_PHYS_BASE) ) >> { >> - if ( virtualize_x2apic_mode && vlapic_x2apic_mode(vlapic) ) >> + if ( v->domain->arch.hvm.assisted_x2apic >> + && vlapic_x2apic_mode(vlapic) ) >> { >> v->arch.hvm.vmx.secondary_exec_control |= >> SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE; >> @@ -3373,7 +3369,7 @@ void vmx_vlapic_msr_changed(struct vcpu *v) >> vmx_clear_msr_intercept(v, MSR_X2APIC_SELF, VMX_MSR_W); >> } >> } >> - else >> + else if ( v->domain->arch.hvm.assisted_xapic ) >> v->arch.hvm.vmx.secondary_exec_control |= >> SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES; >> } >> diff --git a/xen/arch/x86/include/asm/hvm/domain.h b/xen/arch/x86/include/asm/hvm/domain.h >> index 698455444e..92bf53483c 100644 >> --- a/xen/arch/x86/include/asm/hvm/domain.h >> +++ b/xen/arch/x86/include/asm/hvm/domain.h >> @@ -117,6 +117,12 @@ struct hvm_domain { >> >> bool is_s3_suspended; >> >> + /* xAPIC hardware assisted virtualization. */ >> + bool assisted_xapic; >> + >> + /* x2APIC hardware assisted virtualization. */ >> + bool assisted_x2apic; >> + >> /* hypervisor intercepted msix table */ >> struct list_head msixtbl_list; >> >> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c >> index 485bd66971..33694acc99 100644 >> --- a/xen/arch/x86/traps.c >> +++ b/xen/arch/x86/traps.c >> @@ -1115,7 +1115,8 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, uint32_t leaf, >> if ( !is_hvm_domain(d) || subleaf != 0 ) >> break; >> >> - if ( cpu_has_vmx_apic_reg_virt ) >> + if ( cpu_has_vmx_apic_reg_virt && > > You can drop the cpu_has_vmx_apic_reg_virt check here, if > cpu_has_vmx_apic_reg_virt is false assisted_xapic won't be set to true. Oh, but assisted_xapic_available is only set to depend on cpu_has_vmx_virtualize_apic_accesses, unless I should correct this, but I might be missing something... > >> + v->domain->arch.hvm.assisted_xapic ) >> res->a |= XEN_HVM_CPUID_APIC_ACCESS_VIRT; >> >> /* >> @@ -1124,9 +1125,8 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, uint32_t leaf, >> * and wrmsr in the guest will run without VMEXITs (see >> * vmx_vlapic_msr_changed()). >> */ >> - if ( cpu_has_vmx_virtualize_x2apic_mode && >> - cpu_has_vmx_apic_reg_virt && >> - cpu_has_vmx_virtual_intr_delivery ) >> + if ( (cpu_has_vmx_apic_reg_virt && cpu_has_vmx_virtual_intr_delivery) && > ^ unneeded parentheses > Okay. > Thanks, Roger. Thank you, Jane.
On Wed, Feb 09, 2022 at 10:57:28AM +0000, Jane Malalane wrote: > On 08/02/2022 16:17, Roger Pau Monné wrote: > > On Mon, Feb 07, 2022 at 06:21:01PM +0000, Jane Malalane wrote: > >> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c > >> index 485bd66971..33694acc99 100644 > >> --- a/xen/arch/x86/traps.c > >> +++ b/xen/arch/x86/traps.c > >> @@ -1115,7 +1115,8 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, uint32_t leaf, > >> if ( !is_hvm_domain(d) || subleaf != 0 ) > >> break; > >> > >> - if ( cpu_has_vmx_apic_reg_virt ) > >> + if ( cpu_has_vmx_apic_reg_virt && > > > > You can drop the cpu_has_vmx_apic_reg_virt check here, if > > cpu_has_vmx_apic_reg_virt is false assisted_xapic won't be set to true. > Oh, but assisted_xapic_available is only set to depend on > cpu_has_vmx_virtualize_apic_accesses, unless I should correct this, but > I might be missing something... No, you are right. We only report hw virtualized xAPIC to guests if both apic_reg_virt and virtualize_apic_accesses are available. Thanks, Roger.
On Mon, Feb 07, 2022 at 06:21:01PM +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 vitualization, 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 running APIC > read/write accesses without taking a VM exit. > > Being able to disable x{2}APIC hardware assisted vitualization can be > useful for testing and debugging purposes. Might be worth adding a note to the commit log in order to note that vmx_install_vlapic_mapping doesn't require modifications regardless of whether the guest has virtualize_apic_accesses enabled or not. Setting the APIC_ACCESS_ADDR VMCS field is fine even if virtualize_apic_accesses is not enabled for the guest: as long as the feature is supported by the CPU the field will exist. Thanks, Roger.
On 10/02/2022 10:09, Roger Pau Monné wrote: > On Mon, Feb 07, 2022 at 06:21:01PM +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 vitualization, 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 running APIC >> read/write accesses without taking a VM exit. >> >> Being able to disable x{2}APIC hardware assisted vitualization can be >> useful for testing and debugging purposes. > > Might be worth adding a note to the commit log in order to note that > vmx_install_vlapic_mapping doesn't require modifications regardless of > whether the guest has virtualize_apic_accesses enabled or not. > > Setting the APIC_ACCESS_ADDR VMCS field is fine even if > virtualize_apic_accesses is not enabled for the guest: as long as the > feature is supported by the CPU the field will exist. Oh right. Will add these two points. Thanks, Jane.
On 08.02.2022 17:17, Roger Pau Monné wrote: > On Mon, Feb 07, 2022 at 06:21:01PM +0000, Jane Malalane wrote: >> --- a/xen/arch/x86/traps.c >> +++ b/xen/arch/x86/traps.c >> @@ -1115,7 +1115,8 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, uint32_t leaf, >> if ( !is_hvm_domain(d) || subleaf != 0 ) >> break; >> >> - if ( cpu_has_vmx_apic_reg_virt ) >> + if ( cpu_has_vmx_apic_reg_virt && > > You can drop the cpu_has_vmx_apic_reg_virt check here, if > cpu_has_vmx_apic_reg_virt is false assisted_xapic won't be set to true. Along these lines ... >> + v->domain->arch.hvm.assisted_xapic ) >> res->a |= XEN_HVM_CPUID_APIC_ACCESS_VIRT; >> >> /* >> @@ -1124,9 +1125,8 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, uint32_t leaf, >> * and wrmsr in the guest will run without VMEXITs (see >> * vmx_vlapic_msr_changed()). >> */ >> - if ( cpu_has_vmx_virtualize_x2apic_mode && >> - cpu_has_vmx_apic_reg_virt && >> - cpu_has_vmx_virtual_intr_delivery ) >> + if ( (cpu_has_vmx_apic_reg_virt && cpu_has_vmx_virtual_intr_delivery) && > ^ unneeded parentheses ... this also wants simplifying to just v->domain->arch.hvm.assisted_x2apic: The apparently stray parentheses were, I think, added in reply to me pointing out that the check here isn't in line with that put in place by patch 1 in vmx_init_vmcs_config(). I.e. the inner && really was meant to be || as it looks. Yet once the two are in line, the same simplification as above is possible. Jan
diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in index b98d161398..1d98bbd182 100644 --- a/docs/man/xl.cfg.5.pod.in +++ b/docs/man/xl.cfg.5.pod.in @@ -1862,6 +1862,16 @@ 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. +This allows accessing APIC registers without a VM-exit. +The default is settable via L<xl.conf(5)>. + +=item B<assisted_x2APIC=BOOLEAN> +B<(x86 only)> Enables or disables hardware assisted virtualization for x2apic. +This allows accessing APIC registers without a VM-exit. +The default is settable via L<xl.conf(5)>. + =item B<nx=BOOLEAN> B<(x86 only)> Hides or exposes the No-eXecute capability. This allows a guest diff --git a/docs/man/xl.conf.5.pod.in b/docs/man/xl.conf.5.pod.in index df20c08137..30993827e5 100644 --- a/docs/man/xl.conf.5.pod.in +++ b/docs/man/xl.conf.5.pod.in @@ -107,6 +107,18 @@ Sets the default value for the C<max_grant_version> domain config value. Default: maximum grant version supported by the hypervisor. +=item B<assisted_xAPIC=BOOLEAN> + +If enabled, domains will use xAPIC hardware assisted virtualization by default. + +Default: enabled if supported. + +=item B<assisted_x2APIC=BOOLEAN> + +If enabled, domains will use x2APIC hardware assisted virtualization by default. + +Default: enabled if supported. + =item B<vif.default.script="PATH"> Configures the default hotplug script used by virtual network devices. diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go index dd4e6c9f14..90e7b9b205 100644 --- a/tools/golang/xenlight/helpers.gen.go +++ b/tools/golang/xenlight/helpers.gen.go @@ -636,6 +636,12 @@ x.Passthrough = Passthrough(xc.passthrough) if err := x.XendSuspendEvtchnCompat.fromC(&xc.xend_suspend_evtchn_compat);err != nil { return fmt.Errorf("converting field XendSuspendEvtchnCompat: %v", err) } +if err := x.ArchX86.AssistedXapic.fromC(&xc.arch_x86.assisted_xapic);err != nil { +return fmt.Errorf("converting field ArchX86.AssistedXapic: %v", err) +} +if err := x.ArchX86.AssistedX2Apic.fromC(&xc.arch_x86.assisted_x2apic);err != nil { +return fmt.Errorf("converting field ArchX86.AssistedX2Apic: %v", err) +} return nil} @@ -679,6 +685,12 @@ xc.passthrough = C.libxl_passthrough(x.Passthrough) if err := x.XendSuspendEvtchnCompat.toC(&xc.xend_suspend_evtchn_compat); err != nil { return fmt.Errorf("converting field XendSuspendEvtchnCompat: %v", err) } +if err := x.ArchX86.AssistedXapic.toC(&xc.arch_x86.assisted_xapic); err != nil { +return fmt.Errorf("converting field ArchX86.AssistedXapic: %v", err) +} +if err := x.ArchX86.AssistedX2Apic.toC(&xc.arch_x86.assisted_x2apic); err != nil { +return fmt.Errorf("converting field ArchX86.AssistedX2Apic: %v", err) +} return nil } diff --git a/tools/include/libxl.h b/tools/include/libxl.h index 924e142628..83944c17ae 100644 --- a/tools/include/libxl.h +++ b/tools/include/libxl.h @@ -535,6 +535,13 @@ #define LIBXL_HAVE_PHYSINFO_ASSISTED_APIC 1 /* + * LIBXL_HAVE_ASSISTED_APIC indicates that libxl_domain_build_info has + * assisted_x{2}apic fields, for enabling hardware assisted virtualization for + * x{2}apic per domain. + */ +#define LIBXL_HAVE_ASSISTED_APIC 1 + +/* * libxl ABI compatibility * * The only guarantee which libxl makes regarding ABI compatibility diff --git a/tools/libs/light/libxl_arch.h b/tools/libs/light/libxl_arch.h index 207ceac6a1..03b89929e6 100644 --- a/tools/libs/light/libxl_arch.h +++ b/tools/libs/light/libxl_arch.h @@ -71,8 +71,9 @@ void libxl__arch_domain_create_info_setdefault(libxl__gc *gc, libxl_domain_create_info *c_info); _hidden -void libxl__arch_domain_build_info_setdefault(libxl__gc *gc, - libxl_domain_build_info *b_info); +int libxl__arch_domain_build_info_setdefault(libxl__gc *gc, + libxl_domain_build_info *b_info, + const libxl_physinfo *physinfo); _hidden int libxl__arch_passthrough_mode_setdefault(libxl__gc *gc, diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c index 39fdca1b49..ba5b8f433f 100644 --- a/tools/libs/light/libxl_arm.c +++ b/tools/libs/light/libxl_arm.c @@ -1384,8 +1384,9 @@ void libxl__arch_domain_create_info_setdefault(libxl__gc *gc, } } -void libxl__arch_domain_build_info_setdefault(libxl__gc *gc, - libxl_domain_build_info *b_info) +int libxl__arch_domain_build_info_setdefault(libxl__gc *gc, + libxl_domain_build_info *b_info, + const libxl_physinfo *physinfo) { /* ACPI is disabled by default */ libxl_defbool_setdefault(&b_info->acpi, false); @@ -1399,6 +1400,8 @@ void libxl__arch_domain_build_info_setdefault(libxl__gc *gc, memset(&b_info->u, '\0', sizeof(b_info->u)); b_info->type = LIBXL_DOMAIN_TYPE_INVALID; libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PVH); + + return 0; } int libxl__arch_passthrough_mode_setdefault(libxl__gc *gc, diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c index d7a40d7550..843e523df9 100644 --- a/tools/libs/light/libxl_create.c +++ b/tools/libs/light/libxl_create.c @@ -264,7 +264,20 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, if (!b_info->event_channels) b_info->event_channels = 1023; - libxl__arch_domain_build_info_setdefault(gc, b_info); + libxl_physinfo info; + + rc = libxl_get_physinfo(CTX, &info); + if (rc) { + LOG(ERROR, "failed to get hypervisor info"); + return rc; + } + + rc = libxl__arch_domain_build_info_setdefault(gc, b_info, &info); + if (rc) { + LOG(ERROR, "unable to set domain arch build info defaults"); + return rc; + } + libxl_defbool_setdefault(&b_info->dm_restrict, false); if (b_info->iommu_memkb == LIBXL_MEMKB_DEFAULT) @@ -457,14 +470,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, } if (b_info->max_grant_version == LIBXL_MAX_GRANT_DEFAULT) { - libxl_physinfo info; - - rc = libxl_get_physinfo(CTX, &info); - if (rc) { - LOG(ERROR, "failed to get hypervisor info"); - return rc; - } - if (info.cap_gnttab_v2) b_info->max_grant_version = 2; else if (info.cap_gnttab_v1) diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl index 42ac6c357b..db5eb0a0b3 100644 --- a/tools/libs/light/libxl_types.idl +++ b/tools/libs/light/libxl_types.idl @@ -648,6 +648,8 @@ libxl_domain_build_info = Struct("domain_build_info",[ ("vuart", libxl_vuart_type), ])), ("arch_x86", Struct(None, [("msr_relaxed", libxl_defbool), + ("assisted_xapic", libxl_defbool), + ("assisted_x2apic", libxl_defbool), ])), # Alternate p2m is not bound to any architecture or guest type, as it is # supported by x86 HVM and ARM support is planned. diff --git a/tools/libs/light/libxl_x86.c b/tools/libs/light/libxl_x86.c index e0a06ecfe3..f0fa0ceea2 100644 --- a/tools/libs/light/libxl_x86.c +++ b/tools/libs/light/libxl_x86.c @@ -23,6 +23,14 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc, if (libxl_defbool_val(d_config->b_info.arch_x86.msr_relaxed)) config->arch.misc_flags |= XEN_X86_MSR_RELAXED; + if (d_config->c_info.type != LIBXL_DOMAIN_TYPE_PV) + { + if (libxl_defbool_val(d_config->b_info.arch_x86.assisted_xapic)) + config->arch.misc_flags |= XEN_X86_ASSISTED_XAPIC; + + if (libxl_defbool_val(d_config->b_info.arch_x86.assisted_x2apic)) + config->arch.misc_flags |= XEN_X86_ASSISTED_X2APIC; + } return 0; } @@ -819,11 +827,30 @@ void libxl__arch_domain_create_info_setdefault(libxl__gc *gc, { } -void libxl__arch_domain_build_info_setdefault(libxl__gc *gc, - libxl_domain_build_info *b_info) +int libxl__arch_domain_build_info_setdefault(libxl__gc *gc, + libxl_domain_build_info *b_info, + const libxl_physinfo *physinfo) { libxl_defbool_setdefault(&b_info->acpi, true); libxl_defbool_setdefault(&b_info->arch_x86.msr_relaxed, false); + + if (b_info->type != LIBXL_DOMAIN_TYPE_PV) + { + libxl_defbool_setdefault(&b_info->arch_x86.assisted_xapic, + physinfo->cap_assisted_xapic); + libxl_defbool_setdefault(&b_info->arch_x86.assisted_x2apic, + physinfo->cap_assisted_x2apic); + } + + if (b_info->type == LIBXL_DOMAIN_TYPE_PV && + (!libxl_defbool_is_default(b_info->arch_x86.assisted_xapic) || + !libxl_defbool_is_default(b_info->arch_x86.assisted_x2apic))) + { + LOG(ERROR, "Interrupt Controller Virtualization not supported for PV"); + return ERROR_INVAL; + } + + return 0; } int libxl__arch_passthrough_mode_setdefault(libxl__gc *gc, diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml index 7ce832d605..cce30d8731 100644 --- a/tools/ocaml/libs/xc/xenctrl.ml +++ b/tools/ocaml/libs/xc/xenctrl.ml @@ -50,6 +50,8 @@ type x86_arch_emulation_flags = type x86_arch_misc_flags = | X86_MSR_RELAXED + | X86_ASSISTED_XAPIC + | X86_ASSISTED_X2APIC type xen_x86_arch_domainconfig = { diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli index a2b15130ee..67a22ec15c 100644 --- a/tools/ocaml/libs/xc/xenctrl.mli +++ b/tools/ocaml/libs/xc/xenctrl.mli @@ -44,6 +44,8 @@ type x86_arch_emulation_flags = type x86_arch_misc_flags = | X86_MSR_RELAXED + | X86_ASSISTED_XAPIC + | X86_ASSISTED_X2APIC type xen_x86_arch_domainconfig = { emulation_flags: x86_arch_emulation_flags list; diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c index 5b4fe72c8d..0aa957d379 100644 --- a/tools/ocaml/libs/xc/xenctrl_stubs.c +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c @@ -239,7 +239,7 @@ CAMLprim value stub_xc_domain_create(value xch, value wanted_domid, value config cfg.arch.misc_flags = ocaml_list_to_c_bitmap /* ! x86_arch_misc_flags X86_ none */ - /* ! XEN_X86_ XEN_X86_MSR_RELAXED all */ + /* ! XEN_X86_ XEN_X86_ASSISTED_X2APIC max */ (VAL_MISC_FLAGS); #undef VAL_MISC_FLAGS diff --git a/tools/xl/xl.c b/tools/xl/xl.c index 2d1ec18ea3..31eb223309 100644 --- a/tools/xl/xl.c +++ b/tools/xl/xl.c @@ -57,6 +57,8 @@ int max_grant_frames = -1; int max_maptrack_frames = -1; int max_grant_version = LIBXL_MAX_GRANT_DEFAULT; libxl_domid domid_policy = INVALID_DOMID; +int assisted_xapic = -1; +int assisted_x2apic = -1; xentoollog_level minmsglevel = minmsglevel_default; @@ -201,6 +203,12 @@ static void parse_global_config(const char *configfile, if (!xlu_cfg_get_long (config, "claim_mode", &l, 0)) claim_mode = l; + if (!xlu_cfg_get_long (config, "assisted_xapic", &l, 0)) + assisted_xapic = l; + + if (!xlu_cfg_get_long (config, "assisted_x2apic", &l, 0)) + assisted_x2apic = l; + xlu_cfg_replace_string (config, "remus.default.netbufscript", &default_remus_netbufscript, 0); xlu_cfg_replace_string (config, "colo.default.proxyscript", diff --git a/tools/xl/xl.h b/tools/xl/xl.h index c5c4bedbdd..528deb3feb 100644 --- a/tools/xl/xl.h +++ b/tools/xl/xl.h @@ -286,6 +286,8 @@ extern libxl_bitmap global_vm_affinity_mask; extern libxl_bitmap global_hvm_affinity_mask; extern libxl_bitmap global_pv_affinity_mask; extern libxl_domid domid_policy; +extern int assisted_xapic; +extern int assisted_x2apic; enum output_format { OUTPUT_FORMAT_JSON, diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c index 117fcdcb2b..0ab9b145fe 100644 --- a/tools/xl/xl_parse.c +++ b/tools/xl/xl_parse.c @@ -1681,6 +1681,22 @@ void parse_config_data(const char *config_source, xlu_cfg_get_defbool(config, "vpt_align", &b_info->u.hvm.vpt_align, 0); xlu_cfg_get_defbool(config, "apic", &b_info->apic, 0); + e = xlu_cfg_get_long(config, "assisted_xapic", &l , 0); + if ((e == ESRCH && assisted_xapic != -1)) /* use global default if present */ + libxl_defbool_set(&b_info->arch_x86.assisted_xapic, assisted_xapic); + else if (!e) + libxl_defbool_set(&b_info->arch_x86.assisted_xapic, l); + else + exit(1); + + e = xlu_cfg_get_long(config, "assisted_x2apic", &l, 0); + if ((e == ESRCH && assisted_x2apic != -1)) /* use global default if present */ + libxl_defbool_set(&b_info->arch_x86.assisted_x2apic, assisted_x2apic); + else if (!e) + libxl_defbool_set(&b_info->arch_x86.assisted_x2apic, l); + else + exit(1); + switch (xlu_cfg_get_list(config, "viridian", &viridian, &num_viridian, 1)) { diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index ef1812dc14..9033a0e181 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -619,6 +619,8 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config) bool hvm = config->flags & XEN_DOMCTL_CDF_hvm; bool hap = config->flags & XEN_DOMCTL_CDF_hap; bool nested_virt = config->flags & XEN_DOMCTL_CDF_nested_virt; + bool assisted_xapic = config->arch.misc_flags & XEN_X86_ASSISTED_XAPIC; + bool assisted_x2apic = config->arch.misc_flags & XEN_X86_ASSISTED_X2APIC; unsigned int max_vcpus; if ( hvm ? !hvm_enabled : !IS_ENABLED(CONFIG_PV) ) @@ -685,13 +687,31 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config) } } - if ( config->arch.misc_flags & ~XEN_X86_MSR_RELAXED ) + if ( config->arch.misc_flags & ~(XEN_X86_MSR_RELAXED | + XEN_X86_ASSISTED_XAPIC | + XEN_X86_ASSISTED_X2APIC) ) { dprintk(XENLOG_INFO, "Invalid arch misc flags %#x\n", config->arch.misc_flags); return -EINVAL; } + if ( (assisted_xapic || assisted_x2apic) && !hvm ) + { + dprintk(XENLOG_INFO, + "Interrupt Controller Virtualization not supported for PV\n"); + return -EINVAL; + } + + if ( (assisted_xapic && !assisted_xapic_available) || + (assisted_x2apic && !assisted_x2apic_available) ) + { + dprintk(XENLOG_INFO, + "Hardware assisted x%sAPIC requested but not available\n", + assisted_xapic && !assisted_xapic_available ? "" : "2"); + return -EINVAL; + } + return 0; } @@ -863,6 +883,12 @@ int arch_domain_create(struct domain *d, d->arch.msr_relaxed = config->arch.misc_flags & XEN_X86_MSR_RELAXED; + d->arch.hvm.assisted_xapic = + config->arch.misc_flags & XEN_X86_ASSISTED_XAPIC; + + d->arch.hvm.assisted_x2apic = + config->arch.misc_flags & XEN_X86_ASSISTED_X2APIC; + return 0; fail: diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c index 4060aef1bd..614db5c4a4 100644 --- a/xen/arch/x86/hvm/vmx/vmcs.c +++ b/xen/arch/x86/hvm/vmx/vmcs.c @@ -1157,6 +1157,10 @@ static int construct_vmcs(struct vcpu *v) __vmwrite(PLE_WINDOW, ple_window); } + if ( !v->domain->arch.hvm.assisted_xapic ) + v->arch.hvm.vmx.secondary_exec_control &= + ~SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES; + if ( cpu_has_vmx_secondary_exec_control ) __vmwrite(SECONDARY_VM_EXEC_CONTROL, v->arch.hvm.vmx.secondary_exec_control); diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index 36c8a12cfe..3c9ff60154 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -3333,16 +3333,11 @@ static void vmx_install_vlapic_mapping(struct vcpu *v) void vmx_vlapic_msr_changed(struct vcpu *v) { - int virtualize_x2apic_mode; struct vlapic *vlapic = vcpu_vlapic(v); unsigned int msr; - virtualize_x2apic_mode = ( (cpu_has_vmx_apic_reg_virt || - cpu_has_vmx_virtual_intr_delivery) && - cpu_has_vmx_virtualize_x2apic_mode ); - - if ( !cpu_has_vmx_virtualize_apic_accesses && - !virtualize_x2apic_mode ) + if ( ! v->domain->arch.hvm.assisted_xapic && + ! v->domain->arch.hvm.assisted_x2apic ) return; vmx_vmcs_enter(v); @@ -3352,7 +3347,8 @@ void vmx_vlapic_msr_changed(struct vcpu *v) if ( !vlapic_hw_disabled(vlapic) && (vlapic_base_address(vlapic) == APIC_DEFAULT_PHYS_BASE) ) { - if ( virtualize_x2apic_mode && vlapic_x2apic_mode(vlapic) ) + if ( v->domain->arch.hvm.assisted_x2apic + && vlapic_x2apic_mode(vlapic) ) { v->arch.hvm.vmx.secondary_exec_control |= SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE; @@ -3373,7 +3369,7 @@ void vmx_vlapic_msr_changed(struct vcpu *v) vmx_clear_msr_intercept(v, MSR_X2APIC_SELF, VMX_MSR_W); } } - else + else if ( v->domain->arch.hvm.assisted_xapic ) v->arch.hvm.vmx.secondary_exec_control |= SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES; } diff --git a/xen/arch/x86/include/asm/hvm/domain.h b/xen/arch/x86/include/asm/hvm/domain.h index 698455444e..92bf53483c 100644 --- a/xen/arch/x86/include/asm/hvm/domain.h +++ b/xen/arch/x86/include/asm/hvm/domain.h @@ -117,6 +117,12 @@ struct hvm_domain { bool is_s3_suspended; + /* xAPIC hardware assisted virtualization. */ + bool assisted_xapic; + + /* x2APIC hardware assisted virtualization. */ + bool assisted_x2apic; + /* hypervisor intercepted msix table */ struct list_head msixtbl_list; diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c index 485bd66971..33694acc99 100644 --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -1115,7 +1115,8 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, uint32_t leaf, if ( !is_hvm_domain(d) || subleaf != 0 ) break; - if ( cpu_has_vmx_apic_reg_virt ) + if ( cpu_has_vmx_apic_reg_virt && + v->domain->arch.hvm.assisted_xapic ) res->a |= XEN_HVM_CPUID_APIC_ACCESS_VIRT; /* @@ -1124,9 +1125,8 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, uint32_t leaf, * and wrmsr in the guest will run without VMEXITs (see * vmx_vlapic_msr_changed()). */ - if ( cpu_has_vmx_virtualize_x2apic_mode && - cpu_has_vmx_apic_reg_virt && - cpu_has_vmx_virtual_intr_delivery ) + if ( (cpu_has_vmx_apic_reg_virt && cpu_has_vmx_virtual_intr_delivery) && + v->domain->arch.hvm.assisted_x2apic ) res->a |= XEN_HVM_CPUID_X2APIC_VIRT; /* diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h index 7acd94c8eb..9da32c6239 100644 --- a/xen/include/public/arch-x86/xen.h +++ b/xen/include/public/arch-x86/xen.h @@ -317,6 +317,8 @@ struct xen_arch_domainconfig { * doesn't allow the guest to read or write to the underlying MSR. */ #define XEN_X86_MSR_RELAXED (1u << 0) +#define XEN_X86_ASSISTED_XAPIC (1u << 1) +#define XEN_X86_ASSISTED_X2APIC (1u << 2) uint32_t misc_flags; };
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 vitualization, 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 running APIC read/write accesses without taking a VM exit. Being able to disable x{2}APIC hardware assisted vitualization can be useful for testing and debugging purposes. Signed-off-by: Jane Malalane <jane.malalane@citrix.com> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Wei Liu <wl@xen.org> CC: Anthony PERARD <anthony.perard@citrix.com> CC: Juergen Gross <jgross@suse.com> CC: Andrew Cooper <andrew.cooper3@citrix.com> CC: George Dunlap <george.dunlap@citrix.com> CC: Jan Beulich <jbeulich@suse.com> CC: Julien Grall <julien@xen.org> CC: Stefano Stabellini <sstabellini@kernel.org> CC: Christian Lindig <christian.lindig@citrix.com> CC: David Scott <dave@recoil.org> CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> CC: "Roger Pau Monné" <roger.pau@citrix.com> 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 | 10 ++++++++++ docs/man/xl.conf.5.pod.in | 12 ++++++++++++ tools/golang/xenlight/helpers.gen.go | 12 ++++++++++++ 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 | 23 ++++++++++++++--------- tools/libs/light/libxl_types.idl | 2 ++ tools/libs/light/libxl_x86.c | 31 +++++++++++++++++++++++++++++-- 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 | 14 +++++--------- xen/arch/x86/include/asm/hvm/domain.h | 6 ++++++ xen/arch/x86/traps.c | 8 ++++---- xen/include/public/arch-x86/xen.h | 2 ++ 21 files changed, 173 insertions(+), 30 deletions(-)