Skip to content

Commit

Permalink
x86/hvm: Revert per-domain APIC acceleration support
Browse files Browse the repository at this point in the history
I was really hoping to avoid this, but its now too late in the 4.17 freeze and
we still don't have working fixes.

The in-Xen calculations for assistance capabilities are buggy.  For the
avoidance of doubt, the original intention was to be able to control every
aspect of a APIC acceleration so we could comprehensively test Xen's support,
as it has proved to be buggy time and time again.

Even after a protracted discussion on what the new API ought to mean, attempts
to apply it to the existing logic have been unsuccessful, proving that the
API/ABI is too complicated for most people to reason about.

This reverts most of:
  2ce11ce
  6b2b9b3

leaving in place the non-APIC specific changes (minimal as they are).

This takes us back to the behaviour of Xen 4.16 where APIC acceleration is
configured on a per system basis.

This work will be revisted in due course.

Fixes: 2ce11ce ("x86/HVM: allow per-domain usage of hardware virtualized APIC")
Fixes: 6b2b9b3 ("x86: report Interrupt Controller Virtualization capabilities")
Signed-off-by: Andrew Cooper <[email protected]>
Acked-by: Jan Beulich <[email protected]>
Release-acked-by: Henry Wang <[email protected]>
  • Loading branch information
andyhhp committed Nov 17, 2022
1 parent f5d56f4 commit e5ac68a
Show file tree
Hide file tree
Showing 27 changed files with 19 additions and 236 deletions.
15 changes: 0 additions & 15 deletions docs/man/xl.cfg.5.pod.in
Original file line number Diff line number Diff line change
Expand Up @@ -1862,21 +1862,6 @@ 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
a p2m fault, or altogether avoid 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. With this option enabled, certain accesses to MSR APIC
registers will avoid a VM exit into the hypervisor. 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
Expand Down
12 changes: 0 additions & 12 deletions docs/man/xl.conf.5.pod.in
Original file line number Diff line number Diff line change
Expand Up @@ -107,18 +107,6 @@ 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.
Expand Down
16 changes: 0 additions & 16 deletions tools/golang/xenlight/helpers.gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 0 additions & 4 deletions tools/golang/xenlight/types.gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 0 additions & 14 deletions tools/include/libxl.h
Original file line number Diff line number Diff line change
Expand Up @@ -535,20 +535,6 @@
#define LIBXL_HAVE_DISK_TRUSTED 1
#define LIBXL_HAVE_NIC_TRUSTED 1

/*
* LIBXL_HAVE_PHYSINFO_ASSISTED_APIC indicates that libxl_physinfo has
* cap_assisted_xapic and cap_assisted_x2apic fields, which indicates
* the availability of x{2}APIC hardware assisted virtualization.
*/
#define LIBXL_HAVE_PHYSINFO_ASSISTED_APIC 1

/*
* LIBXL_HAVE_ASSISTED_APIC indicates that libxl_domain_build_info has
* assisted_xapic and assisted_x2apic fields for enabling hardware
* assisted virtualization for x{2}apic per domain.
*/
#define LIBXL_HAVE_ASSISTED_APIC 1

/*
* LIBXL_HAVE_DEVICE_DISK_SPECIFICATION indicates that 'specification' and
* 'transport' fields (of libxl_disk_specification and libxl_disk_transport
Expand Down
3 changes: 0 additions & 3 deletions tools/libs/light/libxl.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#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)
Expand Down Expand Up @@ -411,8 +410,6 @@ 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;
}
Expand Down
4 changes: 0 additions & 4 deletions tools/libs/light/libxl_arch.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,6 @@ int libxl__arch_extra_memory(libxl__gc *gc,
const libxl_domain_build_info *info,
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,
Expand Down
5 changes: 0 additions & 5 deletions tools/libs/light/libxl_arm.c
Original file line number Diff line number Diff line change
Expand Up @@ -1606,11 +1606,6 @@ 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)
Expand Down
4 changes: 0 additions & 4 deletions tools/libs/light/libxl_types.idl
Original file line number Diff line number Diff line change
Expand Up @@ -660,8 +660,6 @@ 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.
Expand Down Expand Up @@ -1090,8 +1088,6 @@ 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", [
Expand Down
32 changes: 0 additions & 32 deletions tools/libs/light/libxl_x86.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,6 @@ 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;
}

Expand Down Expand Up @@ -835,18 +826,6 @@ int libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
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);
}
else if (!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;
}

Expand Down Expand Up @@ -890,17 +869,6 @@ 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)
Expand Down
6 changes: 1 addition & 5 deletions tools/ocaml/libs/xc/xenctrl.ml
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@ type x86_arch_emulation_flags =

type x86_arch_misc_flags =
| X86_MSR_RELAXED
| X86_ASSISTED_XAPIC
| X86_ASSISTED_X2APIC

type xen_x86_arch_domainconfig =
{
Expand Down Expand Up @@ -132,9 +130,7 @@ type physinfo_cap_flag =

type arm_physinfo_cap_flag

type x86_physinfo_cap_flag =
| CAP_X86_ASSISTED_XAPIC
| CAP_X86_ASSISTED_X2APIC
type x86_physinfo_cap_flag

type arch_physinfo_cap_flags =
| ARM of arm_physinfo_cap_flag list
Expand Down
6 changes: 1 addition & 5 deletions tools/ocaml/libs/xc/xenctrl.mli
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ 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;
Expand Down Expand Up @@ -117,9 +115,7 @@ type physinfo_cap_flag =

type arm_physinfo_cap_flag

type x86_physinfo_cap_flag =
| CAP_X86_ASSISTED_XAPIC
| CAP_X86_ASSISTED_X2APIC
type x86_physinfo_cap_flag

type arch_physinfo_cap_flags =
| ARM of arm_physinfo_cap_flag list
Expand Down
7 changes: 2 additions & 5 deletions tools/ocaml/libs/xc/xenctrl_stubs.c
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,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_MISC_FLAGS_MAX max */
/* ! XEN_X86_ XEN_X86_MSR_RELAXED all */
(VAL_MISC_FLAGS);

#undef VAL_MISC_FLAGS
Expand Down Expand Up @@ -748,10 +748,7 @@ CAMLprim value stub_xc_physinfo(value xch)
Store_field(physinfo, 9, Val_int(c_physinfo.max_cpu_id + 1));

#if defined(__i386__) || defined(__x86_64__)
arch_cap_list = c_bitmap_to_ocaml_list
/* ! x86_physinfo_cap_flag CAP_X86_ none */
/* ! XEN_SYSCTL_PHYSCAP_X86_ XEN_SYSCTL_PHYSCAP_X86_MAX max */
(c_physinfo.arch_capabilities);
arch_cap_list = Tag_cons;

arch_cap_flags_tag = 1; /* tag x86 */
#else
Expand Down
8 changes: 0 additions & 8 deletions tools/xl/xl.c
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,6 @@ 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;

Expand Down Expand Up @@ -203,12 +201,6 @@ 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",
Expand Down
2 changes: 0 additions & 2 deletions tools/xl/xl.h
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,6 @@ 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,
Expand Down
6 changes: 2 additions & 4 deletions tools/xl/xl_info.c
Original file line number Diff line number Diff line change
Expand Up @@ -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%s%s\n",
maybe_printf("virt_caps :%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" : "",
Expand All @@ -221,9 +221,7 @@ 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_assisted_xapic ? " assisted_xapic" : "",
info.cap_assisted_x2apic ? " assisted_x2apic" : ""
info.cap_gnttab_v2 ? " gnttab-v2" : ""
);

vinfo = libxl_get_version_info(ctx);
Expand Down
19 changes: 0 additions & 19 deletions tools/xl/xl_parse.c
Original file line number Diff line number Diff line change
Expand Up @@ -2765,25 +2765,6 @@ void parse_config_data(const char *config_source,

xlu_cfg_get_defbool(config, "vpmu", &b_info->vpmu, 0);

if (b_info->type != LIBXL_DOMAIN_TYPE_PV) {
e = xlu_cfg_get_long(config, "assisted_xapic", &l , 0);
if (!e)
libxl_defbool_set(&b_info->arch_x86.assisted_xapic, l);
else if (e != ESRCH)
exit(1);
else if (assisted_xapic != -1) /* use global default if present */
libxl_defbool_set(&b_info->arch_x86.assisted_xapic, assisted_xapic);

e = xlu_cfg_get_long(config, "assisted_x2apic", &l, 0);
if (!e)
libxl_defbool_set(&b_info->arch_x86.assisted_x2apic, l);
else if (e != ESRCH)
exit(1);
else if (assisted_x2apic != -1) /* use global default if present */
libxl_defbool_set(&b_info->arch_x86.assisted_x2apic,
assisted_x2apic);
}

xlu_cfg_destroy(config);
}

Expand Down
Loading

0 comments on commit e5ac68a

Please sign in to comment.