[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v6 1/2] xen+tools: Report Interrupt Controller Virtualization capabilities on x86
On Tue, Mar 08, 2022 at 05:31:17PM +0000, Jane Malalane wrote: > Add XEN_SYSCTL_PHYSCAP_X86_ASSISTED_XAPIC and > XEN_SYSCTL_PHYSCAP_X86_ASSISTED_X2APIC to report accelerated xapic > and x2apic, on x86 hardware. > No such features are currently implemented on AMD hardware. > > HW assisted xAPIC virtualization will be reported if HW, at the > minimum, supports virtualize_apic_accesses as this feature alone means > that an access to the APIC page will cause an APIC-access VM exit. An > APIC-access VM exit provides a VMM with information about the access > causing the VM exit, unlike a regular EPT fault, thus simplifying some > internal handling. > > HW assisted x2APIC virtualization will be reported if HW supports > virtualize_x2apic_mode and, at least, either apic_reg_virt or > virtual_intr_delivery. This also means that > sysctl follows the conditionals in vmx_vlapic_msr_changed(). > > For that purpose, also add an arch-specific "capabilities" parameter > to struct xen_sysctl_physinfo. > > Note that this interface is intended to be compatible with AMD so that > AVIC support can be introduced in a future patch. Unlike Intel that > has multiple controls for APIC Virtualization, AMD has one global > 'AVIC Enable' control bit, so fine-graining of APIC virtualization > control cannot be done on a common interface. > > Suggested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> > Signed-off-by: Jane Malalane <jane.malalane@xxxxxxxxxx> Overall LGTM, just one question and one nit. > diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c > b/tools/ocaml/libs/xc/xenctrl_stubs.c > index 5b4fe72c8d..7e9c32ad1b 100644 > --- a/tools/ocaml/libs/xc/xenctrl_stubs.c > +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c > @@ -712,7 +712,7 @@ CAMLprim value stub_xc_send_debug_keys(value xch, value > keys) > CAMLprim value stub_xc_physinfo(value xch) > { > CAMLparam1(xch); > - CAMLlocal2(physinfo, cap_list); > + CAMLlocal3(physinfo, cap_list, arch_cap_list); > xc_physinfo_t c_physinfo; > int r; > > @@ -731,7 +731,7 @@ CAMLprim value stub_xc_physinfo(value xch) > /* ! XEN_SYSCTL_PHYSCAP_ XEN_SYSCTL_PHYSCAP_MAX max */ > (c_physinfo.capabilities); > > - physinfo = caml_alloc_tuple(10); > + physinfo = caml_alloc_tuple(11); > Store_field(physinfo, 0, Val_int(c_physinfo.threads_per_core)); > Store_field(physinfo, 1, Val_int(c_physinfo.cores_per_socket)); > Store_field(physinfo, 2, Val_int(c_physinfo.nr_cpus)); > @@ -743,6 +743,17 @@ CAMLprim value stub_xc_physinfo(value xch) > Store_field(physinfo, 8, cap_list); > Store_field(physinfo, 9, Val_int(c_physinfo.max_cpu_id + 1)); > > +#if defined(__i386__) || defined(__x86_64__) > + /* > + * arch_capabilities: physinfo_arch_cap_flag list; > + */ > + arch_cap_list = c_bitmap_to_ocaml_list > + /* ! physinfo_arch_cap_flag CAP_ none */ > + /* ! XEN_SYSCTL_PHYSCAP_ XEN_SYSCTL_PHYSCAP_X86_MAX max */ > + (c_physinfo.arch_capabilities); > + Store_field(physinfo, 10, arch_cap_list); > +#endif Have you tried to build this on Arm? I wonder whether the compiler will complain about arch_cap_list being unused there? > + > CAMLreturn(physinfo); > } > > 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 e1e1fa14e6..77ce0b2121 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_virtualize_x2apic_mode && > + (cpu_has_vmx_apic_reg_virt || > + cpu_has_vmx_virtual_intr_delivery); > + } > + > /* 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 f82abc2488..ad95c86aef 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; > + > static void cf_check l3_cache_get(void *arg) > { > struct cpuid4_info info; > @@ -135,6 +138,10 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi) > pi->capabilities |= XEN_SYSCTL_PHYSCAP_hap; > if ( IS_ENABLED(CONFIG_SHADOW_PAGING) ) > pi->capabilities |= XEN_SYSCTL_PHYSCAP_shadow; > + if ( assisted_xapic_available ) > + pi->arch_capabilities |= XEN_SYSCTL_PHYSCAP_X86_ASSISTED_XAPIC; > + if ( assisted_x2apic_available ) > + pi->arch_capabilities |= XEN_SYSCTL_PHYSCAP_X86_ASSISTED_X2APIC; > } > > long arch_do_sysctl( > diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h > index 55252e97f2..7fe05be0c9 100644 > --- a/xen/include/public/sysctl.h > +++ b/xen/include/public/sysctl.h > @@ -35,7 +35,7 @@ > #include "domctl.h" > #include "physdev.h" > > -#define XEN_SYSCTL_INTERFACE_VERSION 0x00000014 > +#define XEN_SYSCTL_INTERFACE_VERSION 0x00000015 > > /* > * Read console content from Xen buffer ring. > @@ -111,6 +111,13 @@ struct xen_sysctl_tbuf_op { > /* Max XEN_SYSCTL_PHYSCAP_* constant. Used for ABI checking. */ > #define XEN_SYSCTL_PHYSCAP_MAX XEN_SYSCTL_PHYSCAP_gnttab_v2 > > +/* The platform supports x{2}apic hardware assisted emulation. */ > +#define XEN_SYSCTL_PHYSCAP_X86_ASSISTED_XAPIC (1u << 0) > +#define XEN_SYSCTL_PHYSCAP_X86_ASSISTED_X2APIC (1u << 1) > + > +/* Max XEN_SYSCTL_PHYSCAP_X86__* constant. Used for ABI checking. */ > +#define XEN_SYSCTL_PHYSCAP_X86_MAX XEN_SYSCTL_PHYSCAP_X86_ASSISTED_X2APIC > + > struct xen_sysctl_physinfo { > uint32_t threads_per_core; > uint32_t cores_per_socket; > @@ -120,6 +127,8 @@ struct xen_sysctl_physinfo { > uint32_t max_node_id; /* Largest possible node ID on this host */ > uint32_t cpu_khz; > uint32_t capabilities;/* XEN_SYSCTL_PHYSCAP_??? */ > + uint32_t arch_capabilities;/* XEN_SYSCTL_PHYSCAP_X86{ARM}_??? */ Nit: comment should likely be: XEN_SYSCTL_PHYSCAP_{X86,ARM,...}_??? Thanks, Roger.
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |