[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH for-4.17 2/2] hvm/apic: repurpose the reporting of the APIC assist options



On 04/11/2022 16:01, Roger Pau Monné wrote:
On Fri, Nov 04, 2022 at 03:55:54PM +0000, Paul Durrant wrote:
On 04/11/2022 14:22, Roger Pau Monne wrote:
The current reporting of the hardware assisted APIC options is done by
checking "virtualize APIC accesses" which is not very helpful, as that
feature doesn't avoid a vmexit, instead it does provide some help in
order to detect APIC MMIO accesses in vmexit processing.

Repurpose the current reporting of xAPIC assistance to instead report
such feature as present when there's support for "TPR shadow" and
"APIC register virtualization" because in that case some xAPIC MMIO
register accesses are handled directly by the hardware, without
requiring a vmexit.

For symetry also change assisted x2APIC reporting to require
"virtualize x2APIC mode" and "APIC register virtualization", dropping
the option to also be reported when "virtual interrupt delivery" is
available.  Presence of the "virtual interrupt delivery" feature will
be reported using a different option.

Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
I find the logic in vmx_vlapic_msr_changed() hard to follow, but I
don't want to rewrite the function logic at this point.
---
   xen/arch/x86/hvm/viridian/viridian.c |  2 +-
   xen/arch/x86/hvm/vmx/vmcs.c          |  8 ++++----
   xen/arch/x86/hvm/vmx/vmx.c           | 25 ++++++++++++++++++-------
   xen/arch/x86/traps.c                 |  4 +---
   4 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/hvm/viridian/viridian.c 
b/xen/arch/x86/hvm/viridian/viridian.c
index c4fa0a8b32..bafd8e90de 100644
--- a/xen/arch/x86/hvm/viridian/viridian.c
+++ b/xen/arch/x86/hvm/viridian/viridian.c
@@ -201,7 +201,7 @@ void cpuid_viridian_leaves(const struct vcpu *v, uint32_t 
leaf,
            * Suggest x2APIC mode by default, unless xAPIC registers are 
hardware
            * virtualized and x2APIC ones aren't.
            */
-        if ( !cpu_has_vmx_apic_reg_virt || cpu_has_vmx_virtualize_x2apic_mode )
+        if ( !has_assisted_xapic(d) || has_assisted_x2apic(d) )

So, not sure why this is separated from patch 1 but stated this way it seems
counterintuitive. We only want to use the viridian MSRs if they are going to
be more efficient.. which I think is only in the case where we have neither
an x2apic not an assisted xapic (hence we would trap for MMIO).

I've read the MS HTLFS and I guess I got confused, the section about
this CPUID bit states:

"Bit 3: Recommend using MSRs for accessing APIC registers EOI, ICR and
TPR rather than their memory-mapped"

So I've (wrongly) understood that MSRs for accessing APIC registers
was meant to be a recommendation to use x2APIC mode in order to access
those registers.  Didn't realize Viridian had a way to expose certain
APIC registers using MSRs when the APIC is in xAPIC mode.


Yeah, I think they predate the existence of x2apic.

I withdraw patch 1 and adjust patch 2 accordingly then.

Cool. Thanks,

  Paul




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.