On 09/03/2022 10:36, Roger Pau Monné wrote:
> 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.coop...@citrix.com>
>> Signed-off-by: Jane Malalane <jane.malal...@citrix.com>
>
> 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?
Built it - no error.
Thank you,
Jane.
