diff mbox series

[v4,1/2] xen+tools: Report Interrupt Controller Virtualization capabilities on x86

Message ID 20220302150056.14381-2-jane.malalane@citrix.com (mailing list archive)
State Superseded
Headers show
Series xen: Report and use hardware APIC virtualization capabilities | expand

Commit Message

Jane Malalane March 2, 2022, 3 p.m. UTC
Add XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_xapic and
XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_x2apic to report accelerated xapic
and x2apic, on x86 hardware.
No such features are currently implemented on AMD hardware.

For that purpose, also add an arch-specific "capabilities" parameter
to struct xen_sysctl_physinfo.

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. Therefore, for xAPIC HW
assisted virtualization support to be reported, HW must support
virtualize_apic_accesses as well as apic_reg_virt. For x2APIC HW
assisted virtualization reporting, virtualize_x2apic_mode must be
supported alongside apic_reg_virt and virtual_intr_delivery.

Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jane Malalane <jane.malalane@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: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Bertrand Marquis <bertrand.marquis@arm.com>
CC: Jun Nakajima <jun.nakajima@intel.com>
CC: Kevin Tian <kevin.tian@intel.com>
CC: "Roger Pau Monné" <roger.pau@citrix.com>

v4:
 * Fallback to the original v2/v1 conditions for setting
   assisted_xapic_available and assisted_x2apic_available so that in
   the future APIC virtualization can be exposed on AMD hardware
   since fine-graining of "AVIC" is not supported, i.e., AMD solely
   uses "AVIC Enable". This also means that sysctl mimics what's
   exposed in CPUID.

v3:
 * Define XEN_SYSCTL_PHYSCAP_ARCH_MAX for ABI checking and actually
   set "arch_capbilities", via a call to c_bitmap_to_ocaml_list()
 * Have assisted_x2apic_available only depend on
   cpu_has_vmx_virtualize_x2apic_mode

v2:
 * Use one macro LIBXL_HAVE_PHYSINFO_ASSISTED_APIC instead of two
 * Pass xcpyshinfo as a pointer in libxl__arch_get_physinfo
 * Set assisted_x{2}apic_available to be conditional upon "bsp" and
   annotate it with __ro_after_init
 * Change XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_X{2}APIC to
   _X86_ASSISTED_X{2}APIC
 * Keep XEN_SYSCTL_PHYSCAP_X86_ASSISTED_X{2}APIC contained within
   sysctl.h
 * Fix padding introduced in struct xen_sysctl_physinfo and bump
   XEN_SYSCTL_INTERFACE_VERSION
---
 tools/golang/xenlight/helpers.gen.go |  4 ++++
 tools/golang/xenlight/types.gen.go   |  2 ++
 tools/include/libxl.h                |  7 +++++++
 tools/libs/light/libxl.c             |  3 +++
 tools/libs/light/libxl_arch.h        |  4 ++++
 tools/libs/light/libxl_arm.c         |  5 +++++
 tools/libs/light/libxl_types.idl     |  2 ++
 tools/libs/light/libxl_x86.c         | 11 +++++++++++
 tools/ocaml/libs/xc/xenctrl.ml       |  5 +++++
 tools/ocaml/libs/xc/xenctrl.mli      |  5 +++++
 tools/ocaml/libs/xc/xenctrl_stubs.c  | 14 +++++++++++---
 tools/xl/xl_info.c                   |  6 ++++--
 xen/arch/x86/hvm/vmx/vmcs.c          | 10 ++++++++++
 xen/arch/x86/include/asm/domain.h    |  3 +++
 xen/arch/x86/sysctl.c                |  7 +++++++
 xen/include/public/sysctl.h          | 11 ++++++++++-
 16 files changed, 93 insertions(+), 6 deletions(-)

Comments

Jan Beulich March 3, 2022, 11:37 a.m. UTC | #1
On 02.03.2022 16:00, Jane Malalane wrote:
> Add XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_xapic and
> XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_x2apic to report accelerated xapic
> and x2apic, on x86 hardware.
> No such features are currently implemented on AMD hardware.
> 
> For that purpose, also add an arch-specific "capabilities" parameter
> to struct xen_sysctl_physinfo.
> 
> 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. Therefore, for xAPIC HW
> assisted virtualization support to be reported, HW must support
> virtualize_apic_accesses as well as apic_reg_virt.

Okay, here you now describe _what_ is being implemented, but I'm
afraid it still lacks justification (beyond making this re-usable for
AVIC, which imo can only be a secondary goal). You actually say ...

> For x2APIC HW
> assisted virtualization reporting, virtualize_x2apic_mode must be
> supported alongside apic_reg_virt and virtual_intr_delivery.
> 
> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Jane Malalane <jane.malalane@citrix.com>
> 
> v4:
>  * Fallback to the original v2/v1 conditions for setting
>    assisted_xapic_available and assisted_x2apic_available so that in
>    the future APIC virtualization can be exposed on AMD hardware
>    since fine-graining of "AVIC" is not supported, i.e., AMD solely
>    uses "AVIC Enable". This also means that sysctl mimics what's
>    exposed in CPUID.

... more here: You claim similarity with CPUID. That's a possible route,
but we need to be clear that these CPUID flags are optimization hints
for the guest to use, while the new control is intended to be a functional
one. Hence it's not obvious that CPUID wants following, and not instead
the conditionals used in vmx_vlapic_msr_changed() (or yet something else).

What's worse though: What you say is true for x2APIC, but not for xAPIC.
Which effectively is in line with vmx_vlapic_msr_changed() and CPUID
handling also agreeing as far as x2APIC is concerned, but disagreeing on
the xAPIC side. I can only once again try to express that it may well be
that pre-existing code wants adjusting before actually making the changes
you're after.

> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -343,6 +343,16 @@ 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 &&
> +                                    cpu_has_vmx_apic_reg_virt);
> +        assisted_x2apic_available = (cpu_has_vmx_virtualize_x2apic_mode &&
> +                                     cpu_has_vmx_apic_reg_virt &&
> +                                     cpu_has_vmx_virtual_intr_delivery);
> +    }

If the conditions were to stay as they are, I'd like to suggest pulling
out the cpu_has_vmx_apic_reg_virt into the enclosing if()'s condition.
Additionally I think the comment wants to contain a pointer to what this
wants to remain in sync with. That other side may then want to gain a
comment pointing back here.

Jan
Jane Malalane March 3, 2022, 4:37 p.m. UTC | #2
On 03/03/2022 11:37, Jan Beulich wrote:
> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.
> 
> On 02.03.2022 16:00, Jane Malalane wrote:
>> Add XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_xapic and
>> XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_x2apic to report accelerated xapic
>> and x2apic, on x86 hardware.
>> No such features are currently implemented on AMD hardware.
>>
>> For that purpose, also add an arch-specific "capabilities" parameter
>> to struct xen_sysctl_physinfo.
>>
>> 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. Therefore, for xAPIC HW
>> assisted virtualization support to be reported, HW must support
>> virtualize_apic_accesses as well as apic_reg_virt.
> 
> Okay, here you now describe _what_ is being implemented, but I'm
> afraid it still lacks justification (beyond making this re-usable for
> AVIC, which imo can only be a secondary goal). You actually say ...
> 
>> For x2APIC HW
>> assisted virtualization reporting, virtualize_x2apic_mode must be
>> supported alongside apic_reg_virt and virtual_intr_delivery.
>>
>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Signed-off-by: Jane Malalane <jane.malalane@citrix.com>
>>
>> v4:
>>   * Fallback to the original v2/v1 conditions for setting
>>     assisted_xapic_available and assisted_x2apic_available so that in
>>     the future APIC virtualization can be exposed on AMD hardware
>>     since fine-graining of "AVIC" is not supported, i.e., AMD solely
>>     uses "AVIC Enable". This also means that sysctl mimics what's
>>     exposed in CPUID.
> 
> ... more here: You claim similarity with CPUID. That's a possible route,
> but we need to be clear that these CPUID flags are optimization hints
> for the guest to use, while the new control is intended to be a functional
> one. Hence it's not obvious that CPUID wants following, and not instead
> the conditionals used in vmx_vlapic_msr_changed() (or yet something else).
> 
> What's worse though: What you say is true for x2APIC, but not for xAPIC.
> Which effectively is in line with vmx_vlapic_msr_changed() and CPUID
> handling also agreeing as far as x2APIC is concerned, but disagreeing on
> the xAPIC side. I can only once again try to express that it may well be
> that pre-existing code wants adjusting before actually making the changes
> you're after.


I've been thinking about this. Considering what you say, I propose:

- having assisted_x2apic_available = cpu_has_vmx_virtualize_x2apic_mode 
&& (cpu_has_vmx_apic_reg_virt || cpu_has_vmx_virtual_intr_delivery). 
This would mean that on Intel CPUs has_assisted_x2apic==1 would signify 
that there is at least "some" assistance*, whereas on AMD it would 
signify that there is full assistance (assistance here meaning no VM-exits).
* apic_reg_virt prevents VM exits on execution of RDMSR and 
virtual_intr_delivery prevents VM exits on execution of RDMSR, from what 
I've gathered.
- having assisted_xapic_available = cpu_has_vmx_virtualize_apic_accesses 
&& cpu_has_vmx_apic_reg_virt because apic_reg_virt is neccessary for 
"any" assistance.

- Currently, the code only sets SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE if 
"some" assistance is guaranteed but sets 
SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES even if no assistance is 
guaranteed. So the adjustment to the pre-existing code that I propose is
adding cpu_has_vmx_apic_reg_virt to the initial check in 
vmx_vlapic_msr_changed():

  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 &&
+         !cpu_has_vmx_apic_reg_virt &&
           !virtualize_x2apic_mode )
          return;


which would then eventually just be what I currently have:
+    if ( !has_assisted_xapic(v->domain) &&
+         !has_assisted_x2apic(v->domain) )
          return;

So, essentially, the only difference from v4 would be 
assisted_x2apic_available = (cpu_has_vmx_virtualize_x2apic_mode &&
	  	             (cpu_has_vmx_apic_reg_virt ||
			      cpu_has_vmx_virtual_intr_delivery));	

sysctl would now coincide with CPUID for xAPIC but not x2APIC (for CPUID 
the condition is the AND of all features unlike the 
assisted_x2apic_available proposed). IOW, it would follow the 
conditionals used in vmx_vlapic_msr_changed(), if we take the change to 
vmx_vlapic_msr_changed() above.

Thank you,

Jane.
Jan Beulich March 4, 2022, 8:17 a.m. UTC | #3
On 03.03.2022 17:37, Jane Malalane wrote:
> On 03/03/2022 11:37, Jan Beulich wrote:
>> On 02.03.2022 16:00, Jane Malalane wrote:
>>> Add XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_xapic and
>>> XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_x2apic to report accelerated xapic
>>> and x2apic, on x86 hardware.
>>> No such features are currently implemented on AMD hardware.
>>>
>>> For that purpose, also add an arch-specific "capabilities" parameter
>>> to struct xen_sysctl_physinfo.
>>>
>>> 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. Therefore, for xAPIC HW
>>> assisted virtualization support to be reported, HW must support
>>> virtualize_apic_accesses as well as apic_reg_virt.
>>
>> Okay, here you now describe _what_ is being implemented, but I'm
>> afraid it still lacks justification (beyond making this re-usable for
>> AVIC, which imo can only be a secondary goal). You actually say ...
>>
>>> For x2APIC HW
>>> assisted virtualization reporting, virtualize_x2apic_mode must be
>>> supported alongside apic_reg_virt and virtual_intr_delivery.
>>>
>>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> Signed-off-by: Jane Malalane <jane.malalane@citrix.com>
>>>
>>> v4:
>>>   * Fallback to the original v2/v1 conditions for setting
>>>     assisted_xapic_available and assisted_x2apic_available so that in
>>>     the future APIC virtualization can be exposed on AMD hardware
>>>     since fine-graining of "AVIC" is not supported, i.e., AMD solely
>>>     uses "AVIC Enable". This also means that sysctl mimics what's
>>>     exposed in CPUID.
>>
>> ... more here: You claim similarity with CPUID. That's a possible route,
>> but we need to be clear that these CPUID flags are optimization hints
>> for the guest to use, while the new control is intended to be a functional
>> one. Hence it's not obvious that CPUID wants following, and not instead
>> the conditionals used in vmx_vlapic_msr_changed() (or yet something else).
>>
>> What's worse though: What you say is true for x2APIC, but not for xAPIC.
>> Which effectively is in line with vmx_vlapic_msr_changed() and CPUID
>> handling also agreeing as far as x2APIC is concerned, but disagreeing on
>> the xAPIC side. I can only once again try to express that it may well be
>> that pre-existing code wants adjusting before actually making the changes
>> you're after.
> 
> 
> I've been thinking about this. Considering what you say, I propose:
> 
> - having assisted_x2apic_available = cpu_has_vmx_virtualize_x2apic_mode 
> && (cpu_has_vmx_apic_reg_virt || cpu_has_vmx_virtual_intr_delivery). 
> This would mean that on Intel CPUs has_assisted_x2apic==1 would signify 
> that there is at least "some" assistance*, whereas on AMD it would 
> signify that there is full assistance (assistance here meaning no VM-exits).
> * apic_reg_virt prevents VM exits on execution of RDMSR and 
> virtual_intr_delivery prevents VM exits on execution of RDMSR, from what 
> I've gathered.

I agree with this part of the plan.

> - having assisted_xapic_available = cpu_has_vmx_virtualize_apic_accesses 
> && cpu_has_vmx_apic_reg_virt because apic_reg_virt is neccessary for 
> "any" assistance.

Not exactly, aiui: cpu_has_vmx_virtualize_apic_accesses alone is beneficial
because a separate VM exit is then used, simplifying some internal handling.
There might actually be room for improvement in our handling of this, as we
presently use the exit qualification only to accelerate EOI writes.

> - Currently, the code only sets SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE if 
> "some" assistance is guaranteed but sets 
> SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES even if no assistance is 
> guaranteed. So the adjustment to the pre-existing code that I propose is
> adding cpu_has_vmx_apic_reg_virt to the initial check in 
> vmx_vlapic_msr_changed():
> 
>   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 &&
> +         !cpu_has_vmx_apic_reg_virt &&
>            !virtualize_x2apic_mode )
>           return;

I'd suggest the opposite for the xAPIC case: Leave the condition here
unchanged, but consider tightening the condition for the CPUID flag.
That'll bring xAPIC handling more in line with x2APIC one.

> which would then eventually just be what I currently have:
> +    if ( !has_assisted_xapic(v->domain) &&
> +         !has_assisted_x2apic(v->domain) )
>           return;

Yes, the eventual form is expected in any event.

Jan

> So, essentially, the only difference from v4 would be 
> assisted_x2apic_available = (cpu_has_vmx_virtualize_x2apic_mode &&
> 	  	             (cpu_has_vmx_apic_reg_virt ||
> 			      cpu_has_vmx_virtual_intr_delivery));	
> 
> sysctl would now coincide with CPUID for xAPIC but not x2APIC (for CPUID 
> the condition is the AND of all features unlike the 
> assisted_x2apic_available proposed). IOW, it would follow the 
> conditionals used in vmx_vlapic_msr_changed(), if we take the change to 
> vmx_vlapic_msr_changed() above.
> 
> Thank you,
> 
> Jane.
Jane Malalane March 7, 2022, 12:17 p.m. UTC | #4
On 04/03/2022 08:17, Jan Beulich wrote:
> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.
> 
> On 03.03.2022 17:37, Jane Malalane wrote:
>> On 03/03/2022 11:37, Jan Beulich wrote:
>>> On 02.03.2022 16:00, Jane Malalane wrote:
>>>> Add XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_xapic and
>>>> XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_x2apic to report accelerated xapic
>>>> and x2apic, on x86 hardware.
>>>> No such features are currently implemented on AMD hardware.
>>>>
>>>> For that purpose, also add an arch-specific "capabilities" parameter
>>>> to struct xen_sysctl_physinfo.
>>>>
>>>> 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. Therefore, for xAPIC HW
>>>> assisted virtualization support to be reported, HW must support
>>>> virtualize_apic_accesses as well as apic_reg_virt.
>>>
>>> Okay, here you now describe _what_ is being implemented, but I'm
>>> afraid it still lacks justification (beyond making this re-usable for
>>> AVIC, which imo can only be a secondary goal). You actually say ...
Is the following any better...?

"Add XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_xapic and
XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_x2apic to report accelerated xapic
and x2apic, on x86 hardware.
No such features are currently implemented on AMD hardware.

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 is due to apic_reg_virt and 
virtual_intr_delivery preventing a VM exit from occuring or at least 
replacing a regular EPT fault VM-exit with an APIC-access VM-exit on 
read and write APIC accesses, respectively.
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."

I previously didn't add here any info about the assistance that each CPU 
bit provides to avoid repitition, as I talk about that in patch 2, but I 
interpreted from your comment that it might be helpful to add that here 
too.
>>>
>>>> For x2APIC HW
>>>> assisted virtualization reporting, virtualize_x2apic_mode must be
>>>> supported alongside apic_reg_virt and virtual_intr_delivery.
>>>>
>>>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> Signed-off-by: Jane Malalane <jane.malalane@citrix.com>
>>>>
>>>> v4:
>>>>    * Fallback to the original v2/v1 conditions for setting
>>>>      assisted_xapic_available and assisted_x2apic_available so that in
>>>>      the future APIC virtualization can be exposed on AMD hardware
>>>>      since fine-graining of "AVIC" is not supported, i.e., AMD solely
>>>>      uses "AVIC Enable". This also means that sysctl mimics what's
>>>>      exposed in CPUID.
>>>
>>> ... more here: You claim similarity with CPUID. That's a possible route,
>>> but we need to be clear that these CPUID flags are optimization hints
>>> for the guest to use, while the new control is intended to be a functional
>>> one. Hence it's not obvious that CPUID wants following, and not instead
>>> the conditionals used in vmx_vlapic_msr_changed() (or yet something else).
>>>
>>> What's worse though: What you say is true for x2APIC, but not for xAPIC.
>>> Which effectively is in line with vmx_vlapic_msr_changed() and CPUID
>>> handling also agreeing as far as x2APIC is concerned, but disagreeing on
>>> the xAPIC side. I can only once again try to express that it may well be
>>> that pre-existing code wants adjusting before actually making the changes
>>> you're after.
>>
>>
>> I've been thinking about this. Considering what you say, I propose:
>>
>> - having assisted_x2apic_available = cpu_has_vmx_virtualize_x2apic_mode
>> && (cpu_has_vmx_apic_reg_virt || cpu_has_vmx_virtual_intr_delivery).
>> This would mean that on Intel CPUs has_assisted_x2apic==1 would signify
>> that there is at least "some" assistance*, whereas on AMD it would
>> signify that there is full assistance (assistance here meaning no VM-exits).
>> * apic_reg_virt prevents VM exits on execution of RDMSR and
>> virtual_intr_delivery prevents VM exits on execution of RDMSR, from what
>> I've gathered.
> 
> I agree with this part of the plan.
> 
>> - having assisted_xapic_available = cpu_has_vmx_virtualize_apic_accesses
>> && cpu_has_vmx_apic_reg_virt because apic_reg_virt is neccessary for
>> "any" assistance.
> 
> Not exactly, aiui: cpu_has_vmx_virtualize_apic_accesses alone is beneficial
> because a separate VM exit is then used, simplifying some internal handling.
> There might actually be room for improvement in our handling of this, as we
> presently use the exit qualification only to accelerate EOI writes.
I agree with you, by "assistance" in my response I meant "no VM-exits" 
but yes there is assistance, beyond absence of a VM exit, with 
virtualize_apic_acesses alone.>> - Currently, the code only sets 
SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE if
>> "some" assistance is guaranteed but sets
>> SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES even if no assistance is
>> guaranteed. So the adjustment to the pre-existing code that I propose is
>> adding cpu_has_vmx_apic_reg_virt to the initial check in
>> vmx_vlapic_msr_changed():
>>
>>    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 &&
>> +         !cpu_has_vmx_apic_reg_virt &&
>>             !virtualize_x2apic_mode )
>>            return;
> 
> I'd suggest the opposite for the xAPIC case: Leave the condition here
> unchanged, but consider tightening the condition for the CPUID flag.
> That'll bring xAPIC handling more in line with x2APIC one
Sounds good.


Thanks again,

Jane.
Jan Beulich March 7, 2022, 12:31 p.m. UTC | #5
On 07.03.2022 13:17, Jane Malalane wrote:
> On 04/03/2022 08:17, Jan Beulich wrote:
>> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.
>>
>> On 03.03.2022 17:37, Jane Malalane wrote:
>>> On 03/03/2022 11:37, Jan Beulich wrote:
>>>> On 02.03.2022 16:00, Jane Malalane wrote:
>>>>> Add XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_xapic and
>>>>> XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_x2apic to report accelerated xapic
>>>>> and x2apic, on x86 hardware.
>>>>> No such features are currently implemented on AMD hardware.
>>>>>
>>>>> For that purpose, also add an arch-specific "capabilities" parameter
>>>>> to struct xen_sysctl_physinfo.
>>>>>
>>>>> 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. Therefore, for xAPIC HW
>>>>> assisted virtualization support to be reported, HW must support
>>>>> virtualize_apic_accesses as well as apic_reg_virt.
>>>>
>>>> Okay, here you now describe _what_ is being implemented, but I'm
>>>> afraid it still lacks justification (beyond making this re-usable for
>>>> AVIC, which imo can only be a secondary goal). You actually say ...
> Is the following any better...?
> 
> "Add XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_xapic and
> XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_x2apic to report accelerated xapic
> and x2apic, on x86 hardware.
> No such features are currently implemented on AMD hardware.
> 
> 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 is due to apic_reg_virt and 
> virtual_intr_delivery preventing a VM exit from occuring or at least 
> replacing a regular EPT fault VM-exit with an APIC-access VM-exit on 
> read and write APIC accesses, respectively.
> 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."

Yes, this looks quite a bit better, thanks. Assuming, of course, it's
in sync with the code in v5 ...

Jan
Jane Malalane March 7, 2022, 1:16 p.m. UTC | #6
On 07/03/2022 12:31, Jan Beulich wrote:
> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.
> 
> On 07.03.2022 13:17, Jane Malalane wrote:
>> On 04/03/2022 08:17, Jan Beulich wrote:
>>> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.
>>>
>>> On 03.03.2022 17:37, Jane Malalane wrote:
>>>> On 03/03/2022 11:37, Jan Beulich wrote:
>>>>> On 02.03.2022 16:00, Jane Malalane wrote:
>>>>>> Add XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_xapic and
>>>>>> XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_x2apic to report accelerated xapic
>>>>>> and x2apic, on x86 hardware.
>>>>>> No such features are currently implemented on AMD hardware.
>>>>>>
>>>>>> For that purpose, also add an arch-specific "capabilities" parameter
>>>>>> to struct xen_sysctl_physinfo.
>>>>>>
>>>>>> 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. Therefore, for xAPIC HW
>>>>>> assisted virtualization support to be reported, HW must support
>>>>>> virtualize_apic_accesses as well as apic_reg_virt.
>>>>>
>>>>> Okay, here you now describe _what_ is being implemented, but I'm
>>>>> afraid it still lacks justification (beyond making this re-usable for
>>>>> AVIC, which imo can only be a secondary goal). You actually say ...
>> Is the following any better...?
>>
>> "Add XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_xapic and
>> XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_x2apic to report accelerated xapic
>> and x2apic, on x86 hardware.
>> No such features are currently implemented on AMD hardware.
>>
>> 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 is due to apic_reg_virt and
>> virtual_intr_delivery preventing a VM exit from occuring or at least
>> replacing a regular EPT fault VM-exit with an APIC-access VM-exit on
>> read and write APIC accesses, respectively.
>> 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."
> 
> Yes, this looks quite a bit better, thanks. Assuming, of course, it's
> in sync with the code in v5 ...
Yes, ofc.

Just one thing, since vmx_vlapic_msr_changed() uses 
has_assisted_x{2}apic anyway do you think it's still necessary to add a 
comment pointing to this function in vmx_init_vmcs_config() when setting 
asissted_x{2}apic_available and v.v. ?

Thanks,

Jane.
Jan Beulich March 7, 2022, 1:37 p.m. UTC | #7
On 07.03.2022 14:16, Jane Malalane wrote:
> On 07/03/2022 12:31, Jan Beulich wrote:
>> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.
>>
>> On 07.03.2022 13:17, Jane Malalane wrote:
>>> On 04/03/2022 08:17, Jan Beulich wrote:
>>>> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.
>>>>
>>>> On 03.03.2022 17:37, Jane Malalane wrote:
>>>>> On 03/03/2022 11:37, Jan Beulich wrote:
>>>>>> On 02.03.2022 16:00, Jane Malalane wrote:
>>>>>>> Add XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_xapic and
>>>>>>> XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_x2apic to report accelerated xapic
>>>>>>> and x2apic, on x86 hardware.
>>>>>>> No such features are currently implemented on AMD hardware.
>>>>>>>
>>>>>>> For that purpose, also add an arch-specific "capabilities" parameter
>>>>>>> to struct xen_sysctl_physinfo.
>>>>>>>
>>>>>>> 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. Therefore, for xAPIC HW
>>>>>>> assisted virtualization support to be reported, HW must support
>>>>>>> virtualize_apic_accesses as well as apic_reg_virt.
>>>>>>
>>>>>> Okay, here you now describe _what_ is being implemented, but I'm
>>>>>> afraid it still lacks justification (beyond making this re-usable for
>>>>>> AVIC, which imo can only be a secondary goal). You actually say ...
>>> Is the following any better...?
>>>
>>> "Add XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_xapic and
>>> XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_x2apic to report accelerated xapic
>>> and x2apic, on x86 hardware.
>>> No such features are currently implemented on AMD hardware.
>>>
>>> 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 is due to apic_reg_virt and
>>> virtual_intr_delivery preventing a VM exit from occuring or at least
>>> replacing a regular EPT fault VM-exit with an APIC-access VM-exit on
>>> read and write APIC accesses, respectively.
>>> 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."
>>
>> Yes, this looks quite a bit better, thanks. Assuming, of course, it's
>> in sync with the code in v5 ...
> Yes, ofc.
> 
> Just one thing, since vmx_vlapic_msr_changed() uses 
> has_assisted_x{2}apic anyway do you think it's still necessary to add a 
> comment pointing to this function in vmx_init_vmcs_config() when setting 
> asissted_x{2}apic_available and v.v. ?

If they both use the same, non-open-coded condition, then I don't think a
cross reference is needed.

Jan
diff mbox series

Patch

diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go
index b746ff1081..dd4e6c9f14 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -3373,6 +3373,8 @@  x.CapVmtrace = bool(xc.cap_vmtrace)
 x.CapVpmu = bool(xc.cap_vpmu)
 x.CapGnttabV1 = bool(xc.cap_gnttab_v1)
 x.CapGnttabV2 = bool(xc.cap_gnttab_v2)
+x.CapAssistedXapic = bool(xc.cap_assisted_xapic)
+x.CapAssistedX2Apic = bool(xc.cap_assisted_x2apic)
 
  return nil}
 
@@ -3407,6 +3409,8 @@  xc.cap_vmtrace = C.bool(x.CapVmtrace)
 xc.cap_vpmu = C.bool(x.CapVpmu)
 xc.cap_gnttab_v1 = C.bool(x.CapGnttabV1)
 xc.cap_gnttab_v2 = C.bool(x.CapGnttabV2)
+xc.cap_assisted_xapic = C.bool(x.CapAssistedXapic)
+xc.cap_assisted_x2apic = C.bool(x.CapAssistedX2Apic)
 
  return nil
  }
diff --git a/tools/golang/xenlight/types.gen.go b/tools/golang/xenlight/types.gen.go
index b1e84d5258..87be46c745 100644
--- a/tools/golang/xenlight/types.gen.go
+++ b/tools/golang/xenlight/types.gen.go
@@ -1014,6 +1014,8 @@  CapVmtrace bool
 CapVpmu bool
 CapGnttabV1 bool
 CapGnttabV2 bool
+CapAssistedXapic bool
+CapAssistedX2Apic bool
 }
 
 type Connectorinfo struct {
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index 51a9b6cfac..94e6355822 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -528,6 +528,13 @@ 
 #define LIBXL_HAVE_MAX_GRANT_VERSION 1
 
 /*
+ * LIBXL_HAVE_PHYSINFO_ASSISTED_APIC indicates that libxl_physinfo has
+ * cap_assisted_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 ABI compatibility
  *
  * The only guarantee which libxl makes regarding ABI compatibility
diff --git a/tools/libs/light/libxl.c b/tools/libs/light/libxl.c
index a0bf7d186f..6d699951e2 100644
--- a/tools/libs/light/libxl.c
+++ b/tools/libs/light/libxl.c
@@ -15,6 +15,7 @@ 
 #include "libxl_osdeps.h"
 
 #include "libxl_internal.h"
+#include "libxl_arch.h"
 
 int libxl_ctx_alloc(libxl_ctx **pctx, int version,
                     unsigned flags, xentoollog_logger * lg)
@@ -410,6 +411,8 @@  int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo)
     physinfo->cap_gnttab_v2 =
         !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_gnttab_v2);
 
+    libxl__arch_get_physinfo(physinfo, &xcphysinfo);
+
     GC_FREE;
     return 0;
 }
diff --git a/tools/libs/light/libxl_arch.h b/tools/libs/light/libxl_arch.h
index 1522ecb97f..207ceac6a1 100644
--- a/tools/libs/light/libxl_arch.h
+++ b/tools/libs/light/libxl_arch.h
@@ -86,6 +86,10 @@  int libxl__arch_extra_memory(libxl__gc *gc,
                              uint64_t *out);
 
 _hidden
+void libxl__arch_get_physinfo(libxl_physinfo *physinfo,
+                              const xc_physinfo_t *xcphysinfo);
+
+_hidden
 void libxl__arch_update_domain_config(libxl__gc *gc,
                                       libxl_domain_config *dst,
                                       const libxl_domain_config *src);
diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index eef1de0939..39fdca1b49 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -1431,6 +1431,11 @@  int libxl__arch_passthrough_mode_setdefault(libxl__gc *gc,
     return rc;
 }
 
+void libxl__arch_get_physinfo(libxl_physinfo *physinfo,
+                              const xc_physinfo_t *xcphysinfo)
+{
+}
+
 void libxl__arch_update_domain_config(libxl__gc *gc,
                                       libxl_domain_config *dst,
                                       const libxl_domain_config *src)
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index 2a42da2f7d..42ac6c357b 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -1068,6 +1068,8 @@  libxl_physinfo = Struct("physinfo", [
     ("cap_vpmu", bool),
     ("cap_gnttab_v1", bool),
     ("cap_gnttab_v2", bool),
+    ("cap_assisted_xapic", bool),
+    ("cap_assisted_x2apic", bool),
     ], dir=DIR_OUT)
 
 libxl_connectorinfo = Struct("connectorinfo", [
diff --git a/tools/libs/light/libxl_x86.c b/tools/libs/light/libxl_x86.c
index 1feadebb18..e0a06ecfe3 100644
--- a/tools/libs/light/libxl_x86.c
+++ b/tools/libs/light/libxl_x86.c
@@ -866,6 +866,17 @@  int libxl__arch_passthrough_mode_setdefault(libxl__gc *gc,
     return rc;
 }
 
+void libxl__arch_get_physinfo(libxl_physinfo *physinfo,
+                              const xc_physinfo_t *xcphysinfo)
+{
+    physinfo->cap_assisted_xapic =
+        !!(xcphysinfo->arch_capabilities &
+           XEN_SYSCTL_PHYSCAP_X86_ASSISTED_XAPIC);
+    physinfo->cap_assisted_x2apic =
+        !!(xcphysinfo->arch_capabilities &
+           XEN_SYSCTL_PHYSCAP_X86_ASSISTED_X2APIC);
+}
+
 void libxl__arch_update_domain_config(libxl__gc *gc,
                                       libxl_domain_config *dst,
                                       const libxl_domain_config *src)
diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index 7503031d8f..21783d3622 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -127,6 +127,10 @@  type physinfo_cap_flag =
 	| CAP_Gnttab_v1
 	| CAP_Gnttab_v2
 
+type physinfo_arch_cap_flag =
+	| CAP_X86_ASSISTED_XAPIC
+	| CAP_X86_ASSISTED_X2APIC
+
 type physinfo =
 {
 	threads_per_core : int;
@@ -139,6 +143,7 @@  type physinfo =
 	scrub_pages      : nativeint;
 	(* XXX hw_cap *)
 	capabilities     : physinfo_cap_flag list;
+	arch_capabilities : physinfo_arch_cap_flag list;
 	max_nr_cpus      : int;
 }
 
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index d1d9c9247a..af6ba3d1a0 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -112,6 +112,10 @@  type physinfo_cap_flag =
   | CAP_Gnttab_v1
   | CAP_Gnttab_v2
 
+type physinfo_arch_cap_flag =
+  | CAP_X86_ASSISTED_XAPIC
+  | CAP_X86_ASSISTED_X2APIC
+
 type physinfo = {
   threads_per_core : int;
   cores_per_socket : int;
@@ -122,6 +126,7 @@  type physinfo = {
   free_pages       : nativeint;
   scrub_pages      : nativeint;
   capabilities     : physinfo_cap_flag list;
+  arch_capabilities : physinfo_arch_cap_flag list;
   max_nr_cpus      : int; (** compile-time max possible number of nr_cpus *)
 }
 type version = { major : int; minor : int; extra : string; }
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c
index 5b4fe72c8d..e0d49b18d2 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;
 
@@ -730,8 +730,15 @@  CAMLprim value stub_xc_physinfo(value xch)
 		/* ! physinfo_cap_flag CAP_ lc */
 		/* ! XEN_SYSCTL_PHYSCAP_ XEN_SYSCTL_PHYSCAP_MAX max */
 		(c_physinfo.capabilities);
+	/*
+	 * 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);
 
-	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));
@@ -741,7 +748,8 @@  CAMLprim value stub_xc_physinfo(value xch)
 	Store_field(physinfo, 6, caml_copy_nativeint(c_physinfo.free_pages));
 	Store_field(physinfo, 7, caml_copy_nativeint(c_physinfo.scrub_pages));
 	Store_field(physinfo, 8, cap_list);
-	Store_field(physinfo, 9, Val_int(c_physinfo.max_cpu_id + 1));
+	Store_field(physinfo, 9, arch_cap_list);
+	Store_field(physinfo, 10, Val_int(c_physinfo.max_cpu_id + 1));
 
 	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..6f1fec49b9 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -343,6 +343,16 @@  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 &&
+                                    cpu_has_vmx_apic_reg_virt);
+        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}_??? */
+    uint32_t pad;
     uint64_aligned_t total_pages;
     uint64_aligned_t free_pages;
     uint64_aligned_t scrub_pages;