[PATCH v1 00/28] x86: initial SVM AVIC support

[PATCH v1 00/28] x86: initial SVM AVIC support

[dmukhin]

Overview
========

Modern AMD processors provide hardware-assisted LAPIC virtualization
support known as AVIC (Advanced Virtual Interrupt Controller). See
"15.29 Advanced Virtual Interrupt Controller" in [1].

The main objective is to improve interrupt delivery for a given VM
by avoiding expensive LAPIC emulation in hypervisor.

At a high-level, there are several types of interrupts needed to be
managed for a Xen domain:
  (a) IPIs
  (b) Posted interrupts (Xen-to-domain, such as event channels)
  (c) External devices (MSI, needs coordination with IOMMU)

This initial implementation addresses (a) and (b).

Current limitations:
- Only vLAPIC in xAPIC mode with AVIC enabled
- No GALog support

Please see [2] for more details on the scope.

This work is loosely based on previously submitted work [3] and [4],
including AVIC support in Linux KVM.

The feature is experimental and disabled in the default configuration.

Patch Series
============

Basically the entire work is divided into three big series:
 (A) Current series: initial SVM AVIC [5]
 (B) XTF interrupt latency test [6]
 (C) XTF infra in public CI [5]

For this AVIC series:

 Patches 1, 5, 6, 9, 11-16 introduce various cleanups and fixups.

 Patch 20 changes XEN_DOMCTL_debug_op implementation to allow
 more debug operations added (like enabling/disabling AVIC at
 run-time - used by the XTF test).

 Patches 2, 3, 4, 7, 8, 10, 17, 18-25 represent AVIC implementation
 including performance counters and run-time controls via debug
 hypercall.

 Patches 26-28 cover various documentation updates.

Testing
=======

Mainly, the patch series has been tested through public hardware runners.
Full CI run (including manually scheduled jobs) is available at [10].

Approach: I decided to write my own synthetic XTF [6] which runs as dom0
on a hardware target to capture some fine-grained metrics for IPIs and
posted interrupts (event channel).

System                                  | Test results
----------------------------------------+-------------
Zen2: Family 23 (0x17), Model 96 (0x60) | [7]
Zen3: Family 25 (0x19), Model 68 (0x44) | [8]
Zen5: Family 26 (0x1a), Model 36 (0x24) | [9]

Synthetic test:

    vCPU0                                     vCPU1
                                       executes some workload
 +-> t0
 |   |
 |   write ICR2 [*]
 r   write ICR
 e   |---------------- interrupt -----------> t0' = recv_tsc
 p   return from APIC emulation [*']          ISR
 e   |                                        t1' = handler_tsc
 a   t1
 t   |
 |   sync against ISR
 |   |
 +---+

First 1K samples is dropped, total number of samples is 20M.

IRQ delivery latency : t0' - t0
IRQ issue latency    : t1  - t0
IRQ handler latency  : t1' - t0'

[*]  For event channel that is EVTCHNOP_send hypercall.
[*'] For event channel that is return from the EVTCHNOP_send hypercall.

For all tested Zen2/Zen3/Zen5 targets there are overall improvements
w/ AVIC enabled: the test duration decreased, IPI delivery improved,
ISR latency improved.

Basically, AVIC optimizes long tails in the latency distribution leading
to more deterministic workload.

There's some peculiar behavior w.r.t. doorbell MSR which roughly takes 2us on
Zen5 - fallback to vcpu_kick() turned out to be faster. I added a run-time
configuration knob for a fallback, just in case.

There are also some outliers in the latency distributions which I haven't
ruled out yet: like look at max latency w/ and w/o AVIC in [7], [8], [9]: once
it a blue moon I've seen AVIC giving p99.999 values as a maximum (~15us vs
~45us for the entire 20M population).

Results for Zen5 target are more pronouced, so I am duplicating them below.

Zen5: Workload #1: IPI
----------------------

                    no AVIC                  |             AVIC
---------------------------------------------+------------------------------------
A: noaccel: IPI send latency (VCPU#0) (20000000 samples)
B: accel: IPI send latency (VCPU#0) (20000000 samples)
metric          cycles A              time A |        cycles B              time B
min                12240            6.131 us |             140               70 ns
avg                12955            6.489 us |             155               77 ns
p50       <        13000            6.512 us |  <          170               85 ns
p99       <        14000            7.013 us |  <          170               85 ns
p99.9     <        34000           17.031 us |  <          170               85 ns
p99.99    <        35000           17.532 us |  <          190               95 ns
p99.999   <        98000           49.092 us |  <        23000           11.521 us
max                99100           49.643 us |           86660           43.411 us
A: noaccel: IPI delivery latency (VCPU#0 -> VCPU#1) (20000000 samples)
B: accel: IPI delivery latency (VCPU#0 -> VCPU#1) (20000000 samples)
metric          cycles A              time A |        cycles B              time B
min                    0                0 ns |             620              310 ns
avg                 1402              702 ns |             626              313 ns
p50       <         1400              701 ns |  <          650              325 ns
p99       <         1500              751 ns |  <          700              350 ns
p99.9     <        20000           10.018 us |  <          700              350 ns
p99.99    <        20000           10.018 us |  <          700              350 ns
p99.999   <        71000           35.566 us |  <        23000           11.521 us
max                87740           43.952 us |           87000           43.581 us
A: noaccel: IPI handler latency (VCPU#1) (20000000 samples)
B: accel: IPI handler latency (VCPU#1) (20000000 samples)
metric          cycles A              time A |        cycles B              time B
min                15520            7.774 us |             160               80 ns
avg                16256            8.143 us |             174               87 ns
p50       <        17000            8.515 us |  <          190               95 ns
p99       <        17000            8.515 us |  <          190               95 ns
p99.9     <        17000            8.515 us |  <          190               95 ns
p99.99    <        17000            8.515 us |  <          190               95 ns
p99.999   <        23000           11.521 us |  <          190               95 ns
max                33880           16.971 us |             200              100 ns

Workload time                      182552 ms                               9919 ms
(20000000 samples)                                                    (~18x boost)

Zen5: Workload #2: evtchn
-------------------------

                    no AVIC                  |             AVIC
---------------------------------------------+------------------------------------
A: noaccel: evtchn send latency (VCPU#0) (20000000 samples)
B: accel: evtchn send latency (VCPU#0) (20000000 samples)
metric          cycles A              time A |        cycles B              time B
min                 5800            2.905 us |            6020            3.015 us
avg                 5988            2.999 us |            6204            3.107 us
p50       <         6000            3.005 us |  <         6200            3.105 us
p99       <         6100            3.055 us |  <         6200            3.105 us
p99.9     <        27000           13.525 us |  <        27000           13.525 us
p99.99    <        28000           14.026 us |  <        29000           14.527 us
p99.999   <        28000           14.026 us |  <        92000           46.086 us
max                92160           46.166 us |           92680           46.427 us
A: noaccel: evtchn delivery latency (VCPU#0 -> VCPU#1) (20000000 samples)
B: accel: evtchn delivery latency (VCPU#0 -> VCPU#1) (20000000 samples)
metric          cycles A              time A |        cycles B              time B
min                    0                0 ns |            5440            2.725 us
avg                 1412              707 ns |            5497            2.753 us
p50       <         1400              701 ns |  <         5600            2.805 us
p99       <         1500              751 ns |  <         5600            2.805 us
p99.9     <        19000            9.517 us |  <         6000            3.005 us
p99.99    <        19000            9.517 us |  <         6200            3.105 us
p99.999   <        71000           35.566 us |  <        28000           14.026 us
max                87600           43.882 us |           91760           45.966 us
A: noaccel: evtchn handler latency (VCPU#1) (20000000 samples)
B: accel: evtchn handler latency (VCPU#1) (20000000 samples)
metric          cycles A              time A |        cycles B              time B
min                15540            7.784 us |             180               90 ns
avg                15803            7.916 us |             184               92 ns
p50       <        16000            8.015 us |  <          190               95 ns
p99       <        16000            8.015 us |  <          210              105 ns
p99.9     <        16000            8.015 us |  <          210              105 ns
p99.99    <        16000            8.015 us |  <          210              105 ns
p99.999   <        24000           12.022 us |  <          210              105 ns
max                33120           16.591 us |             200              100 ns

metric          cycles A              time A |        cycles B              time B
min                    0                0 ns |            5400            2.705 us
avg                 1425              713 ns |            5467            2.738 us
p50       <         1500              751 ns |  <         5500            2.755 us
p99       <         1500              751 ns |  <         5600            2.805 us
p99.9     <        19000            9.517 us |  <         6000            3.005 us
p99.99    <        19000            9.517 us |  <         6200            3.105 us
p99.999   <        71000           35.566 us |  <        28000           14.026 us
max                87800           43.981 us |           91940           46.055 us

Workload time                      175951 ms                              62951 ms
(20000000 samples)                                                   (~2.5x boost)

Thanks a lot to Marek for hooking Zen5 runner to public CI.

Thanks a lot to Stefano and Andrew with advices and fixing the access to the
fleet of hardware runners.

References
==========

[1] AMD APM Volumes 1-5:
    https://docs.amd.com/v/u/en-US/40332_4.11_APM_Vol1-5
[2] Xen Summit 2026 SVM AVIC:
  - YouTube: https://www.youtube.com/watch?v=RhaJsOU_PIA&list=PLGe6RWOETgTQ
  - Slides: https://hosted-files.sched.co/xensummit2026/05/dmukhin_2026_x86_SVM_AVIC.pdf?_gl=1*158gk36*_gcl_au*MjU3OTQwMDQ2LjE3ODg0NzE3NDQ.*FPAU*MjU3OTQwMDQ2LjE3ODg0NzE3NDQ
[3] Archive v1:
    https://lore.kernel.org/xen-devel/1474264368-4104-1-git-send-email-suravee.suthikulpanit@amd.com/
[4] Archive v2:
    https://lore.kernel.org/xen-devel/20180507210753.2280-1-Janakarajan.Natarajan@amd.com/
[5] Current patch series:
    https://gitlab.com/xen-project/people/dmukhin/xen/-/commits/svm-avic-v3
[6] XTF interrupt latency test:
    https://gitlab.com/xen-project/people/dmukhin/xtf/-/tree/x86_64-ipi-latency?ref_type=heads
[7] Zen2: Family 23 (0x17), Model 96 (0x60): interrupt latency results:
    https://gist.github.com/dmkhn/289f1af656cce74c0fbc81642727a2a4
[8] Zen3: Family 25 (0x19), Model 68 (0x44): interrupt latency results:
    https://gist.github.com/dmkhn/9192105efc49cfedd15e2820b8ad23fd
[9] Zen5: Family 26 (0x1a), Model 36 (0x24): interrupt latency results:
    https://gist.github.com/dmkhn/e9f50b43cbc48b82175ec517d6cfe6f5
[10] CI:
    https://gitlab.com/xen-project/people/dmukhin/xen/-/pipelines/3025987819

Denis Mukhin (28):
  x86/hvm: move SVM feature printout to a separate function
  x86/hvm: report SVM AVIC support
  x86/hvm: report SVM x2AVIC support
  x86/hvm: avic: add domain state initialization
  x86/hvm: cleanup svm_create_vmcb()
  x86/hvm: introduce vlapic_reg_read()
  x86/hvm: avic: add VMCB state initialization
  x86/hvm: avic: add AVIC_INCOMP_IPI VMEXIT handler
  x86/hvm: fix hvm_emulate_one_vm_event() definition
  x86/hvm: avic: add AVIC_NOACCEL VMEXIT handler
  x86/hvm: introduce vcpu_is_vlapic_tpr_emulated()
  x86: apic: use APIC_EIO_ACK instead of open-coded 0
  x86/time: fix typo s/interruopt/interrupt/
  x86/hvm: vlapic: update signatures of both vlapic_{clear,test}_*
  x86/hvm: vlapic: add checks before posted interrupt delivery
  x86/hvm: move VINTR disable to svm_clear_intr_window()
  x86/hvm: avic: route interrupt delivery to vCPU
  x86/hvm: add facility to print hardware-assisted vLAPIC info
  x86/hvm: avic: enable SVM AVIC statistics reporting
  x86/hvm: rework XEN_DOMCTL_debug_op
  x86/hvm: enable debug hypercall for posted interrupts
  x86/hvm: avic: ensure SVM AVIC supports xAPIC operation only
  x86/hvm: avic: enable vLAPIC mode switch hook
  x86/hvm: avic: ensure SVM AVIC is disabled with nested guest
  x86/hvm: avic: account for AMD erratum 1235
  docs: add coverage for svm_avic=
  CHANGELOG.md: add note on initial SVM AVIC support
  MAINTAINERS: add myself as SVM AVIC reviewer

 CHANGELOG.md                             |   3 +
 MAINTAINERS                              |  11 +
 docs/misc/xen-command-line.pandoc        |  23 +-
 xen/arch/x86/cpu-policy.c                |   3 +-
 xen/arch/x86/domctl.c                    |  24 +-
 xen/arch/x86/hvm/emulate.c               |   5 +
 xen/arch/x86/hvm/hvm.c                   |  74 +-
 xen/arch/x86/hvm/svm/Makefile            |   1 +
 xen/arch/x86/hvm/svm/avic.c              | 921 +++++++++++++++++++++++
 xen/arch/x86/hvm/svm/avic.h              |  41 +
 xen/arch/x86/hvm/svm/intr.c              |  22 +
 xen/arch/x86/hvm/svm/nestedsvm.c         |   9 +-
 xen/arch/x86/hvm/svm/svm.c               | 186 ++++-
 xen/arch/x86/hvm/svm/svm.h               |   2 +
 xen/arch/x86/hvm/svm/vmcb.c              |  29 +-
 xen/arch/x86/hvm/svm/vmcb.h              |  27 +-
 xen/arch/x86/hvm/vlapic.c                |  25 +-
 xen/arch/x86/hvm/vmx/vmx.c               |   4 +
 xen/arch/x86/include/asm/apic.h          |   3 +-
 xen/arch/x86/include/asm/apicdef.h       |   2 +-
 xen/arch/x86/include/asm/hvm/hvm.h       |  47 +-
 xen/arch/x86/include/asm/hvm/svm-types.h |  31 +
 xen/arch/x86/include/asm/hvm/svm.h       |   4 +
 xen/arch/x86/include/asm/hvm/vlapic.h    |   1 +
 xen/arch/x86/include/asm/msr-index.h     |   1 +
 xen/arch/x86/irq.c                       |  24 +
 xen/arch/x86/time.c                      |   2 +-
 xen/include/public/domctl.h              |   2 +
 28 files changed, 1411 insertions(+), 116 deletions(-)
 create mode 100644 xen/arch/x86/hvm/svm/avic.c
 create mode 100644 xen/arch/x86/hvm/svm/avic.h

-- 
2.55.0

[PATCH v1 01/28] x86/hvm: move SVM feature printout to a separate function

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

Isolate long SVM feature list printout in a separate function to
improve readability.

Not a functional change.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/hvm/svm/svm.c | 49 +++++++++++++++++++++-----------------
 1 file changed, 27 insertions(+), 22 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 666bcd3eb483..e11d5c55248d 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2434,33 +2434,12 @@ static struct hvm_function_table __initdata_cf_clobber svm_function_table = {
     },
 };
 
-const struct hvm_function_table * __init start_svm(void)
+static void __init print_svm_features(void)
 {
     bool printed = false;
 
-    svm_host_osvw_reset();
-
-    if ( _svm_cpu_up(true) )
-    {
-        setup_clear_cpu_cap(X86_FEATURE_SVM);
-        printk("SVM: failed to initialise.\n");
-        return NULL;
-    }
-
-    setup_vmcb_dump();
-
-    if ( boot_cpu_data.extended_cpuid_level >= 0x8000000aU )
-        svm_feature_flags = cpuid_edx(0x8000000aU);
-
     printk("SVM: Supported advanced features:\n");
 
-    /* DecodeAssists fast paths assume nextrip is valid for fast rIP update. */
-    if ( !cpu_has_svm_nrips )
-        __clear_bit(SVM_FEATURE_DECODEASSISTS, &svm_feature_flags);
-
-    if ( cpu_has_tsc_ratio )
-        svm_function_table.tsc_scaling.ratio_frac_bits = 32;
-
 #define P(p,s) if ( p ) { printk(" - %s\n", s); printed = 1; }
     P(cpu_has_svm_npt, "Nested Page Tables (NPT)");
     P(cpu_has_svm_lbrv, "Last Branch Record (LBR) Virtualisation");
@@ -2481,11 +2460,37 @@ const struct hvm_function_table * __init start_svm(void)
 
     if ( !printed )
         printk(" - none\n");
+}
+
+const struct hvm_function_table * __init start_svm(void)
+{
+    svm_host_osvw_reset();
+
+    if ( _svm_cpu_up(true) )
+    {
+        setup_clear_cpu_cap(X86_FEATURE_SVM);
+        printk("SVM: failed to initialise.\n");
+        return NULL;
+    }
+
+    setup_vmcb_dump();
+
+    if ( boot_cpu_data.extended_cpuid_level >= 0x8000000aU )
+        svm_feature_flags = cpuid_edx(0x8000000aU);
+
+    /* DecodeAssists fast paths assume nextrip is valid for fast rIP update. */
+    if ( !cpu_has_svm_nrips )
+        __clear_bit(SVM_FEATURE_DECODEASSISTS, &svm_feature_flags);
+
+    if ( cpu_has_tsc_ratio )
+        svm_function_table.tsc_scaling.ratio_frac_bits = 32;
 
     svm_function_table.caps.hap = cpu_has_svm_npt;
     svm_function_table.caps.hap_superpage_2mb = true;
     svm_function_table.caps.hap_superpage_1gb = cpu_has_page1gb;
 
+    print_svm_features();
+
     return &svm_function_table;
 }
 
-- 
2.55.0

[PATCH v1 02/28] x86/hvm: report SVM AVIC support

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

Report SVM AVIC support from CPUID Fn8000000A_EDX[AVIC](bit 13).

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/hvm/svm/svm.c         | 1 +
 xen/arch/x86/include/asm/hvm/svm.h | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index e11d5c55248d..cf008a8acdf2 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2456,6 +2456,7 @@ static void __init print_svm_features(void)
     P(cpu_has_svm_spec_ctrl, "MSR_SPEC_CTRL virtualisation");
     P(cpu_has_svm_vnmi, "Virtual NMI");
     P(cpu_has_svm_bus_lock, "Bus Lock Filter");
+    P(cpu_has_svm_avic, "Advanced Virtual Interrupt Controller");
 #undef P
 
     if ( !printed )
diff --git a/xen/arch/x86/include/asm/hvm/svm.h b/xen/arch/x86/include/asm/hvm/svm.h
index a35a61273baa..0fc33f41b09d 100644
--- a/xen/arch/x86/include/asm/hvm/svm.h
+++ b/xen/arch/x86/include/asm/hvm/svm.h
@@ -33,6 +33,7 @@ extern u32 svm_feature_flags;
 #define SVM_FEATURE_DECODEASSISTS  7 /* Decode assists support */
 #define SVM_FEATURE_PAUSEFILTER   10 /* Pause intercept filter support */
 #define SVM_FEATURE_PAUSETHRESH   12 /* Pause intercept filter support */
+#define SVM_FEATURE_AVIC          13 /* Advanced Virtual Interrupt Controller */
 #define SVM_FEATURE_VLOADSAVE     15 /* virtual vmload/vmsave */
 #define SVM_FEATURE_VGIF          16 /* Virtual GIF */
 #define SVM_FEATURE_SSS           19 /* NPT Supervisor Shadow Stacks */
@@ -55,6 +56,7 @@ static inline bool cpu_has_svm_feature(unsigned int feat)
 #define cpu_has_pause_filter  cpu_has_svm_feature(SVM_FEATURE_PAUSEFILTER)
 #define cpu_has_pause_thresh  cpu_has_svm_feature(SVM_FEATURE_PAUSETHRESH)
 #define cpu_has_tsc_ratio     cpu_has_svm_feature(SVM_FEATURE_TSCRATEMSR)
+#define cpu_has_svm_avic      cpu_has_svm_feature(SVM_FEATURE_AVIC)
 #define cpu_has_svm_vloadsave cpu_has_svm_feature(SVM_FEATURE_VLOADSAVE)
 #define cpu_has_svm_sss       cpu_has_svm_feature(SVM_FEATURE_SSS)
 #define cpu_has_svm_spec_ctrl cpu_has_svm_feature(SVM_FEATURE_SPEC_CTRL)
-- 
2.55.0

[PATCH v1 03/28] x86/hvm: report SVM x2AVIC support

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

Report SVM AVIC support from CPUID Fn8000000A_EDX[X2AVIC](bit 18).

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/hvm/svm/svm.c         | 1 +
 xen/arch/x86/include/asm/hvm/svm.h | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index cf008a8acdf2..db81dad4b681 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2457,6 +2457,7 @@ static void __init print_svm_features(void)
     P(cpu_has_svm_vnmi, "Virtual NMI");
     P(cpu_has_svm_bus_lock, "Bus Lock Filter");
     P(cpu_has_svm_avic, "Advanced Virtual Interrupt Controller");
+    P(cpu_has_svm_x2avic, "x2 Advanced Virtual Interrupt Controller");
 #undef P
 
     if ( !printed )
diff --git a/xen/arch/x86/include/asm/hvm/svm.h b/xen/arch/x86/include/asm/hvm/svm.h
index 0fc33f41b09d..292a9ee19b11 100644
--- a/xen/arch/x86/include/asm/hvm/svm.h
+++ b/xen/arch/x86/include/asm/hvm/svm.h
@@ -36,6 +36,7 @@ extern u32 svm_feature_flags;
 #define SVM_FEATURE_AVIC          13 /* Advanced Virtual Interrupt Controller */
 #define SVM_FEATURE_VLOADSAVE     15 /* virtual vmload/vmsave */
 #define SVM_FEATURE_VGIF          16 /* Virtual GIF */
+#define SVM_FEATURE_X2AVIC        18 /* x2 Advanced Virtual Interrupt Controller */
 #define SVM_FEATURE_SSS           19 /* NPT Supervisor Shadow Stacks */
 #define SVM_FEATURE_SPEC_CTRL     20 /* MSR_SPEC_CTRL virtualisation */
 #define SVM_FEATURE_VNMI          25 /* Virtual NMI */
@@ -57,6 +58,7 @@ static inline bool cpu_has_svm_feature(unsigned int feat)
 #define cpu_has_pause_thresh  cpu_has_svm_feature(SVM_FEATURE_PAUSETHRESH)
 #define cpu_has_tsc_ratio     cpu_has_svm_feature(SVM_FEATURE_TSCRATEMSR)
 #define cpu_has_svm_avic      cpu_has_svm_feature(SVM_FEATURE_AVIC)
+#define cpu_has_svm_x2avic    cpu_has_svm_feature(SVM_FEATURE_X2AVIC)
 #define cpu_has_svm_vloadsave cpu_has_svm_feature(SVM_FEATURE_VLOADSAVE)
 #define cpu_has_svm_sss       cpu_has_svm_feature(SVM_FEATURE_SSS)
 #define cpu_has_svm_spec_ctrl cpu_has_svm_feature(SVM_FEATURE_SPEC_CTRL)
-- 
2.55.0

[PATCH v1 04/28] x86/hvm: avic: add domain state initialization

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

Establish per-domain SVM AVIC backing page mapping and SPA mappings
for APIC physical/logical ID tables. Use MEMF_no_refcount mappings
similarly to Intel's VMX APICv setup.

Gate per-domain AVIC state initialization behind 'svm_avic=' command
line option.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/hvm/svm/Makefile            |   1 +
 xen/arch/x86/hvm/svm/avic.c              | 136 +++++++++++++++++++++++
 xen/arch/x86/hvm/svm/avic.h              |  14 +++
 xen/arch/x86/hvm/svm/svm.c               |  20 ++++
 xen/arch/x86/include/asm/hvm/svm-types.h |   8 ++
 5 files changed, 179 insertions(+)
 create mode 100644 xen/arch/x86/hvm/svm/avic.c
 create mode 100644 xen/arch/x86/hvm/svm/avic.h

diff --git a/xen/arch/x86/hvm/svm/Makefile b/xen/arch/x86/hvm/svm/Makefile
index 8a072cafd572..7dd16e32a77d 100644
--- a/xen/arch/x86/hvm/svm/Makefile
+++ b/xen/arch/x86/hvm/svm/Makefile
@@ -1,4 +1,5 @@
 obj-y += asid.o
+obj-y += avic.o
 obj-y += emulate.o
 obj-bin-y += entry.o
 obj-y += intr.o
diff --git a/xen/arch/x86/hvm/svm/avic.c b/xen/arch/x86/hvm/svm/avic.c
new file mode 100644
index 000000000000..2c0e113ae34e
--- /dev/null
+++ b/xen/arch/x86/hvm/svm/avic.c
@@ -0,0 +1,136 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * AMD Advanced Virtual Interrupt Controller (AVIC)
+ */
+
+#include <asm/hvm/emulate.h>
+#include <asm/msr.h>
+#include <asm/p2m.h>
+#include <xen/param.h>
+
+#include "avic.h"
+
+bool __read_mostly svm_avic_enable;
+
+static const char __section(".bss.page_aligned.const") __aligned(PAGE_SIZE)
+    avic_apic_access_page[PAGE_SIZE];
+
+static int cf_check avic_parse_cmdline(const char *s)
+{
+    int rc;
+
+    rc = parse_bool(s, NULL);
+    if ( rc != -1 )
+    {
+        svm_avic_enable = !!rc;
+    }
+    else if ( *s == '\0' )
+    {
+        svm_avic_enable = true;
+    }
+    else
+    {
+        return -EINVAL;
+    }
+
+    return 0;
+}
+custom_param("svm_avic", avic_parse_cmdline);
+
+/*
+ * Enable guest accesses to APIC_DEFAULT_PHYS_BASE.
+ *
+ * AVIC walks the nested page table to check access permissions, but
+ * ignores the SPA from the leaf entry and instead uses the address in
+ * VMCB.avic_backing_page.
+ *
+ * Install a mapping so APIC accesses don't trap to Xen while xAPIC is
+ * accelerated.
+ */
+static int avic_set_mmio_p2m_entry(struct domain *d)
+{
+    gfn_t gfn = _gfn(paddr_to_pfn(APIC_DEFAULT_PHYS_BASE));
+    mfn_t mfn = _mfn(virt_to_mfn(avic_apic_access_page));
+
+    return map_mmio_regions(d, gfn, 1, mfn);
+}
+
+static int avic_clear_mmio_p2m_entry(struct domain *d)
+{
+    gfn_t gfn = _gfn(paddr_to_pfn(APIC_DEFAULT_PHYS_BASE));
+    mfn_t mfn = _mfn(virt_to_mfn(avic_apic_access_page));
+
+    return unmap_mmio_regions(d, gfn, 1, mfn);
+}
+
+int svm_avic_domain_initialise(struct domain *d)
+{
+    struct svm_domain *sd = &d->arch.hvm.svm;
+    struct page_info *pg;
+    mfn_t mfn;
+
+    if ( !svm_avic_enable || !has_vlapic(d) )
+        return 0;
+
+    /* AVIC logical APIC ID table */
+    pg = alloc_domheap_page(d, MEMF_no_refcount);
+    if ( !pg )
+        return -ENOMEM;
+
+    mfn = page_to_mfn(pg);
+    clear_domain_page(mfn);
+
+    sd->avic_logical_id_table_mfn = mfn;
+    sd->avic_logical_id_table = map_domain_page_global(mfn);
+
+    /* AVIC physical APIC ID table */
+    pg = alloc_domheap_page(d, MEMF_no_refcount);
+    if ( !pg )
+        return -ENOMEM;
+
+    mfn = page_to_mfn(pg);
+    clear_domain_page(mfn);
+
+    sd->avic_physical_id_table_mfn = mfn;
+    sd->avic_physical_id_table = map_domain_page_global(mfn);
+
+    return avic_set_mmio_p2m_entry(d);
+}
+
+void svm_avic_domain_relinquish_resources(struct domain *d)
+{
+    struct svm_domain *sd = &d->arch.hvm.svm;
+    mfn_t mfn;
+
+    (void)avic_clear_mmio_p2m_entry(d);
+
+    mfn = sd->avic_physical_id_table_mfn;
+    if ( !mfn_eq(mfn, _mfn(0)) )
+    {
+        unmap_domain_page_global(sd->avic_physical_id_table);
+        sd->avic_physical_id_table = NULL;
+
+        free_domheap_page(mfn_to_page(mfn));
+        sd->avic_physical_id_table_mfn = _mfn(0);
+    }
+
+    mfn = sd->avic_logical_id_table_mfn;
+    if ( !mfn_eq(mfn, _mfn(0)) )
+    {
+        unmap_domain_page_global(sd->avic_logical_id_table);
+        sd->avic_logical_id_table = NULL;
+
+        free_domheap_page(mfn_to_page(mfn));
+        sd->avic_logical_id_table_mfn = _mfn(0);
+    }
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/x86/hvm/svm/avic.h b/xen/arch/x86/hvm/svm/avic.h
new file mode 100644
index 000000000000..3da62f59b0cf
--- /dev/null
+++ b/xen/arch/x86/hvm/svm/avic.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * AMD Advanced Virtual Interrupt Controller (AVIC)
+ */
+
+#ifndef __SVM_AVIC_H__
+#define __SVM_AVIC_H__
+
+extern bool svm_avic_enable;
+
+int svm_avic_domain_initialise(struct domain *d);
+void svm_avic_domain_relinquish_resources(struct domain *d);
+
+#endif /* __SVM_AVIC_H__ */
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index db81dad4b681..8b7bfed0049f 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -40,6 +40,7 @@
 
 #include <public/sched.h>
 
+#include "avic.h"
 #include "nestedhvm.h"
 #include "svm.h"
 #include "vmcb.h"
@@ -1084,6 +1085,11 @@ static int cf_check svm_domain_initialise(struct domain *d)
         .to   = svm_ctxt_switch_to,
         .tail = svm_do_resume,
     };
+    int rc;
+
+    rc = svm_avic_domain_initialise(d);
+    if ( rc )
+        return rc;
 
     d->arch.ctxt_switch = &csw;
 
@@ -1095,6 +1101,11 @@ static int cf_check svm_domain_initialise(struct domain *d)
     return 0;
 }
 
+static void cf_check svm_domain_relinquish_resources(struct domain *d)
+{
+    svm_avic_domain_relinquish_resources(d);
+}
+
 static int cf_check svm_vcpu_initialise(struct vcpu *v)
 {
     int rc;
@@ -2384,6 +2395,7 @@ static struct hvm_function_table __initdata_cf_clobber svm_function_table = {
     .cpu_up               = svm_cpu_up,
     .cpu_down             = svm_cpu_down,
     .domain_initialise    = svm_domain_initialise,
+    .domain_relinquish_resources = svm_domain_relinquish_resources,
     .vcpu_initialise      = svm_vcpu_initialise,
     .vcpu_destroy         = svm_vcpu_destroy,
     .save_cpu_ctxt        = svm_save_vmcb_ctxt,
@@ -2464,6 +2476,12 @@ static void __init print_svm_features(void)
         printk(" - none\n");
 }
 
+static void __init svm_avic_start(void)
+{
+    if ( !cpu_has_svm_avic || pv_shim )
+        svm_avic_enable = false;
+}
+
 const struct hvm_function_table * __init start_svm(void)
 {
     svm_host_osvw_reset();
@@ -2487,6 +2505,8 @@ const struct hvm_function_table * __init start_svm(void)
     if ( cpu_has_tsc_ratio )
         svm_function_table.tsc_scaling.ratio_frac_bits = 32;
 
+    svm_avic_start();
+
     svm_function_table.caps.hap = cpu_has_svm_npt;
     svm_function_table.caps.hap_superpage_2mb = true;
     svm_function_table.caps.hap_superpage_1gb = cpu_has_page1gb;
diff --git a/xen/arch/x86/include/asm/hvm/svm-types.h b/xen/arch/x86/include/asm/hvm/svm-types.h
index 71427b3c11d1..30bdaf27a856 100644
--- a/xen/arch/x86/include/asm/hvm/svm-types.h
+++ b/xen/arch/x86/include/asm/hvm/svm-types.h
@@ -15,6 +15,14 @@ struct svm_domain {
             uint64_t status;
         };
     } osvw;
+
+    /*
+     * Domain AVIC state.
+     */
+    mfn_t avic_logical_id_table_mfn;
+    uint32_t *avic_logical_id_table;
+    mfn_t avic_physical_id_table_mfn;
+    uint64_t *avic_physical_id_table;
 };
 
 struct svm_vcpu {
-- 
2.55.0

[PATCH v1 05/28] x86/hvm: cleanup svm_create_vmcb()

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

svm_create_vmcb() does not need to be idempotent because in case of
failure caller must call svm_destroy_vmcb().

Simplify code in svm_create_vmcb() to account for that.

Also, fix MISRA violations in double assignment to vmcb_pa and
nv_n1vmcx_pa.

Not a functional change.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/hvm/svm/vmcb.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
index 33c85ea3bfc5..3ec5bdddf588 100644
--- a/xen/arch/x86/hvm/svm/vmcb.c
+++ b/xen/arch/x86/hvm/svm/vmcb.c
@@ -201,26 +201,24 @@ int svm_create_vmcb(struct vcpu *v)
 {
     struct nestedvcpu *nv = &vcpu_nestedhvm(v);
     struct svm_vcpu *svm = &v->arch.hvm.svm;
+    paddr_t paddr;
     int rc;
 
-    if ( (nv->nv_n1vmcx == NULL) &&
-         (nv->nv_n1vmcx = alloc_vmcb()) == NULL )
-    {
-        printk("Failed to create a new VMCB\n");
+    ASSERT(nv->nv_n1vmcx == NULL);
+
+    nv->nv_n1vmcx = alloc_vmcb();
+    if ( !nv->nv_n1vmcx )
         return -ENOMEM;
-    }
 
     svm->vmcb = nv->nv_n1vmcx;
     rc = construct_vmcb(v);
-    if ( rc != 0 )
-    {
-        free_vmcb(nv->nv_n1vmcx);
-        nv->nv_n1vmcx = NULL;
-        svm->vmcb = NULL;
+    if ( rc )
         return rc;
-    }
 
-    svm->vmcb_pa = nv->nv_n1vmcx_pa = virt_to_maddr(svm->vmcb);
+    paddr = virt_to_maddr(nv->nv_n1vmcx);
+    nv->nv_n1vmcx_pa = paddr;
+    svm->vmcb_pa = paddr;
+
     return 0;
 }
 
-- 
2.55.0

[PATCH v1 06/28] x86/hvm: introduce vlapic_reg_read()

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

Add vlapic_reg_read() for integration with components relying
on emulated LAPIC accesses.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/hvm/vlapic.c             | 5 +++++
 xen/arch/x86/include/asm/hvm/vlapic.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index 6698779e227c..f929fdfd74ee 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -618,6 +618,11 @@ static uint32_t vlapic_read_aligned(const struct vlapic *vlapic,
     return 0;
 }
 
+uint32_t vlapic_reg_read(const struct vcpu *v, unsigned int reg)
+{
+    return vlapic_read_aligned(vcpu_vlapic(v), reg);
+}
+
 static int cf_check vlapic_mmio_read(
     struct vcpu *v, unsigned long address, unsigned int len,
     unsigned long *pval)
diff --git a/xen/arch/x86/include/asm/hvm/vlapic.h b/xen/arch/x86/include/asm/hvm/vlapic.h
index f442dec3d975..95f6dfe9e941 100644
--- a/xen/arch/x86/include/asm/hvm/vlapic.h
+++ b/xen/arch/x86/include/asm/hvm/vlapic.h
@@ -97,6 +97,7 @@ static inline void vlapic_set_reg(
 }
 
 void vlapic_reg_write(struct vcpu *v, unsigned int reg, uint32_t val);
+uint32_t vlapic_reg_read(const struct vcpu *v, unsigned int reg);
 
 bool is_vlapic_lvtpc_enabled(struct vlapic *vlapic);
 
-- 
2.55.0

[PATCH v1 07/28] x86/hvm: avic: add VMCB state initialization

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

Update SVM VMCB with AVIC-related data types as per [1].

Install AVIC backing page and logical/physical APIC ID tables mappings
into VMCB.

Set VMCB 'avic_enable' bit to 1 in case of successful initialization.

[1] AMD APM Volumes 1-5 "15.29 Advanced Virtual Interrupt Controller".
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/hvm/svm/avic.c | 110 ++++++++++++++++++++++++++++++++++++
 xen/arch/x86/hvm/svm/avic.h |   3 +
 xen/arch/x86/hvm/svm/vmcb.c |   7 +++
 xen/arch/x86/hvm/svm/vmcb.h |  19 +++++--
 4 files changed, 135 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/avic.c b/xen/arch/x86/hvm/svm/avic.c
index 2c0e113ae34e..bc98661a7057 100644
--- a/xen/arch/x86/hvm/svm/avic.c
+++ b/xen/arch/x86/hvm/svm/avic.c
@@ -9,6 +9,16 @@
 #include <xen/param.h>
 
 #include "avic.h"
+#include "vmcb.h"
+
+#define AVIC_PHYSICAL_ID_ENTRY_BACKING_PAGE_MASK        GENMASK_ULL(51, 12U)
+#define AVIC_PHYSICAL_ID_TABLE_SIZE_MASK                (0xFFULL)
+
+/*
+ * For AVIC, the max index allowed for physical APIC ID table is 0xfe (254), as
+ * 0xff is a broadcast to all CPUs, i.e. can't be targeted individually.
+ */
+#define AVIC_MAX_PHYSICAL_ID                            (0xFEU)
 
 bool __read_mostly svm_avic_enable;
 
@@ -125,6 +135,106 @@ void svm_avic_domain_relinquish_resources(struct domain *d)
     }
 }
 
+static bool avic_vcpu_supported(struct vcpu *v)
+{
+    struct domain *d = v->domain;
+    unsigned int id;
+
+    if ( !svm_avic_enable || !has_vlapic(d) )
+        return false;
+
+    if ( d->max_vcpus * 2 >= AVIC_PHYSICAL_ID_TABLE_SIZE_MASK )
+    {
+        gprintk(XENLOG_WARNING,
+                "%pv: AVIC: unsupported domain max_vcpus %u\n",
+                v, d->max_vcpus);
+        return false;
+    }
+
+    id = GET_xAPIC_ID(vlapic_reg_read(v, APIC_ID));
+    if ( id > AVIC_MAX_PHYSICAL_ID )
+    {
+        gprintk(XENLOG_WARNING,
+                "%pv: AVIC: unsupported APIC ID %u\n",
+                v, id);
+        return false;
+    }
+
+    return true;
+}
+
+static void avic_vcpu_enable(const struct vcpu *v, bool enable)
+{
+    const struct svm_vcpu *sv = &v->arch.hvm.svm;
+    struct vmcb_struct *vmcb = sv->vmcb;
+
+    vmcb->_vintr.fields.x2avic_enable = 0;
+    vmcb->_vintr.fields.avic_enable = enable;
+    vmcb->cleanbits.avic = 0;
+
+    printk(XENLOG_INFO
+           "%pv: AVIC: %s: apic_id=%#x avic_physical_id=%#"PRIx64" max_vcpus=%u\n",
+           v,
+           enable ? "enable" : "disable",
+           GET_xAPIC_ID(vlapic_reg_read(v, APIC_ID)),
+           sv->avic_physical_id ? ACCESS_ONCE(*sv->avic_physical_id) :
+                                  0xDEADDEADUL,
+           v->domain->max_vcpus);
+}
+
+int svm_avic_vmcb_create(struct vcpu *v)
+{
+    struct domain *d = v->domain;
+    struct svm_domain *sd = &d->arch.hvm.svm;
+    struct svm_vcpu *sv = &v->arch.hvm.svm;
+    struct vmcb_struct *vmcb = sv->vmcb;
+    const struct vlapic *vlapic = vcpu_vlapic(v);
+    mfn_t mfn;
+    uint64_t entry;
+    unsigned int id;
+
+    if ( !avic_vcpu_supported(v) )
+        return 0;
+
+    vmcb->avic_vapic_bar = APIC_DEFAULT_PHYS_BASE;
+    vmcb->avic_backing_page = page_to_maddr(vlapic->regs_page);
+
+    mfn = domain_page_map_to_mfn(sd->avic_logical_id_table);
+    vmcb->avic_logical_id_table = mfn_to_maddr(mfn);
+
+    mfn = domain_page_map_to_mfn(sd->avic_physical_id_table);
+    vmcb->avic_physical_id_table = mfn_to_maddr(mfn);
+    vmcb->avic_physical_id_table |= (d->max_vcpus * 2) &
+                                    AVIC_PHYSICAL_ID_TABLE_SIZE_MASK;
+
+    id = GET_xAPIC_ID(vlapic_reg_read(v, APIC_ID));
+
+    sv->avic_physical_id = &sd->avic_physical_id_table[id];
+
+    entry = vmcb->avic_backing_page & AVIC_PHYSICAL_ID_ENTRY_BACKING_PAGE_MASK;
+    entry |= AVIC_PHYSICAL_ID_ENTRY_VALID_MASK;
+
+    ACCESS_ONCE(*sv->avic_physical_id) = entry;
+
+    avic_vcpu_enable(v, true);
+
+    return 0;
+}
+
+void svm_avic_vmcb_destroy(struct vcpu *v)
+{
+    struct svm_vcpu *sv = &v->arch.hvm.svm;
+    struct vmcb_struct *vmcb = sv->vmcb;
+
+    if ( svm_avic_vcpu_enabled(v) )
+        avic_vcpu_enable(v, false);
+
+    vmcb->avic_physical_id_table = 0;
+    vmcb->avic_logical_id_table = 0;
+    vmcb->avic_backing_page = 0;
+    vmcb->avic_vapic_bar = 0;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/hvm/svm/avic.h b/xen/arch/x86/hvm/svm/avic.h
index 3da62f59b0cf..1084ae683944 100644
--- a/xen/arch/x86/hvm/svm/avic.h
+++ b/xen/arch/x86/hvm/svm/avic.h
@@ -11,4 +11,7 @@ extern bool svm_avic_enable;
 int svm_avic_domain_initialise(struct domain *d);
 void svm_avic_domain_relinquish_resources(struct domain *d);
 
+int svm_avic_vmcb_create(struct vcpu *v);
+void svm_avic_vmcb_destroy(struct vcpu *v);
+
 #endif /* __SVM_AVIC_H__ */
diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
index 3ec5bdddf588..c4282bfd6c6e 100644
--- a/xen/arch/x86/hvm/svm/vmcb.c
+++ b/xen/arch/x86/hvm/svm/vmcb.c
@@ -20,6 +20,7 @@
 #include <asm/p2m.h>
 #include <asm/spec_ctrl.h>
 
+#include "avic.h"
 #include "svm.h"
 #include "vmcb.h"
 
@@ -215,6 +216,10 @@ int svm_create_vmcb(struct vcpu *v)
     if ( rc )
         return rc;
 
+    rc = svm_avic_vmcb_create(v);
+    if ( rc )
+        return rc;
+
     paddr = virt_to_maddr(nv->nv_n1vmcx);
     nv->nv_n1vmcx_pa = paddr;
     svm->vmcb_pa = paddr;
@@ -227,6 +232,8 @@ void svm_destroy_vmcb(struct vcpu *v)
     struct nestedvcpu *nv = &vcpu_nestedhvm(v);
     struct svm_vcpu *svm = &v->arch.hvm.svm;
 
+    svm_avic_vmcb_destroy(v);
+
     if ( nv->nv_n1vmcx != NULL )
         free_vmcb(nv->nv_n1vmcx);
 
diff --git a/xen/arch/x86/hvm/svm/vmcb.h b/xen/arch/x86/hvm/svm/vmcb.h
index 8b351679fac5..1ee5da053a9a 100644
--- a/xen/arch/x86/hvm/svm/vmcb.h
+++ b/xen/arch/x86/hvm/svm/vmcb.h
@@ -346,7 +346,9 @@ typedef union
         u64 intr_masking: 1;
         u64 vgif_enable:  1;
         u64 vnmi_enable:  1;
-        u64 :             5;
+        u64 rsvd2:        3;
+        u64 x2avic_enable:1;
+        u64 avic_enable:  1;  /* bit 31 */
         u64 vector:       8;
         u64 rsvd3:       24;
     } fields;
@@ -362,6 +364,10 @@ typedef union
     uint64_t raw;
 } virt_ext_t;
 
+/*
+ * AMD64 Achitecture's Programmer's Manual, Volume 2
+ * 15.15.3 VMCB Clean Field
+ */
 typedef union
 {
     struct {
@@ -377,7 +383,7 @@ typedef union
         bool seg:1;        /* 8:  cs, ds, es, ss, cpl */
         bool cr2:1;        /* 9:  cr2 */
         bool lbr:1;        /* 10: debugctlmsr, last{branch,int}{to,from}ip */
-        bool :1;
+        bool avic:1;       /* 11: AVIC */
         bool cet:1;        /* 12: msr_s_set, ssp, msr_isst */
     };
     uint32_t raw;
@@ -469,7 +475,8 @@ struct vmcb_struct {
         };
         uint64_t _np_ctrl;
     };
-    uint64_t res08[2];
+    uint64_t avic_vapic_bar;    /* offset 0x98 */
+    uint64_t res08;             /* offset 0xA0 */
     intinfo_t event_inj;        /* offset 0xA8 */
     uint64_t _h_cr3;            /* offset 0xB0 - cleanbit 4 */
     virt_ext_t virt_ext;        /* offset 0xB8 */
@@ -478,7 +485,11 @@ struct vmcb_struct {
     uint64_t nextrip;           /* offset 0xC8 */
     uint8_t  guest_ins_len;     /* offset 0xD0 */
     uint8_t  guest_ins[15];     /* offset 0xD1 */
-    uint64_t res10a[8];         /* offset 0xE0 */
+    uint64_t avic_backing_page;      /* offset 0xE0 */
+    uint64_t res09a;                 /* offset 0xE8 */
+    uint64_t avic_logical_id_table;  /* offset 0xF0 */
+    uint64_t avic_physical_id_table; /* offset 0xF8 */
+    uint64_t res10a[4];         /* offset 0x100 pad to save area */
     uint16_t bus_lock_count;    /* offset 0x120 */
     uint16_t res10b[3];         /* offset 0x122 */
     uint64_t res10c[91];        /* offset 0x128 pad to save area */
-- 
2.55.0

[PATCH v1 08/28] x86/hvm: avic: add AVIC_INCOMP_IPI VMEXIT handler

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

Introduce new VMEXIT reason 0x401 [1] to handle the case when an IPI cannot
be delivered to all targeted guest virtual processors because at least one
guest virtual processor was not allocated to a physical core at the time.

If incomplete IPI occurs, try to emulate an IPI.

[1] AMD64 APM Volume 2, Table 15-22 "Guest vAPIC Register Access Behavior"
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/hvm/svm/avic.c              | 115 +++++++++++++++++++++++
 xen/arch/x86/hvm/svm/avic.h              |   2 +
 xen/arch/x86/hvm/svm/svm.c               |   4 +
 xen/arch/x86/hvm/svm/vmcb.h              |   7 ++
 xen/arch/x86/include/asm/hvm/svm-types.h |  10 ++
 5 files changed, 138 insertions(+)

diff --git a/xen/arch/x86/hvm/svm/avic.c b/xen/arch/x86/hvm/svm/avic.c
index bc98661a7057..0cfb21cb8e3f 100644
--- a/xen/arch/x86/hvm/svm/avic.c
+++ b/xen/arch/x86/hvm/svm/avic.c
@@ -9,6 +9,7 @@
 #include <xen/param.h>
 
 #include "avic.h"
+#include "svm.h"
 #include "vmcb.h"
 
 #define AVIC_PHYSICAL_ID_ENTRY_BACKING_PAGE_MASK        GENMASK_ULL(51, 12U)
@@ -235,6 +236,120 @@ void svm_avic_vmcb_destroy(struct vcpu *v)
     vmcb->avic_vapic_bar = 0;
 }
 
+/*
+ * Handle the AVIC_INCOMP_IPI #VMEXIT when AVIC is enabled.
+ *
+ * The hardware generates this fault when an IPI could not be delivered to
+ * all targeted guest virtual processors because at least one guest virtual
+ * processor was not allocated to a physical core at the time.
+ */
+void svm_avic_vmexit_incomp_ipi(struct cpu_user_regs *regs)
+{
+    struct vcpu *v = current;
+    struct domain *d = v->domain;
+    struct svm_vcpu *sv = &v->arch.hvm.svm;
+    const struct vmcb_struct *vmcb = sv->vmcb;
+    uint32_t icrh = vmcb->exitinfo1 >> 32;
+    uint32_t icrl = vmcb->exitinfo1;
+    uint32_t id = vmcb->exitinfo2 >> 32;
+    uint32_t index = vmcb->exitinfo2 & 0x1FFU;
+
+    if ( id >= AVIC_IPI_FAILURE_MAX )
+    {
+        domain_crash(
+            d, "%pv: AVIC: Unknown IPI interception (exitinfo1=%#"PRIx64" exitinfo2=%#"PRIx64")\n",
+            v, vmcb->exitinfo1, vmcb->exitinfo2);
+        return;
+    }
+
+    BUG_ON(sv->avic_physical_id == NULL);
+
+    switch ( id )
+    {
+    case AVIC_IPI_FAILURE_INVALID_INT_TYPE:
+        /*
+         * AVIC hardware handles the delivery of IPIs when the specified
+         * Message Type is Fixed (also known as fixed delivery mode) and the
+         * Trigger Mode is edge-triggered. The hardware also supports self and
+         * broadcast delivery modes specified via the Destination
+         * Shorthand(DSH) field of the ICRL. Logical and physical APIC ID
+         * formats are supported. All other IPI types cause a #VMEXIT, which
+         * needs to emulated.
+         */
+        gprintk(XENLOG_ERR,
+                "%pv: AVIC: Invalid IPI type (icr=%#08x:%08x, idx=%u)"
+                " avic_physical_id=%#"PRIx64"\n",
+                v, icrh, icrl, index, ACCESS_ONCE(*sv->avic_physical_id));
+
+        vlapic_reg_write(v, APIC_ICR2, icrh);
+        vlapic_reg_write(v, APIC_ICR,  icrl);
+
+        break;
+
+    case AVIC_IPI_FAILURE_INVALID_TARGET:
+        /*
+         * AVIC can report an invalid target for guest IPIs, for example when
+         * a logical destination mask contains IDs that are not backed by
+         * vCPUs. This is guest-controlled input and is not a reason to crash
+         * the domain. Handle invalid-target exits like unsupported interrupt
+         * types.
+         */
+        gprintk(XENLOG_ERR,
+                "%pv: AVIC: Invalid IPI target (icr=%#08x:%08x, idx=%u)"
+                " avic_physical_id=%#"PRIx64"\n",
+                v, icrh, icrl, index, ACCESS_ONCE(*sv->avic_physical_id));
+
+        vlapic_reg_write(v, APIC_ICR2, icrh);
+        vlapic_reg_write(v, APIC_ICR,  icrl);
+
+        break;
+
+    case AVIC_IPI_FAILURE_TARGET_NOT_RUNNING:
+    {
+        /*
+         * At this point, we expect that the AVIC HW has already set the
+         * appropriate IRR bits on the valid target vCPUs. Kick all the
+         * appropriate vCPUs (e.g. logically addressed or broadcast IPI).
+         */
+        struct vcpu *target;
+        uint32_t dest = GET_xAPIC_DEST_FIELD(icrh);
+        uint32_t short_hand = icrl & APIC_SHORT_MASK;
+        bool dest_mode = icrl & APIC_DEST_MASK;
+
+        for_each_vcpu ( d, target )
+        {
+            if ( target == v )
+                continue;
+
+            if ( vlapic_match_dest(vcpu_vlapic(target),
+                                   vcpu_vlapic(v),
+                                   short_hand, dest, dest_mode) )
+            {
+                vcpu_kick(target);
+            }
+        }
+
+        break;
+    }
+
+    case AVIC_IPI_FAILURE_INVALID_BACKING_PAGE:
+        domain_crash(
+            d, "%pv: AVIC: Invalid backing page (icr=%#08x:%08x, idx=%u)\n",
+            v, icrh, icrl, index);
+        break;
+
+    case AVIC_IPI_FAILURE_INVALID_IPI_VECTOR:
+        domain_crash(
+            d, "%pv: AVIC: Invalid IPI with vector < 16 (%#x)\n",
+            v, index);
+        break;
+
+    default:
+        ASSERT_UNREACHABLE();
+        break;
+    }
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/hvm/svm/avic.h b/xen/arch/x86/hvm/svm/avic.h
index 1084ae683944..2668d4481425 100644
--- a/xen/arch/x86/hvm/svm/avic.h
+++ b/xen/arch/x86/hvm/svm/avic.h
@@ -14,4 +14,6 @@ void svm_avic_domain_relinquish_resources(struct domain *d);
 int svm_avic_vmcb_create(struct vcpu *v);
 void svm_avic_vmcb_destroy(struct vcpu *v);
 
+void svm_avic_vmexit_incomp_ipi(struct cpu_user_regs *regs);
+
 #endif /* __SVM_AVIC_H__ */
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 8b7bfed0049f..693b5c29287a 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -3078,6 +3078,10 @@ void asmlinkage svm_vmexit_handler(void)
         vmcb->bus_lock_count = 1;
         break;
 
+    case VMEXIT_AVIC_INCOMP_IPI:
+        svm_avic_vmexit_incomp_ipi(regs);
+        break;
+
     default:
     unexpected_exit_type:
         gprintk(XENLOG_ERR, "Unexpected vmexit: reason %#"PRIx64", "
diff --git a/xen/arch/x86/hvm/svm/vmcb.h b/xen/arch/x86/hvm/svm/vmcb.h
index 1ee5da053a9a..532382bd02f3 100644
--- a/xen/arch/x86/hvm/svm/vmcb.h
+++ b/xen/arch/x86/hvm/svm/vmcb.h
@@ -291,6 +291,13 @@ enum VMEXIT_EXITCODE
     VMEXIT_BUS_LOCK         = 165, /* 0xa5 */
     /* Remember to also update VMEXIT_NPF_PERFC! */
     VMEXIT_NPF              = 1024, /* 0x400, nested paging fault */
+
+    /*
+     * AMD64 Architecture Programmer's Manual Volume 2: System Programming
+     * Table 15-22. Guest vAPIC Register Access Behavior
+     */
+    VMEXIT_AVIC_INCOMP_IPI  = 1025, /* 0x401 */
+
     /* Remember to also update SVM_PERF_EXIT_REASON_SIZE! */
     VMEXIT_INVALID          =  -1
 };
diff --git a/xen/arch/x86/include/asm/hvm/svm-types.h b/xen/arch/x86/include/asm/hvm/svm-types.h
index 30bdaf27a856..93629418c225 100644
--- a/xen/arch/x86/include/asm/hvm/svm-types.h
+++ b/xen/arch/x86/include/asm/hvm/svm-types.h
@@ -25,6 +25,16 @@ struct svm_domain {
     uint64_t *avic_physical_id_table;
 };
 
+/* AVIC_INCOMP_IPI #VMEXIT reason. */
+enum avic_ipi_failure_cause {
+    AVIC_IPI_FAILURE_INVALID_INT_TYPE       = 0,
+    AVIC_IPI_FAILURE_TARGET_NOT_RUNNING     = 1,
+    AVIC_IPI_FAILURE_INVALID_TARGET         = 2,
+    AVIC_IPI_FAILURE_INVALID_BACKING_PAGE   = 3,
+    AVIC_IPI_FAILURE_INVALID_IPI_VECTOR     = 4,
+    AVIC_IPI_FAILURE_MAX,
+};
+
 struct svm_vcpu {
     struct vmcb_struct *vmcb;
     unsigned long *msrpm;
-- 
2.55.0

[PATCH v1 09/28] x86/hvm: fix hvm_emulate_one_vm_event() definition

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

Add empty definition for !CONFIG_VM_EVENT case.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/hvm/emulate.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index cd2831d34b44..c441cf278614 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -2925,6 +2925,11 @@ void hvm_emulate_one_vm_event(enum emul_kind kind, unsigned int trapnr,
 
     hvm_emulate_writeback(&ctx);
 }
+#else
+void hvm_emulate_one_vm_event(enum emul_kind kind, unsigned int trapnr,
+                              unsigned int errcode)
+{
+}
 #endif /* CONFIG_VM_EVENT */
 
 void hvm_emulate_init_once(
-- 
2.55.0

[PATCH v1 10/28] x86/hvm: avic: add AVIC_NOACCEL VMEXIT handler

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

Normally, TPR, ICR and EOI guest accesses to vLAPIC are accelerated.

Introduce new VMEXIT reason 0x402 [1] to handle the case when a when a
guest access to an vLAPIC register that cannot be accelerated by AVIC.
In this case, emulate register access, specifically, LDR and DFR.

Also, generate fault if an EOI is attempted when the highest priority
in-service interrupt is set for level-triggered mode.

[1] AMD64 APM Volume 2, Table 15-22 "Guest vAPIC Register Access Behavior"
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/hvm/svm/avic.c              | 196 +++++++++++++++++++++++
 xen/arch/x86/hvm/svm/avic.h              |   1 +
 xen/arch/x86/hvm/svm/svm.c               |   4 +
 xen/arch/x86/hvm/svm/vmcb.h              |   1 +
 xen/arch/x86/include/asm/hvm/svm-types.h |   6 +
 5 files changed, 208 insertions(+)

diff --git a/xen/arch/x86/hvm/svm/avic.c b/xen/arch/x86/hvm/svm/avic.c
index 0cfb21cb8e3f..18a814871133 100644
--- a/xen/arch/x86/hvm/svm/avic.c
+++ b/xen/arch/x86/hvm/svm/avic.c
@@ -12,9 +12,20 @@
 #include "svm.h"
 #include "vmcb.h"
 
+#define AVIC_PHYSICAL_ID_ENTRY_VALID_MASK               BIT(63, ULL)
+#define AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK          BIT(62, ULL)
 #define AVIC_PHYSICAL_ID_ENTRY_BACKING_PAGE_MASK        GENMASK_ULL(51, 12U)
+/* Width of Host Physical APIC ID in the AVIC physical table. */
+#define AVIC_PHYSICAL_ID_ENTRY_HOST_PHYSICAL_ID_MASK    GENMASK_ULL(11, 0U)
 #define AVIC_PHYSICAL_ID_TABLE_SIZE_MASK                (0xFFULL)
 
+#define AVIC_LOGICAL_ID_ENTRY_GUEST_PHYSICAL_ID_MASK    (0xFFU)
+#define AVIC_LOGICAL_ID_ENTRY_VALID_BIT                 (31U)
+#define AVIC_LOGICAL_ID_ENTRY_VALID_MASK                BIT(31, U)
+
+#define AVIC_UNACCEL_ACCESS_WRITE_MASK                  (0x1U)
+#define AVIC_UNACCEL_ACCESS_OFFSET_MASK                 (0xFF0U)
+
 /*
  * For AVIC, the max index allowed for physical APIC ID table is 0xfe (254), as
  * 0xff is a broadcast to all CPUs, i.e. can't be targeted individually.
@@ -211,6 +222,7 @@ int svm_avic_vmcb_create(struct vcpu *v)
     id = GET_xAPIC_ID(vlapic_reg_read(v, APIC_ID));
 
     sv->avic_physical_id = &sd->avic_physical_id_table[id];
+    sv->avic_dfr = APIC_DFR_FLAT;
 
     entry = vmcb->avic_backing_page & AVIC_PHYSICAL_ID_ENTRY_BACKING_PAGE_MASK;
     entry |= AVIC_PHYSICAL_ID_ENTRY_VALID_MASK;
@@ -350,6 +362,190 @@ void svm_avic_vmexit_incomp_ipi(struct cpu_user_regs *regs)
     }
 }
 
+static uint32_t *
+avic_get_logical_id_entry(const struct svm_domain *sd, uint32_t ldr, bool flat)
+{
+    unsigned int dest_id = GET_xAPIC_LOGICAL_ID(ldr);
+    unsigned int cluster;
+    unsigned int index;
+
+    if (flat)
+    {
+        cluster = 0;
+    }
+    else
+    {
+        cluster = dest_id >> 4;
+
+        if ( cluster >= 0xF )
+            return NULL;
+
+        dest_id &= 0x0F;
+    }
+
+    if ( !dest_id || (dest_id & (dest_id - 1)) )
+        return NULL;
+
+    index = ffs(dest_id) - 1;
+    if ( index > 7 )
+        return NULL;
+
+    index += cluster << 2;
+
+    return &sd->avic_logical_id_table[index];
+}
+
+static void avic_invalidate_logical_id_entry(struct vcpu *v)
+{
+    const struct svm_domain *sd = &v->domain->arch.hvm.svm;
+    const struct svm_vcpu *sv = &v->arch.hvm.svm;
+    uint32_t *entry;
+    uint32_t ldr = sv->avic_ldr;
+    bool flat = (sv->avic_dfr == APIC_DFR_FLAT);
+
+    entry = avic_get_logical_id_entry(sd, ldr, flat);
+    if ( !entry )
+        return;
+
+    clear_bit(AVIC_LOGICAL_ID_ENTRY_VALID_BIT, entry);
+}
+
+static void avic_ldr_write(struct vcpu *v, uint8_t id, uint32_t ldr)
+{
+    struct svm_domain *sd = &v->domain->arch.hvm.svm;
+    uint32_t *entry, new_entry;
+    uint32_t dfr = vlapic_reg_read(v, APIC_DFR);
+    bool flat = (dfr == APIC_DFR_FLAT);
+
+    entry = avic_get_logical_id_entry(sd, ldr, flat);
+    if ( !entry )
+        return;
+
+    new_entry = ACCESS_ONCE(*entry);
+    new_entry &= ~AVIC_LOGICAL_ID_ENTRY_GUEST_PHYSICAL_ID_MASK;
+    new_entry |= (id & AVIC_LOGICAL_ID_ENTRY_GUEST_PHYSICAL_ID_MASK);
+    new_entry |= AVIC_LOGICAL_ID_ENTRY_VALID_MASK;
+    ACCESS_ONCE(*entry) = new_entry;
+}
+
+static void avic_noaccel_ldr_update(struct vcpu *v)
+{
+    struct svm_vcpu *sv = &v->arch.hvm.svm;
+    uint32_t ldr = vlapic_reg_read(v, APIC_LDR);
+    uint32_t id = GET_xAPIC_ID(vlapic_reg_read(v, APIC_ID));
+
+    if ( ldr == sv->avic_ldr )
+        return;
+
+    avic_invalidate_logical_id_entry(v);
+
+    sv->avic_ldr = ldr;
+
+    avic_ldr_write(v, id, ldr);
+}
+
+static void avic_noaccel_dfr_update(struct vcpu *v)
+{
+    struct svm_vcpu *sv = &v->arch.hvm.svm;
+    uint32_t dfr = vlapic_reg_read(v, APIC_DFR);
+
+    if ( dfr == sv->avic_dfr )
+        return;
+
+    avic_invalidate_logical_id_entry(v);
+
+    sv->avic_dfr = dfr;
+}
+
+static int avic_noaccel_trap_write(struct vcpu *v)
+{
+    const struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
+    uint32_t offset = vmcb->exitinfo1 & AVIC_UNACCEL_ACCESS_OFFSET_MASK;
+    uint32_t reg = vlapic_reg_read(v, offset);
+
+    switch ( offset )
+    {
+    case APIC_ID:
+        /*
+         * Currently, we do not support APIC_ID update while the vCPUs are
+         * running, which might require updating AVIC max APIC ID in all
+         * VMCBs. This would require synchronize update on all running VCPUs.
+         */
+        return X86EMUL_UNHANDLEABLE;
+
+    case APIC_LDR:
+        avic_noaccel_ldr_update(v);
+        break;
+
+    case APIC_DFR:
+        avic_noaccel_dfr_update(v);
+        break;
+
+    default:
+        break;
+    }
+
+    vlapic_reg_write(v, offset, reg);
+
+    return X86EMUL_OKAY;
+}
+
+static inline bool is_svm_avic_vmexit_noaccel_trap(uint32_t offset)
+{
+    static const unsigned long avic_trap[] = {
+#define REG(x) (1UL << (APIC_ ## x >> 4))
+        REG(ID)   | REG(EOI)     | REG(RRR)   | REG(LDR)  |
+        REG(DFR)  | REG(SPIV)    | REG(ESR)   | REG(ICR)  |
+        REG(LVTT) | REG(LVTTHMR) | REG(LVTPC) | REG(LVT0) |
+        REG(LVT1) | REG(LVTERR)  | REG(TMICT) | REG(TDCR)
+#undef REG
+    };
+    uint32_t pos = offset >> 4;
+
+    return pos < (sizeof(avic_trap) * 8) && test_bit(pos, avic_trap);
+}
+
+/*
+ * Handle VMEXIT_AVIC_NOACCEL when AVIC is enabled.
+ *
+ * The hardware generates this fault when:
+ * - A guest access to an APIC register that is not accelerated by AVIC
+ *   hardware.
+ * - EOI is attempted when the highest priority in-service interrupt is
+ *   level-triggered.
+ */
+void svm_avic_vmexit_noaccel(struct cpu_user_regs *regs)
+{
+    struct vcpu *v = current;
+    struct domain *d = v->domain;
+    struct svm_vcpu *sv = &v->arch.hvm.svm;
+    const struct vmcb_struct *vmcb = sv->vmcb;
+    uint32_t offset = vmcb->exitinfo1 & AVIC_UNACCEL_ACCESS_OFFSET_MASK;
+    uint32_t rw = (vmcb->exitinfo1 >> 32) & AVIC_UNACCEL_ACCESS_WRITE_MASK;
+
+    if ( is_svm_avic_vmexit_noaccel_trap(offset) )
+    {
+        /* Handle AVIC Trap (intercept right after the access). */
+
+        if ( !rw )
+            /*
+             * If a read trap happens, the CPU microcode does not implement
+             * the spec.
+             */
+            domain_crash(
+                d, "%pv: AVIC: Invalid #VMEXIT due to trap read (%#x)\n",
+                v, offset);
+
+        if ( avic_noaccel_trap_write(v) != X86EMUL_OKAY )
+            domain_crash(
+                d, "%pv: AVIC: Failed to handle trap write (%#x)\n",
+                v, offset);
+    }
+    else
+        /* Handle AVIC Fault (intercept before the access). */
+        hvm_emulate_one_vm_event(EMUL_KIND_NORMAL, X86_EXC_UD, X86_EVENT_NO_EC);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/hvm/svm/avic.h b/xen/arch/x86/hvm/svm/avic.h
index 2668d4481425..f9ff3ffc9346 100644
--- a/xen/arch/x86/hvm/svm/avic.h
+++ b/xen/arch/x86/hvm/svm/avic.h
@@ -15,5 +15,6 @@ int svm_avic_vmcb_create(struct vcpu *v);
 void svm_avic_vmcb_destroy(struct vcpu *v);
 
 void svm_avic_vmexit_incomp_ipi(struct cpu_user_regs *regs);
+void svm_avic_vmexit_noaccel(struct cpu_user_regs *regs);
 
 #endif /* __SVM_AVIC_H__ */
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 693b5c29287a..673c818a0039 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -3082,6 +3082,10 @@ void asmlinkage svm_vmexit_handler(void)
         svm_avic_vmexit_incomp_ipi(regs);
         break;
 
+    case VMEXIT_AVIC_NOACCEL:
+        svm_avic_vmexit_noaccel(regs);
+        break;
+
     default:
     unexpected_exit_type:
         gprintk(XENLOG_ERR, "Unexpected vmexit: reason %#"PRIx64", "
diff --git a/xen/arch/x86/hvm/svm/vmcb.h b/xen/arch/x86/hvm/svm/vmcb.h
index 532382bd02f3..db3f2689d7f5 100644
--- a/xen/arch/x86/hvm/svm/vmcb.h
+++ b/xen/arch/x86/hvm/svm/vmcb.h
@@ -297,6 +297,7 @@ enum VMEXIT_EXITCODE
      * Table 15-22. Guest vAPIC Register Access Behavior
      */
     VMEXIT_AVIC_INCOMP_IPI  = 1025, /* 0x401 */
+    VMEXIT_AVIC_NOACCEL     = 1026, /* 0x402 */
 
     /* Remember to also update SVM_PERF_EXIT_REASON_SIZE! */
     VMEXIT_INVALID          =  -1
diff --git a/xen/arch/x86/include/asm/hvm/svm-types.h b/xen/arch/x86/include/asm/hvm/svm-types.h
index 93629418c225..377ad2494e7c 100644
--- a/xen/arch/x86/include/asm/hvm/svm-types.h
+++ b/xen/arch/x86/include/asm/hvm/svm-types.h
@@ -45,6 +45,12 @@ struct svm_vcpu {
 
     /* VMCB has a cached instruction from #PF/#NPF Decode Assist? */
     uint8_t cached_insn_len; /* Zero if no cached instruction. */
+
+    /*
+     * vCPU AVIC state.
+     */
+    uint32_t avic_ldr;
+    uint32_t avic_dfr;
 };
 
 struct nestedsvm {
-- 
2.55.0

[PATCH v1 11/28] x86/hvm: introduce vcpu_is_vlapic_tpr_emulated()

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

Introduce vcpu_is_vlapic_tpr_emulated() in genetic SVM code to account
for the case when platform provides accelerated access to vLAPIC's TPR.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/hvm/svm/svm.c | 38 +++++++++++++++++++++++++++-----------
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 673c818a0039..14a86ca4ff8f 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -925,6 +925,19 @@ static void cf_check svm_ctxt_switch_to(struct vcpu *v)
     }
 }
 
+static bool vcpu_is_vlapic_tpr_emulated(const struct vcpu *v, bool force_noaccel)
+{
+    const struct vlapic *vlapic = vcpu_vlapic(v);
+
+    if ( force_noaccel )
+        return false;
+
+    if ( vlapic_hw_disabled(vlapic) )
+        return false;
+
+    return true;
+}
+
 static void noreturn cf_check svm_do_resume(void)
 {
     struct vcpu *v = current;
@@ -933,7 +946,6 @@ static void noreturn cf_check svm_do_resume(void)
                         v->domain->arch.monitor.software_breakpoint_enabled ||
                         v->domain->arch.monitor.debug_exception_enabled);
     bool vcpu_guestmode = false;
-    struct vlapic *vlapic = vcpu_vlapic(v);
 
     if ( nestedhvm_enabled(v->domain) && nestedhvm_vcpu_in_guestmode(v) )
         vcpu_guestmode = 1;
@@ -959,8 +971,9 @@ static void noreturn cf_check svm_do_resume(void)
         vmcb->cleanbits.raw = 0;
     }
 
-    if ( !vcpu_guestmode && !vlapic_hw_disabled(vlapic) )
+    if ( vcpu_is_vlapic_tpr_emulated(v, vcpu_guestmode) )
     {
+        struct vlapic *vlapic = vcpu_vlapic(v);
         vintr_t intr;
 
         /* Reflect the vlapic's TPR in the hardware vtpr */
@@ -2538,7 +2551,6 @@ void asmlinkage svm_vmexit_handler(void)
     int insn_len;
     vintr_t intr;
     bool vcpu_guestmode = false;
-    struct vlapic *vlapic = vcpu_vlapic(v);
 
     regs->rax = vmcb->rax;
     regs->rip = vmcb->rip;
@@ -2565,8 +2577,10 @@ void asmlinkage svm_vmexit_handler(void)
      * NB. We need to preserve the low bits of the TPR to make checked builds
      * of Windows work, even though they don't actually do anything.
      */
-    if ( !vcpu_guestmode && !vlapic_hw_disabled(vlapic) )
+    if ( vcpu_is_vlapic_tpr_emulated(v, vcpu_guestmode) )
     {
+        struct vlapic *vlapic = vcpu_vlapic(v);
+
         intr = vmcb_get_vintr(vmcb);
         vlapic_set_reg(vlapic, APIC_TASKPRI,
                    ((intr.fields.tpr & 0x0F) << 4) |
@@ -3097,14 +3111,16 @@ void asmlinkage svm_vmexit_handler(void)
     }
 
   out:
-    if ( vcpu_guestmode || vlapic_hw_disabled(vlapic) )
-        return;
+    if ( vcpu_is_vlapic_tpr_emulated(v, vcpu_guestmode ) )
+    {
+        struct vlapic *vlapic = vcpu_vlapic(v);
 
-    /* The exit may have updated the TPR: reflect this in the hardware vtpr */
-    intr = vmcb_get_vintr(vmcb);
-    intr.fields.tpr =
-        (vlapic_get_reg(vlapic, APIC_TASKPRI) & 0xFF) >> 4;
-    vmcb_set_vintr(vmcb, intr);
+        /* The exit may have updated the TPR: reflect this in the hardware vtpr */
+        intr = vmcb_get_vintr(vmcb);
+        intr.fields.tpr =
+            (vlapic_get_reg(vlapic, APIC_TASKPRI) & 0xFF) >> 4;
+        vmcb_set_vintr(vmcb, intr);
+    }
 }
 
 /*
-- 
2.55.0

[PATCH v1 12/28] x86: apic: use APIC_EIO_ACK instead of open-coded 0

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

Use the symbolic value in ack_APIC_irq() instead of open-coded 0.

Also, correct the EIO -> EOI typo in the constant name.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/include/asm/apic.h    | 3 +--
 xen/arch/x86/include/asm/apicdef.h | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/include/asm/apic.h b/xen/arch/x86/include/asm/apic.h
index f30d57ad22c5..a4f6d2c26ce1 100644
--- a/xen/arch/x86/include/asm/apic.h
+++ b/xen/arch/x86/include/asm/apic.h
@@ -155,8 +155,7 @@ void apic_wait_icr_idle(void);
 
 static inline void ack_APIC_irq(void)
 {
-	/* Docs say use 0 for future compatibility */
-	apic_write(APIC_EOI, 0);
+	apic_write(APIC_EOI, APIC_EOI_ACK);
 }
 
 extern int get_maxlvt(void);
diff --git a/xen/arch/x86/include/asm/apicdef.h b/xen/arch/x86/include/asm/apicdef.h
index 112c1dc613a1..d3dc7b1d7643 100644
--- a/xen/arch/x86/include/asm/apicdef.h
+++ b/xen/arch/x86/include/asm/apicdef.h
@@ -25,7 +25,7 @@
 #define			APIC_ARBPRI_MASK	0xFF
 #define		APIC_PROCPRI	0xA0
 #define		APIC_EOI	0xB0
-#define			APIC_EIO_ACK		0x0		/* Write this to the EOI register */
+#define			APIC_EOI_ACK		0x0		/* Docs say use 0 for future compatibility */
 #define		APIC_RRR	0xC0
 #define		APIC_LDR	0xD0
 #define			APIC_LDR_MASK		(0xFFu<<24)
-- 
2.55.0

[PATCH v1 13/28] x86/time: fix typo s/interruopt/interrupt/

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/time.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index f37fddc61e47..0e5d06a9a879 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -207,7 +207,7 @@ static void cf_check timer_interrupt(int irq, void *dev_id)
     if ( hpet_legacy_irq_tick() )
         return;
 
-    /* Only for start-of-day interruopt tests in io_apic.c. */
+    /* Only for start-of-day interrupt tests in io_apic.c. */
     pit0_ticks++;
 
     /* Rough hack to allow accurate timers to sort-of-work with no APIC. */
-- 
2.55.0

[PATCH v1 14/28] x86/hvm: vlapic: update signatures of both vlapic_{clear,test}_*

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

Update the signatures of both vlapic_{clear,test}_* calls to
pass 'struct vlapic *' as the first argument and correct 'vector'
to be 'uint8_t'.

Not a functional change.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/hvm/vlapic.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index f929fdfd74ee..22c472d569b5 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -85,14 +85,14 @@ static int vlapic_find_highest_vector(const void *bitmap)
  * IRR-specific bitmap update & search routines.
  */
 
-static int vlapic_test_and_set_irr(int vector, struct vlapic *vlapic)
+static int vlapic_test_and_set_irr(struct vlapic *vlapic, uint8_t vec)
 {
-    return vlapic_test_and_set_vector(vector, &vlapic->regs->data[APIC_IRR]);
+    return vlapic_test_and_set_vector(vec, &vlapic->regs->data[APIC_IRR]);
 }
 
-static void vlapic_clear_irr(int vector, struct vlapic *vlapic)
+static void vlapic_clear_irr(struct vlapic *vlapic, uint8_t vec)
 {
-    vlapic_clear_vector(vector, &vlapic->regs->data[APIC_IRR]);
+    vlapic_clear_vector(vec, &vlapic->regs->data[APIC_IRR]);
 }
 
 static int vlapic_find_highest_irr(struct vlapic *vlapic)
@@ -166,7 +166,7 @@ void vlapic_set_irq(struct vlapic *vlapic, uint8_t vec, uint8_t trig)
 
     if ( hvm_funcs.deliver_posted_intr )
         alternative_vcall(hvm_funcs.deliver_posted_intr, target, vec);
-    else if ( !vlapic_test_and_set_irr(vec, vlapic) )
+    else if ( !vlapic_test_and_set_irr(vlapic, vec) )
         vcpu_kick(target);
 }
 
@@ -1414,7 +1414,7 @@ int vlapic_ack_pending_irq(struct vcpu *v, int vector, bool force_ack)
          !viridian_synic_is_auto_eoi_sint(v, vector) )
         vlapic_set_vector(vector, &vlapic->regs->data[APIC_ISR]);
 
-    vlapic_clear_irr(vector, vlapic);
+    vlapic_clear_irr(vlapic, vector);
 
     return 1;
 }
-- 
2.55.0

[PATCH v1 15/28] x86/hvm: vlapic: add checks before posted interrupt delivery

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

Hoist IRR check for pending vector in question to avoid calling into
posted interrupt delivery callback.

Also, avoid calling posted interrupts when current vCPU is target vCPU.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/hvm/vlapic.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index 22c472d569b5..222a78b02b77 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -164,9 +164,17 @@ void vlapic_set_irq(struct vlapic *vlapic, uint8_t vec, uint8_t trig)
     if ( hvm_funcs.update_eoi_exit_bitmap )
         alternative_vcall(hvm_funcs.update_eoi_exit_bitmap, target, vec, trig);
 
+    if ( vlapic_test_and_set_irr(vlapic, vec) )
+        /* Interrupt vector is already pending */
+        return;
+
+    if ( target == current )
+        /* Kick remote vCPUs only */
+        return;
+
     if ( hvm_funcs.deliver_posted_intr )
         alternative_vcall(hvm_funcs.deliver_posted_intr, target, vec);
-    else if ( !vlapic_test_and_set_irr(vlapic, vec) )
+    else
         vcpu_kick(target);
 }
 
-- 
2.55.0

[PATCH v1 16/28] x86/hvm: move VINTR disable to svm_clear_intr_window()

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

Wrap duplicate code for disabling VINTR into svm_clear_intr_window().

Not a functional change.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/hvm/svm/intr.c      | 13 +++++++++++++
 xen/arch/x86/hvm/svm/nestedsvm.c |  9 +--------
 xen/arch/x86/hvm/svm/svm.c       | 11 +----------
 xen/arch/x86/hvm/svm/svm.h       |  2 ++
 4 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/intr.c b/xen/arch/x86/hvm/svm/intr.c
index 2224f1834741..1d39f25e5209 100644
--- a/xen/arch/x86/hvm/svm/intr.c
+++ b/xen/arch/x86/hvm/svm/intr.c
@@ -238,6 +238,19 @@ void asmlinkage svm_intr_assist(void)
         svm_enable_intr_window(v, intack);
 }
 
+void svm_clear_intr_window(struct vcpu *v)
+{
+    struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
+    uint32_t general1_intercepts = vmcb_get_general1_intercepts(vmcb);
+    vintr_t intr = vmcb_get_vintr(vmcb);
+
+    intr.fields.irq = 0;
+    general1_intercepts &= ~GENERAL1_INTERCEPT_VINTR;
+
+    vmcb_set_vintr(vmcb, intr);
+    vmcb_set_general1_intercepts(vmcb, general1_intercepts);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index d970869cfd96..c7fde48d5281 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -1491,10 +1491,7 @@ void svm_vmexit_do_stgi(struct cpu_user_regs *regs, struct vcpu *v)
 
 void svm_vmexit_do_clgi(struct cpu_user_regs *regs, struct vcpu *v)
 {
-    struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
     unsigned int inst_len;
-    uint32_t general1_intercepts = vmcb_get_general1_intercepts(vmcb);
-    vintr_t intr;
 
     if ( !nsvm_efer_svm_enabled(v) )
     {
@@ -1508,11 +1505,7 @@ void svm_vmexit_do_clgi(struct cpu_user_regs *regs, struct vcpu *v)
     vcpu_nestedsvm(v).ns_gif = 0;
 
     /* After a CLGI no interrupts should come */
-    intr = vmcb_get_vintr(vmcb);
-    intr.fields.irq = 0;
-    general1_intercepts &= ~GENERAL1_INTERCEPT_VINTR;
-    vmcb_set_vintr(vmcb, intr);
-    vmcb_set_general1_intercepts(vmcb, general1_intercepts);
+    svm_clear_intr_window(v);
 
     __update_guest_eip(regs, inst_len);
 }
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 14a86ca4ff8f..d1a95e441f03 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2813,17 +2813,8 @@ void asmlinkage svm_vmexit_handler(void)
         break;
 
     case VMEXIT_VINTR:
-    {
-        u32 general1_intercepts = vmcb_get_general1_intercepts(vmcb);
-        intr = vmcb_get_vintr(vmcb);
-
-        intr.fields.irq = 0;
-        general1_intercepts &= ~GENERAL1_INTERCEPT_VINTR;
-
-        vmcb_set_vintr(vmcb, intr);
-        vmcb_set_general1_intercepts(vmcb, general1_intercepts);
+        svm_clear_intr_window(v);
         break;
-    }
 
     case VMEXIT_INVD:
     case VMEXIT_WBINVD:
diff --git a/xen/arch/x86/hvm/svm/svm.h b/xen/arch/x86/hvm/svm/svm.h
index cfa411ad5ae1..0dd5ee78fce3 100644
--- a/xen/arch/x86/hvm/svm/svm.h
+++ b/xen/arch/x86/hvm/svm/svm.h
@@ -21,6 +21,8 @@ void svm_asid_handle_vmrun(void);
 unsigned long *svm_msrbit(unsigned long *msr_bitmap, uint32_t msr);
 void __update_guest_eip(struct cpu_user_regs *regs, unsigned int inst_len);
 
+void svm_clear_intr_window(struct vcpu *v);
+
 static inline void svm_vmload_pa(paddr_t vmcb)
 {
     asm volatile ( "vmload" :: "a" (vmcb) : "memory" );
-- 
2.55.0

[PATCH v1 17/28] x86/hvm: avic: route interrupt delivery to vCPU

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

Enabling AVIC implicitly disables the V_IRQ, V_INTR_PRIO, V_IGN_TPR,
and V_INTR_VECTOR fields in the VMCB Control Word.

Introduce new interrupt injection code via AVIC backing page.

Essentially, that is handling of 'IsRunning' bit in physical APIC ID
table entry: it needs to be in sync with the scheduled vCPU. Shadow
the state to work around hardware not perceiving 'IsRunning' bit.

Add new doorbell MSR to accelerate IPI emulation. The MSR can be used
for posting interrupts from hypervisor to the guest (e.g. event
channels).

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/hvm/svm/avic.c              | 156 +++++++++++++++++++++++
 xen/arch/x86/hvm/svm/avic.h              |  17 +++
 xen/arch/x86/hvm/svm/intr.c              |   9 ++
 xen/arch/x86/hvm/svm/svm.c               |  32 +++++
 xen/arch/x86/include/asm/hvm/svm-types.h |   2 +
 xen/arch/x86/include/asm/msr-index.h     |   1 +
 6 files changed, 217 insertions(+)

diff --git a/xen/arch/x86/hvm/svm/avic.c b/xen/arch/x86/hvm/svm/avic.c
index 18a814871133..0bcab0465227 100644
--- a/xen/arch/x86/hvm/svm/avic.c
+++ b/xen/arch/x86/hvm/svm/avic.c
@@ -33,6 +33,8 @@
 #define AVIC_MAX_PHYSICAL_ID                            (0xFEU)
 
 bool __read_mostly svm_avic_enable;
+unsigned int __read_mostly svm_avic_features = AVIC_HW_ISRUNNING |
+                                               AVIC_DOORBELL_MSR;
 
 static const char __section(".bss.page_aligned.const") __aligned(PAGE_SIZE)
     avic_apic_access_page[PAGE_SIZE];
@@ -50,6 +52,11 @@ static int cf_check avic_parse_cmdline(const char *s)
     {
         svm_avic_enable = true;
     }
+    else if ( !strcmp(s, "nodoorbell") )
+    {
+        svm_avic_enable = true;
+        svm_avic_features &= ~AVIC_DOORBELL_MSR;
+    }
     else
     {
         return -EINVAL;
@@ -228,6 +235,7 @@ int svm_avic_vmcb_create(struct vcpu *v)
     entry |= AVIC_PHYSICAL_ID_ENTRY_VALID_MASK;
 
     ACCESS_ONCE(*sv->avic_physical_id) = entry;
+    ACCESS_ONCE(sv->avic_shadow_id) = entry;
 
     avic_vcpu_enable(v, true);
 
@@ -248,6 +256,102 @@ void svm_avic_vmcb_destroy(struct vcpu *v)
     vmcb->avic_vapic_bar = 0;
 }
 
+static void avic_vcpu_set_cpu(struct vcpu *v)
+{
+    struct svm_vcpu *sv = &v->arch.hvm.svm;
+    unsigned apic_id = cpu_data[v->processor].apicid;
+    uint64_t entry;
+
+    BUG_ON(sv->avic_physical_id == NULL);
+    ASSERT(!(apic_id & ~AVIC_PHYSICAL_ID_ENTRY_HOST_PHYSICAL_ID_MASK));
+
+    entry = ACCESS_ONCE(sv->avic_shadow_id);
+    entry &= ~AVIC_PHYSICAL_ID_ENTRY_HOST_PHYSICAL_ID_MASK;
+    entry |= apic_id;
+
+    ACCESS_ONCE(*sv->avic_physical_id) = entry;
+
+    ACCESS_ONCE(sv->avic_shadow_id) = entry;
+}
+
+static void avic_vcpu_set_running(struct vcpu *v)
+{
+    struct svm_vcpu *sv = &v->arch.hvm.svm;
+    uint64_t entry;
+
+    BUG_ON(sv->avic_physical_id == NULL);
+
+    entry = ACCESS_ONCE(sv->avic_shadow_id);
+    entry |= AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK;
+
+    /* Shadow IsRunning state update. */
+    ACCESS_ONCE(sv->avic_shadow_id) = entry;
+
+    /*
+     * If hardware has problems detecting IsRunning (erratum 1235)
+     * keep IsRunning in the shadow entry only.
+     */
+    if ( svm_avic_features & AVIC_HW_ISRUNNING )
+        ACCESS_ONCE(*sv->avic_physical_id) = entry;
+}
+
+static void avic_vcpu_clear_running(struct vcpu *v)
+{
+    struct svm_vcpu *sv = &v->arch.hvm.svm;
+    unsigned apic_id = cpu_data[v->processor].apicid;
+    uint64_t entry;
+
+    BUG_ON(sv->avic_physical_id == NULL);
+    ASSERT(!(apic_id & ~AVIC_PHYSICAL_ID_ENTRY_HOST_PHYSICAL_ID_MASK));
+
+    entry = ACCESS_ONCE(sv->avic_shadow_id);
+    entry &= ~AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK;
+
+    /*
+     * If hardware has problems detecting IsRunning (erratum 1235)
+     * keep IsRunning in the shadow entry only.
+     */
+    if ( svm_avic_features & AVIC_HW_ISRUNNING )
+        ACCESS_ONCE(*sv->avic_physical_id) = entry;
+
+    /* Shadow IsRunning state update. */
+    ACCESS_ONCE(sv->avic_shadow_id) = entry;
+}
+
+/* vCPU enters blocking state. */
+void svm_avic_vcpu_block(struct vcpu *v)
+{
+    avic_vcpu_clear_running(v);
+}
+
+/* vCPU is preparing to resume guest execution. */
+void svm_avic_vcpu_resume(struct vcpu *v)
+{
+    avic_vcpu_set_running(v);
+}
+
+/* vCPU is being assigned a new pCPU. */
+void svm_avic_vcpu_switch_to(struct vcpu *v)
+{
+    avic_vcpu_set_cpu(v);
+}
+
+/* vCPU is being removed from a pCPU. */
+void svm_avic_vcpu_switch_from(struct vcpu *v)
+{
+    avic_vcpu_clear_running(v);
+}
+
+bool svm_avic_vcpu_enabled(const struct vcpu *v)
+{
+    const struct svm_vcpu *sv = &v->arch.hvm.svm;
+    const struct vmcb_struct *vmcb = sv->vmcb;
+
+    ASSERT(!(vmcb->_vintr.fields.x2avic_enable));
+
+    return vmcb->_vintr.fields.avic_enable;
+}
+
 /*
  * Handle the AVIC_INCOMP_IPI #VMEXIT when AVIC is enabled.
  *
@@ -546,6 +650,58 @@ void svm_avic_vmexit_noaccel(struct cpu_user_regs *regs)
         hvm_emulate_one_vm_event(EMUL_KIND_NORMAL, X86_EXC_UD, X86_EVENT_NO_EC);
 }
 
+static void avic_deliver_posted_intr(struct vcpu *v)
+{
+    struct svm_vcpu *sv = &v->arch.hvm.svm;
+    uint64_t entry;
+    unsigned int apicid;
+
+    smp_mb();
+
+    entry = ACCESS_ONCE(sv->avic_shadow_id);
+
+    if ( !(entry & AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK) )
+        return;
+
+    /*
+     * Some AMD systems (e.g. Zen5 Family 26 (0x1a), Model 36 (0x24)) have
+     * broken remote notification via doorbell MSR.
+     * Fallback to brute force, if requested.
+     */
+    if ( svm_avic_features & AVIC_DOORBELL_MSR )
+    {
+        apicid = entry & AVIC_PHYSICAL_ID_ENTRY_HOST_PHYSICAL_ID_MASK;
+
+        /* Ensure the pending interrupt wakes vCPU. */
+        vcpu_unblock(v);
+
+        /*
+         * AMD APM, 5.29.8.2 Doorbell Mechanism
+         * The serializing semantics of WRMSR are relaxed when writing to
+         * the Doorbell Register.
+         */
+        wrmsrl(MSR_AMD_AVIC_DOORBELL, apicid);
+    }
+    else
+    {
+        vcpu_kick(v);
+    }
+}
+
+void cf_check svm_avic_hvm_deliver_posted_intr(struct vcpu *v, uint8_t vector)
+{
+    ASSERT(v != current);
+
+    if ( svm_avic_vcpu_enabled(v) )
+    {
+        avic_deliver_posted_intr(v);
+    }
+    else
+    {
+        vcpu_kick(v);
+    }
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/hvm/svm/avic.h b/xen/arch/x86/hvm/svm/avic.h
index f9ff3ffc9346..098924925e89 100644
--- a/xen/arch/x86/hvm/svm/avic.h
+++ b/xen/arch/x86/hvm/svm/avic.h
@@ -6,6 +6,15 @@
 #ifndef __SVM_AVIC_H__
 #define __SVM_AVIC_H__
 
+#include <xen/const.h>
+
+/* Known AVIC hardware quirks. */
+enum {
+    AVIC_HW_ISRUNNING       = BIT(0, U),    /* See erratum 1235 */
+    AVIC_DOORBELL_MSR       = BIT(1, U),    /* Remote vCPU notification OK */
+};
+
+extern unsigned int svm_avic_features;
 extern bool svm_avic_enable;
 
 int svm_avic_domain_initialise(struct domain *d);
@@ -17,4 +26,12 @@ void svm_avic_vmcb_destroy(struct vcpu *v);
 void svm_avic_vmexit_incomp_ipi(struct cpu_user_regs *regs);
 void svm_avic_vmexit_noaccel(struct cpu_user_regs *regs);
 
+bool svm_avic_vcpu_enabled(const struct vcpu *v);
+void svm_avic_vcpu_block(struct vcpu *v);
+void svm_avic_vcpu_resume(struct vcpu *v);
+void svm_avic_vcpu_switch_to(struct vcpu *v);
+void svm_avic_vcpu_switch_from(struct vcpu *v);
+
+void cf_check svm_avic_hvm_deliver_posted_intr(struct vcpu *v, uint8_t vector);
+
 #endif /* __SVM_AVIC_H__ */
diff --git a/xen/arch/x86/hvm/svm/intr.c b/xen/arch/x86/hvm/svm/intr.c
index 1d39f25e5209..9cceca704a91 100644
--- a/xen/arch/x86/hvm/svm/intr.c
+++ b/xen/arch/x86/hvm/svm/intr.c
@@ -24,6 +24,7 @@
 #include <public/hvm/ioreq.h>
 #include <xen/domain_page.h>
 
+#include "avic.h"
 #include "nestedhvm.h"
 #include "vmcb.h"
 
@@ -80,6 +81,14 @@ static void svm_enable_intr_window(struct vcpu *v, struct hvm_intack intack)
 
     ASSERT(intack.source != hvm_intsrc_none);
 
+    if ( svm_avic_vcpu_enabled(v) )
+        /*
+         * AVIC delivers pending LAPIC interrupts from the backing page
+         * when the guest becomes interruptible, no need to arm the legacy
+         * V_IRQ/VINTR interrupt-window mechanism.
+         */
+        return;
+
     if ( nestedhvm_enabled(v->domain) )
     {
         struct nestedvcpu *nv = &vcpu_nestedhvm(v);
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index d1a95e441f03..2fac76725c4f 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -35,6 +35,7 @@
 #include <asm/p2m.h>
 #include <asm/paging.h>
 #include <asm/processor.h>
+#include <asm/pv/shim.h>
 #include <asm/vm_event.h>
 #include <asm/x86_emulate.h>
 
@@ -877,6 +878,10 @@ static void cf_check svm_ctxt_switch_from(struct vcpu *v)
     svm_tsc_ratio_save(v);
 
     svm_sync_vmcb(v, vmcb_needs_vmload);
+
+    if ( svm_avic_vcpu_enabled(v) )
+        svm_avic_vcpu_switch_from(v);
+
     svm_vmload_pa(per_cpu(host_vmcb, cpu));
 
     /* Resume use of ISTs now that the host TR is reinstated. */
@@ -914,6 +919,9 @@ static void cf_check svm_ctxt_switch_to(struct vcpu *v)
 
     svm_tsc_ratio_load(v);
 
+    if ( svm_avic_vcpu_enabled(v) )
+        svm_avic_vcpu_switch_to(v);
+
     if ( cpu_has_msr_tsc_aux )
         wrmsr_tsc_aux(v->arch.msrs->tsc_aux);
 
@@ -935,6 +943,9 @@ static bool vcpu_is_vlapic_tpr_emulated(const struct vcpu *v, bool force_noaccel
     if ( vlapic_hw_disabled(vlapic) )
         return false;
 
+    if ( svm_avic_vcpu_enabled(v) )
+        return false;
+
     return true;
 }
 
@@ -982,6 +993,9 @@ static void noreturn cf_check svm_do_resume(void)
             (vlapic_get_reg(vlapic, APIC_TASKPRI) & 0xFF) >> 4;
         vmcb_set_vintr(vmcb, intr);
     }
+    else if ( svm_avic_vcpu_enabled(v) )
+        /* AVIC accelerates TPR accesses. */
+        svm_avic_vcpu_resume(v);
 
     hvm_do_resume(v);
 
@@ -1091,6 +1105,12 @@ static int cf_check acpi_c1e_quirk(
     return X86EMUL_OKAY;
 }
 
+static void cf_check svm_vcpu_block(struct vcpu *v)
+{
+    if ( svm_avic_vcpu_enabled(v) )
+        svm_avic_vcpu_block(v);
+}
+
 static int cf_check svm_domain_initialise(struct domain *d)
 {
     static const struct arch_csw csw = {
@@ -1104,6 +1124,7 @@ static int cf_check svm_domain_initialise(struct domain *d)
     if ( rc )
         return rc;
 
+    d->arch.hvm.pi_ops.vcpu_block = svm_vcpu_block;
     d->arch.ctxt_switch = &csw;
 
     svm_guest_osvw_init(d);
@@ -2493,6 +2514,13 @@ static void __init svm_avic_start(void)
 {
     if ( !cpu_has_svm_avic || pv_shim )
         svm_avic_enable = false;
+
+    if ( !svm_avic_enable )
+        return;
+
+    svm_function_table.deliver_posted_intr = svm_avic_hvm_deliver_posted_intr;
+
+    svm_function_table.caps.virtual_intr_delivery = true;
 }
 
 const struct hvm_function_table * __init start_svm(void)
@@ -2814,6 +2842,10 @@ void asmlinkage svm_vmexit_handler(void)
 
     case VMEXIT_VINTR:
         svm_clear_intr_window(v);
+
+        if ( svm_avic_vcpu_enabled(v) )
+            domain_crash(v->domain, "%pv: VMEXIT_VINTR=%#"PRIx64"\n", v, intr.bytes);
+
         break;
 
     case VMEXIT_INVD:
diff --git a/xen/arch/x86/include/asm/hvm/svm-types.h b/xen/arch/x86/include/asm/hvm/svm-types.h
index 377ad2494e7c..cc40b1b02095 100644
--- a/xen/arch/x86/include/asm/hvm/svm-types.h
+++ b/xen/arch/x86/include/asm/hvm/svm-types.h
@@ -51,6 +51,8 @@ struct svm_vcpu {
      */
     uint32_t avic_ldr;
     uint32_t avic_dfr;
+    uint64_t *avic_physical_id;
+    uint64_t avic_shadow_id;
 };
 
 struct nestedsvm {
diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h
index ad1c6c97f8f7..84fd9b6581b6 100644
--- a/xen/arch/x86/include/asm/msr-index.h
+++ b/xen/arch/x86/include/asm/msr-index.h
@@ -406,6 +406,7 @@
 #define MSR_K8_PSTATE7			0xc001006BU
 #define MSR_K8_ENABLE_C1E		0xc0010055U
 #define MSR_K8_VM_HSAVE_PA		0xc0010117U
+#define MSR_AMD_AVIC_DOORBELL		0xc001011bU
 
 #define MSR_AMD_FAM15H_EVNTSEL0		0xc0010200U
 #define MSR_AMD_FAM15H_PERFCTR0		0xc0010201U
-- 
2.55.0

[PATCH v1 18/28] x86/hvm: add facility to print hardware-assisted vLAPIC info

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

Introduce new method 'dump_posted_intr' in 'hvm_function_table' to enable
hardware-assisted vLAPIC printouts (e.g. performance counters).

Update 'interrupt bindings' keyhandler with a call to the newly introduced
hvm_dump_posted_intr().

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/include/asm/hvm/hvm.h | 12 ++++++++++++
 xen/arch/x86/irq.c                 | 24 ++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/xen/arch/x86/include/asm/hvm/hvm.h b/xen/arch/x86/include/asm/hvm/hvm.h
index 9bf2064925fa..1f0f7749b579 100644
--- a/xen/arch/x86/include/asm/hvm/hvm.h
+++ b/xen/arch/x86/include/asm/hvm/hvm.h
@@ -217,6 +217,7 @@ struct hvm_function_table {
     void (*update_eoi_exit_bitmap)(struct vcpu *v, uint8_t vector, bool set);
     void (*process_isr)(int isr, struct vcpu *v);
     void (*deliver_posted_intr)(struct vcpu *v, u8 vector);
+    void (*dump_posted_intr)(const struct vcpu *v);
     void (*sync_pir_to_irr)(struct vcpu *v);
     bool (*test_pir)(const struct vcpu *v, uint8_t vector);
     void (*handle_eoi)(uint8_t vector, int isr);
@@ -851,6 +852,12 @@ static inline void hvm_sync_pir_to_irr(struct vcpu *v)
         alternative_vcall(hvm_funcs.sync_pir_to_irr, v);
 }
 
+static inline void hvm_dump_posted_intr(const struct vcpu *v)
+{
+    if ( hvm_funcs.dump_posted_intr )
+        alternative_vcall(hvm_funcs.dump_posted_intr, v);
+}
+
 #else  /* CONFIG_HVM */
 
 #define hvm_enabled false
@@ -958,6 +965,11 @@ static inline void hvm_set_reg(struct vcpu *v, unsigned int reg, uint64_t val)
     ASSERT_UNREACHABLE();
 }
 
+static inline void hvm_dump_posted_intr(const struct vcpu *v)
+{
+    ASSERT_UNREACHABLE();
+}
+
 #define is_viridian_domain(d) ((void)(d), false)
 #define is_viridian_vcpu(v) ((void)(v), false)
 #define has_viridian_time_ref_count(d) ((void)(d), false)
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 32a75f77a46d..de79e36fdfe7 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -2527,6 +2527,28 @@ void free_domain_pirqs(struct domain *d)
     pcidevs_unlock();
 }
 
+static void dump_posted_intr_info(void)
+{
+    struct domain *d;
+    struct vcpu *v;
+
+    rcu_read_lock(&domlist_read_lock);
+
+    printk("****** Posted Interrupts Statistics ******\n");
+
+    for_each_domain ( d )
+    {
+        printk("\n>>> Domain %d <<<\n", d->domain_id);
+
+        for_each_vcpu ( d, v )
+            hvm_dump_posted_intr(v);
+    }
+
+    printk("******************************************\n");
+
+    rcu_read_unlock(&domlist_read_lock);
+}
+
 static void cf_check dump_irqs(unsigned char key)
 {
     int i, irq, pirq;
@@ -2605,6 +2627,8 @@ static void cf_check dump_irqs(unsigned char key)
             printk("   %#02x -> %ps()\n", i, direct_apic_vector[i]);
 
     dump_ioapic_irq_info();
+
+    dump_posted_intr_info();
 }
 
 static int __init cf_check setup_dump_irqs(void)
-- 
2.55.0

[PATCH v1 19/28] x86/hvm: avic: enable SVM AVIC statistics reporting

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

Introduce performance counters for AVIC.

Enable statistics reporing via new 'dump_posted_intr' mechanism.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/hvm/svm/avic.c              | 50 ++++++++++++++++++++++++
 xen/arch/x86/hvm/svm/avic.h              |  1 +
 xen/arch/x86/hvm/svm/svm.c               |  1 +
 xen/arch/x86/include/asm/hvm/svm-types.h |  5 +++
 4 files changed, 57 insertions(+)

diff --git a/xen/arch/x86/hvm/svm/avic.c b/xen/arch/x86/hvm/svm/avic.c
index 0bcab0465227..4dcd2c94ea87 100644
--- a/xen/arch/x86/hvm/svm/avic.c
+++ b/xen/arch/x86/hvm/svm/avic.c
@@ -201,6 +201,17 @@ static void avic_vcpu_enable(const struct vcpu *v, bool enable)
            v->domain->max_vcpus);
 }
 
+static void avic_stats_reset(struct vcpu *v)
+{
+    struct svm_vcpu *sv = &v->arch.hvm.svm;
+
+    memset(sv->avic_cnt_incomp_ipi, 0, sizeof(sv->avic_cnt_incomp_ipi));
+    sv->avic_cnt_noaccel = 0;
+    sv->avic_cnt_post_intr = 0;
+    sv->avic_cnt_doorbell = 0;
+    sv->avic_cnt_kick = 0;
+}
+
 int svm_avic_vmcb_create(struct vcpu *v)
 {
     struct domain *d = v->domain;
@@ -237,6 +248,8 @@ int svm_avic_vmcb_create(struct vcpu *v)
     ACCESS_ONCE(*sv->avic_physical_id) = entry;
     ACCESS_ONCE(sv->avic_shadow_id) = entry;
 
+    avic_stats_reset(v);
+
     avic_vcpu_enable(v, true);
 
     return 0;
@@ -380,6 +393,8 @@ void svm_avic_vmexit_incomp_ipi(struct cpu_user_regs *regs)
 
     BUG_ON(sv->avic_physical_id == NULL);
 
+    sv->avic_cnt_incomp_ipi[id]++;
+
     switch ( id )
     {
     case AVIC_IPI_FAILURE_INVALID_INT_TYPE:
@@ -627,6 +642,8 @@ void svm_avic_vmexit_noaccel(struct cpu_user_regs *regs)
     uint32_t offset = vmcb->exitinfo1 & AVIC_UNACCEL_ACCESS_OFFSET_MASK;
     uint32_t rw = (vmcb->exitinfo1 >> 32) & AVIC_UNACCEL_ACCESS_WRITE_MASK;
 
+    sv->avic_cnt_noaccel++;
+
     if ( is_svm_avic_vmexit_noaccel_trap(offset) )
     {
         /* Handle AVIC Trap (intercept right after the access). */
@@ -675,6 +692,8 @@ static void avic_deliver_posted_intr(struct vcpu *v)
         /* Ensure the pending interrupt wakes vCPU. */
         vcpu_unblock(v);
 
+        sv->avic_cnt_doorbell++;
+
         /*
          * AMD APM, 5.29.8.2 Doorbell Mechanism
          * The serializing semantics of WRMSR are relaxed when writing to
@@ -684,24 +703,55 @@ static void avic_deliver_posted_intr(struct vcpu *v)
     }
     else
     {
+        sv->avic_cnt_kick++;
+
         vcpu_kick(v);
     }
 }
 
 void cf_check svm_avic_hvm_deliver_posted_intr(struct vcpu *v, uint8_t vector)
 {
+    struct svm_vcpu *sv = &v->arch.hvm.svm;
+
     ASSERT(v != current);
 
     if ( svm_avic_vcpu_enabled(v) )
     {
+        sv->avic_cnt_post_intr++;
+
         avic_deliver_posted_intr(v);
     }
     else
     {
+        sv->avic_cnt_kick++;
+
         vcpu_kick(v);
     }
 }
 
+void cf_check svm_avic_hvm_dump_posted_intr(const struct vcpu *v)
+{
+    const struct svm_vcpu *sv = &v->arch.hvm.svm;
+
+    /* NB: Do not gate with svm_avic_vcpu_enabled() check for tests */
+    printk("%pv: AVIC: incomp_ipi = %"PRIu64"/%"PRIu64"/%"PRIu64"/%"PRIu64"/%"PRIu64""
+                     " noaccel = %"PRIu64""
+                     " post_intr = %"PRIu64""
+                     " doorbell = %"PRIu64""
+                     " kick = %"PRIu64""
+                     "\n",
+           v,
+           sv->avic_cnt_incomp_ipi[AVIC_IPI_FAILURE_INVALID_INT_TYPE],
+           sv->avic_cnt_incomp_ipi[AVIC_IPI_FAILURE_TARGET_NOT_RUNNING],
+           sv->avic_cnt_incomp_ipi[AVIC_IPI_FAILURE_INVALID_TARGET],
+           sv->avic_cnt_incomp_ipi[AVIC_IPI_FAILURE_INVALID_BACKING_PAGE],
+           sv->avic_cnt_incomp_ipi[AVIC_IPI_FAILURE_INVALID_IPI_VECTOR],
+           sv->avic_cnt_noaccel,
+           sv->avic_cnt_post_intr,
+           sv->avic_cnt_doorbell,
+           sv->avic_cnt_kick);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/hvm/svm/avic.h b/xen/arch/x86/hvm/svm/avic.h
index 098924925e89..24d5fab59b29 100644
--- a/xen/arch/x86/hvm/svm/avic.h
+++ b/xen/arch/x86/hvm/svm/avic.h
@@ -33,5 +33,6 @@ void svm_avic_vcpu_switch_to(struct vcpu *v);
 void svm_avic_vcpu_switch_from(struct vcpu *v);
 
 void cf_check svm_avic_hvm_deliver_posted_intr(struct vcpu *v, uint8_t vector);
+void cf_check svm_avic_hvm_dump_posted_intr(const struct vcpu *v);
 
 #endif /* __SVM_AVIC_H__ */
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 2fac76725c4f..83977d411b0a 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2519,6 +2519,7 @@ static void __init svm_avic_start(void)
         return;
 
     svm_function_table.deliver_posted_intr = svm_avic_hvm_deliver_posted_intr;
+    svm_function_table.dump_posted_intr = svm_avic_hvm_dump_posted_intr;
 
     svm_function_table.caps.virtual_intr_delivery = true;
 }
diff --git a/xen/arch/x86/include/asm/hvm/svm-types.h b/xen/arch/x86/include/asm/hvm/svm-types.h
index cc40b1b02095..ac411f781ed4 100644
--- a/xen/arch/x86/include/asm/hvm/svm-types.h
+++ b/xen/arch/x86/include/asm/hvm/svm-types.h
@@ -53,6 +53,11 @@ struct svm_vcpu {
     uint32_t avic_dfr;
     uint64_t *avic_physical_id;
     uint64_t avic_shadow_id;
+    uint64_t avic_cnt_incomp_ipi[AVIC_IPI_FAILURE_MAX];
+    uint64_t avic_cnt_noaccel;
+    uint64_t avic_cnt_post_intr;
+    uint64_t avic_cnt_doorbell;
+    uint64_t avic_cnt_kick;
 };
 
 struct nestedsvm {
-- 
2.55.0

[PATCH v1 20/28] x86/hvm: rework XEN_DOMCTL_debug_op

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

Currently, XEN_DOMCTL_debug_op assumes operations on VCPUs only
and only for Intel VMX-supported HVM domains. This limits its
usability in the test infrastructure.

Generalize XEN_DOMCTL_debug_op to allow operations on any x86 HVM
domain.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/domctl.c              | 24 ++++--------
 xen/arch/x86/hvm/hvm.c             | 59 ++++++++++++++++++++----------
 xen/arch/x86/include/asm/hvm/hvm.h |  3 +-
 xen/include/public/domctl.h        |  1 +
 4 files changed, 49 insertions(+), 38 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 12c14bb3f514..7a52aff71620 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -912,23 +912,13 @@ long arch_do_domctl(
 
     case XEN_DOMCTL_debug_op:
     {
-        struct vcpu *v;
-
-        ret = -EOPNOTSUPP;
-        if ( !IS_ENABLED(CONFIG_HVM) )
-            break;
-
-        ret = -EINVAL;
-        if ( (domctl->u.debug_op.vcpu >= d->max_vcpus) ||
-             ((v = d->vcpu[domctl->u.debug_op.vcpu]) == NULL) )
-            break;
-
-        ret = -EINVAL;
-        if ( (v == curr) || /* no vcpu_pause() */
-             !is_hvm_domain(d) )
-            break;
-
-        ret = hvm_debug_op(v, domctl->u.debug_op.op);
+        if ( IS_ENABLED(CONFIG_HVM) )
+            ret = hvm_debug_op(d,
+                               domctl->u.debug_op.op,
+                               domctl->u.debug_op.vcpu,
+                               domctl->u.debug_op.arg);
+        else
+            ret = -EOPNOTSUPP;
         break;
     }
 
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index ef9c7cd05777..42139837904a 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -5200,32 +5200,52 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
     return rc;
 }
 
-int hvm_debug_op(struct vcpu *v, int32_t op)
+static int hvm_debug_op_single_step_allowed(const struct domain *d,
+                                            unsigned int vcpu)
 {
+    if ( (vcpu >= d->max_vcpus) || (d->vcpu[vcpu] == NULL) )
+        return -EINVAL;
+
+    if ( (d->vcpu[vcpu] == current) || /* no vcpu_pause() */
+         !is_hvm_domain(d) )
+        return -EINVAL;
+
+    if ( !IS_ENABLED(CONFIG_INTEL_VMX) || !cpu_has_monitor_trap_flag )
+        return -EOPNOTSUPP;
+
+    return 0;
+}
+
+int hvm_debug_op(struct domain *d, unsigned int op, unsigned int vcpu,
+                 unsigned int arg)
+{
+    struct vcpu *v;
     int rc = 0;
 
-    switch ( op )
-    {
-        case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_ON:
-        case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_OFF:
-            if ( !cpu_has_monitor_trap_flag )
-                return -EOPNOTSUPP;
-            break;
-        default:
-            return -ENOSYS;
-    }
-
-    BUILD_ERROR_IF_NOT(CONFIG_INTEL_VMX);
-
-    vcpu_pause(v);
-
     switch ( op )
     {
     case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_ON:
+        rc = hvm_debug_op_single_step_allowed(d, vcpu);
+        if ( rc )
+            break;
+
+        v = d->vcpu[vcpu];
+
+        vcpu_pause(v);
         v->arch.hvm.single_step = true;
+        vcpu_unpause(v); /* guest will latch new state */
+
         break;
 
     case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_OFF:
+        rc = hvm_debug_op_single_step_allowed(d, vcpu);
+        if ( rc )
+            break;
+
+        v = d->vcpu[vcpu];
+
+        vcpu_pause(v);
+
         v->arch.hvm.single_step = false;
 
 #ifdef CONFIG_ALTP2M
@@ -5233,15 +5253,14 @@ int hvm_debug_op(struct vcpu *v, int32_t op)
         v->arch.hvm.fast_single_step.p2midx = 0;
 #endif
 
+        vcpu_unpause(v); /* guest will latch new state */
+
         break;
 
-    default: /* Excluded above */
-        ASSERT_UNREACHABLE();
+    default:
         return -ENOSYS;
     }
 
-    vcpu_unpause(v); /* guest will latch new state */
-
     return rc;
 }
 
diff --git a/xen/arch/x86/include/asm/hvm/hvm.h b/xen/arch/x86/include/asm/hvm/hvm.h
index 1f0f7749b579..6efc533b35df 100644
--- a/xen/arch/x86/include/asm/hvm/hvm.h
+++ b/xen/arch/x86/include/asm/hvm/hvm.h
@@ -369,7 +369,8 @@ void *hvm_map_guest_frame_ro(unsigned long gfn, bool permanent);
 void hvm_unmap_guest_frame(void *p, bool permanent);
 void hvm_mapped_guest_frames_mark_dirty(struct domain *d);
 
-int hvm_debug_op(struct vcpu *v, int32_t op);
+int hvm_debug_op(struct domain *d, unsigned int op, unsigned int vcpu,
+                 unsigned int arg);
 
 /* Caller should pause vcpu before calling this function */
 void hvm_toggle_singlestep(struct vcpu *v);
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 4ca8a2d7ca65..546d9a8ac9c0 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -768,6 +768,7 @@ struct xen_domctl_subscribe {
 struct xen_domctl_debug_op {
     uint32_t op;   /* IN */
     uint32_t vcpu; /* IN */
+    uint32_t arg;  /* IN */
 };
 
 /*
-- 
2.55.0

[PATCH v1 21/28] x86/hvm: enable debug hypercall for posted interrupts

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

There is a need to change AVIC behavior at run-time to enable feature
testing.

Add new subcommand XEN_DOMCTL_DEBUG_OP_POSTED_INTR for XEN_DOMCTL_debug_op
hypercall.

Perform necessary plumbing through hvm_function_table and export
convenience functions for AVIC.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/hvm/hvm.c             |  15 ++++
 xen/arch/x86/hvm/svm/avic.c        | 132 +++++++++++++++++++++++++++++
 xen/arch/x86/hvm/svm/avic.h        |   2 +
 xen/arch/x86/hvm/svm/svm.c         |   1 +
 xen/arch/x86/include/asm/hvm/hvm.h |  17 ++++
 xen/include/public/domctl.h        |   1 +
 6 files changed, 168 insertions(+)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 42139837904a..a6ca56b08942 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -5257,6 +5257,21 @@ int hvm_debug_op(struct domain *d, unsigned int op, unsigned int vcpu,
 
         break;
 
+    case XEN_DOMCTL_DEBUG_OP_POSTED_INTR:
+        gdprintk(XENLOG_G_INFO,
+                "%pd: XEN_DOMCTL_DEBUG_OP_POSTED_INTR %u\n",
+                d, arg);
+
+        if ( current->domain != d )
+            domain_pause(d);
+
+        rc = hvm_debug_posted_intr(d, arg);
+
+        if ( current->domain != d )
+            domain_unpause(d);
+
+        break;
+
     default:
         return -ENOSYS;
     }
diff --git a/xen/arch/x86/hvm/svm/avic.c b/xen/arch/x86/hvm/svm/avic.c
index 4dcd2c94ea87..ca779be22eea 100644
--- a/xen/arch/x86/hvm/svm/avic.c
+++ b/xen/arch/x86/hvm/svm/avic.c
@@ -729,6 +729,81 @@ void cf_check svm_avic_hvm_deliver_posted_intr(struct vcpu *v, uint8_t vector)
     }
 }
 
+static void avic_domain_noaccel_state_sync(struct domain *d)
+{
+    struct svm_domain *sd = &d->arch.hvm.svm;
+    struct vcpu *v;
+
+    memset(sd->avic_logical_id_table, 0, PAGE_SIZE);
+
+    for_each_vcpu ( d, v )
+    {
+        struct svm_vcpu *sv = &v->arch.hvm.svm;
+        uint32_t ldr = vlapic_reg_read(v, APIC_LDR);
+        uint32_t dfr = vlapic_reg_read(v, APIC_DFR);
+        uint32_t id = GET_xAPIC_ID(vlapic_reg_read(v, APIC_ID));
+
+        sv->avic_ldr = ldr;
+        sv->avic_dfr = dfr;
+
+        avic_ldr_write(v, id, ldr);
+    }
+}
+
+static void avic_domain_enable(struct domain *d)
+{
+    struct vcpu *v;
+    int rc;
+
+    rc = avic_set_mmio_p2m_entry(d);
+    if ( rc )
+        goto out;
+
+    /* Re-load LDR/DFR from vLAPIC */
+    avic_domain_noaccel_state_sync(d);
+
+    for_each_vcpu ( d, v )
+    {
+        /* VINTR=0 */
+        svm_clear_intr_window(v);
+
+        /* VMCB.avic_enable=1 */
+        avic_vcpu_enable(v, true);
+    }
+
+    return;
+
+ out:
+    gdprintk(XENLOG_INFO,
+             "%pd: AVIC: failed to enable APIC MMIO access: %d\n",
+             d, rc);
+}
+
+static void avic_domain_disable(struct domain *d)
+{
+    struct vcpu *v;
+    int rc;
+
+    for_each_vcpu ( d, v )
+    {
+        /* IsRunning=0 */
+        avic_vcpu_clear_running(v);
+
+        /* VMCB.avic_enable=0 */
+        avic_vcpu_enable(v, false);
+
+        avic_stats_reset(v);
+    }
+
+    rc = avic_clear_mmio_p2m_entry(d);
+    if ( !rc )
+        return;
+
+    gdprintk(XENLOG_INFO,
+             "%pd: AVIC: failed to disable APIC MMIO access: %d\n",
+             d, rc);
+}
+
 void cf_check svm_avic_hvm_dump_posted_intr(const struct vcpu *v)
 {
     const struct svm_vcpu *sv = &v->arch.hvm.svm;
@@ -752,6 +827,63 @@ void cf_check svm_avic_hvm_dump_posted_intr(const struct vcpu *v)
            sv->avic_cnt_kick);
 }
 
+static void avic_domain_dump_stats(struct domain *d)
+{
+    struct vcpu *v;
+
+    for_each_vcpu ( d, v )
+    {
+        svm_avic_hvm_dump_posted_intr(v);
+        avic_stats_reset(v);
+    }
+}
+
+/* Interrupt acceleration control operations. */
+enum {
+    POSTED_INTR_DISABLE     = 0,
+    POSTED_INTR_ENABLE      = 1,
+    POSTED_INTR_DUMP_STATS  = 2,
+};
+
+void cf_check svm_avic_hvm_debug_posted_intr(struct domain *d,
+                                             unsigned int arg)
+{
+    struct vcpu *v;
+
+    /* Ensure no vCPU can VMRUN while updating VMCB for AVIC. */
+    for_each_vcpu ( d, v )
+    {
+        if ( !avic_vcpu_supported(v) )
+            goto out;
+
+        if ( v != current )
+            vcpu_pause(v);
+    }
+
+    switch ( arg ) {
+    case POSTED_INTR_DISABLE:
+        avic_domain_disable(d);
+        break;
+
+    case POSTED_INTR_ENABLE:
+        avic_domain_enable(d);
+        break;
+
+    case POSTED_INTR_DUMP_STATS:
+        avic_domain_dump_stats(d);
+        break;
+
+    default:
+        ASSERT_UNREACHABLE();
+        break;
+    }
+
+ out:
+    for_each_vcpu ( d, v )
+        if ( v != current )
+            vcpu_unpause(v);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/hvm/svm/avic.h b/xen/arch/x86/hvm/svm/avic.h
index 24d5fab59b29..e40aa77264ad 100644
--- a/xen/arch/x86/hvm/svm/avic.h
+++ b/xen/arch/x86/hvm/svm/avic.h
@@ -34,5 +34,7 @@ void svm_avic_vcpu_switch_from(struct vcpu *v);
 
 void cf_check svm_avic_hvm_deliver_posted_intr(struct vcpu *v, uint8_t vector);
 void cf_check svm_avic_hvm_dump_posted_intr(const struct vcpu *v);
+void cf_check svm_avic_hvm_debug_posted_intr(struct domain *d,
+                                             unsigned int arg);
 
 #endif /* __SVM_AVIC_H__ */
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 83977d411b0a..f2eb01799c63 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2520,6 +2520,7 @@ static void __init svm_avic_start(void)
 
     svm_function_table.deliver_posted_intr = svm_avic_hvm_deliver_posted_intr;
     svm_function_table.dump_posted_intr = svm_avic_hvm_dump_posted_intr;
+    svm_function_table.debug_posted_intr = svm_avic_hvm_debug_posted_intr;
 
     svm_function_table.caps.virtual_intr_delivery = true;
 }
diff --git a/xen/arch/x86/include/asm/hvm/hvm.h b/xen/arch/x86/include/asm/hvm/hvm.h
index 6efc533b35df..d44f136fae8f 100644
--- a/xen/arch/x86/include/asm/hvm/hvm.h
+++ b/xen/arch/x86/include/asm/hvm/hvm.h
@@ -218,6 +218,7 @@ struct hvm_function_table {
     void (*process_isr)(int isr, struct vcpu *v);
     void (*deliver_posted_intr)(struct vcpu *v, u8 vector);
     void (*dump_posted_intr)(const struct vcpu *v);
+    void (*debug_posted_intr)(struct domain *d, unsigned int arg);
     void (*sync_pir_to_irr)(struct vcpu *v);
     bool (*test_pir)(const struct vcpu *v, uint8_t vector);
     void (*handle_eoi)(uint8_t vector, int isr);
@@ -859,6 +860,16 @@ static inline void hvm_dump_posted_intr(const struct vcpu *v)
         alternative_vcall(hvm_funcs.dump_posted_intr, v);
 }
 
+static inline int hvm_debug_posted_intr(struct domain *d, unsigned int arg)
+{
+    if ( !hvm_funcs.debug_posted_intr )
+        return -ENOSYS;
+
+    alternative_vcall(hvm_funcs.debug_posted_intr, d, arg);
+
+    return 0;
+}
+
 #else  /* CONFIG_HVM */
 
 #define hvm_enabled false
@@ -971,6 +982,12 @@ static inline void hvm_dump_posted_intr(const struct vcpu *v)
     ASSERT_UNREACHABLE();
 }
 
+static inline int hvm_debug_posted_intr(struct domain *d, unsigned int arg)
+{
+    ASSERT_UNREACHABLE();
+    return -EOPNOTSUPP;
+}
+
 #define is_viridian_domain(d) ((void)(d), false)
 #define is_viridian_vcpu(v) ((void)(v), false)
 #define has_viridian_time_ref_count(d) ((void)(d), false)
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 546d9a8ac9c0..b72164f61ce2 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -765,6 +765,7 @@ struct xen_domctl_subscribe {
 /* XEN_DOMCTL_debug_op */
 #define XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_OFF         0
 #define XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_ON          1
+#define XEN_DOMCTL_DEBUG_OP_POSTED_INTR             3
 struct xen_domctl_debug_op {
     uint32_t op;   /* IN */
     uint32_t vcpu; /* IN */
-- 
2.55.0

[PATCH v1 22/28] x86/hvm: avic: ensure SVM AVIC supports xAPIC operation only

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

xAVIC does not support x2APIC operation (yet).

Until x2APIC mode is supported, don't advertise x2APIC to HVM
guests when SVM AVIC is enabled.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/cpu-policy.c          |  3 ++-
 xen/arch/x86/hvm/svm/svm.c         | 12 ++++++++++++
 xen/arch/x86/hvm/vmx/vmx.c         |  4 ++++
 xen/arch/x86/include/asm/hvm/hvm.h | 15 ++++++++++++++-
 4 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index 48a3185eed8d..ca9b1a523570 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -764,7 +764,8 @@ static void __init calculate_hvm_max_policy(void)
      * (x2)APIC isn't enabled.
      */
     __set_bit(X86_FEATURE_APIC, fs);
-    __set_bit(X86_FEATURE_X2APIC, fs);
+    if ( hvm_x2apic_supported() )
+        __set_bit(X86_FEATURE_X2APIC, fs);
 
     /*
      * We don't support EFER.LMSLE at all.  AMD has dropped the feature from
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index f2eb01799c63..667e8b05eecc 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2478,6 +2478,10 @@ static struct hvm_function_table __initdata_cf_clobber svm_function_table = {
     .tsc_scaling = {
         .max_ratio = ~TSC_RATIO_RSVD_BITS,
     },
+
+    .caps = {
+        .x2apic = true,
+    },
 };
 
 static void __init print_svm_features(void)
@@ -2523,6 +2527,14 @@ static void __init svm_avic_start(void)
     svm_function_table.debug_posted_intr = svm_avic_hvm_debug_posted_intr;
 
     svm_function_table.caps.virtual_intr_delivery = true;
+
+    /*
+     * xAVIC does not support x2APIC operation.
+     *
+     * Until x2AVIC/hybrid AVIC support is implemented, don't expose
+     * x2APIC to guests when SVM AVIC is enabled.
+     */
+    svm_function_table.caps.x2apic = false;
 }
 
 const struct hvm_function_table * __init start_svm(void)
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 3c7374a9ff42..bad43b38efea 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2916,6 +2916,10 @@ static struct hvm_function_table __initdata_cf_clobber vmx_function_table = {
     .tsc_scaling = {
         .max_ratio = VMX_TSC_MULTIPLIER_MAX,
     },
+
+    .caps = {
+        .x2apic = true,
+    },
 };
 
 /* Handle VT-d posted-interrupt when VCPU is blocked. */
diff --git a/xen/arch/x86/include/asm/hvm/hvm.h b/xen/arch/x86/include/asm/hvm/hvm.h
index d44f136fae8f..b701fc003ced 100644
--- a/xen/arch/x86/include/asm/hvm/hvm.h
+++ b/xen/arch/x86/include/asm/hvm/hvm.h
@@ -119,7 +119,9 @@ struct hvm_function_table {
              virtual_intr_delivery:1,
 
              /* Nested virt capabilities */
-             nested_virt:1;
+             nested_virt:1,
+
+             x2apic:1;
     } caps;
 
     /*
@@ -716,6 +718,11 @@ static inline bool hvm_nested_virt_supported(void)
     return hvm_funcs.caps.nested_virt;
 }
 
+static inline bool hvm_x2apic_supported(void)
+{
+    return hvm_funcs.caps.x2apic;
+}
+
 #ifdef CONFIG_ALTP2M
 /* updates the current hardware p2m */
 static inline void altp2m_vcpu_update_p2m(struct vcpu *v)
@@ -905,6 +912,12 @@ static inline bool hvm_nested_virt_supported(void)
     return false;
 }
 
+static inline bool hvm_x2apic_supported(void)
+{
+    ASSERT_UNREACHABLE();
+    return false;
+}
+
 static inline bool nhvm_vmcx_hap_enabled(const struct vcpu *v)
 {
     ASSERT_UNREACHABLE();
-- 
2.55.0

[PATCH v1 23/28] x86/hvm: avic: enable vLAPIC mode switch hook

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

xAVIC does not support x2APIC operation (yet).

Until x2APIC mode is supported, ensure that AVIC gets disabled
if guest OS attempts to switch vLAPIC to x2APIC mode.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/hvm/svm/avic.c | 17 +++++++++++++++++
 xen/arch/x86/hvm/svm/avic.h |  1 +
 xen/arch/x86/hvm/svm/svm.c  |  1 +
 3 files changed, 19 insertions(+)

diff --git a/xen/arch/x86/hvm/svm/avic.c b/xen/arch/x86/hvm/svm/avic.c
index ca779be22eea..37a857f28d14 100644
--- a/xen/arch/x86/hvm/svm/avic.c
+++ b/xen/arch/x86/hvm/svm/avic.c
@@ -827,6 +827,23 @@ void cf_check svm_avic_hvm_dump_posted_intr(const struct vcpu *v)
            sv->avic_cnt_kick);
 }
 
+void cf_check svm_avic_hvm_update_vlapic_mode(struct vcpu *v)
+{
+    const struct vlapic *vlapic = vcpu_vlapic(v);
+
+    if ( !svm_avic_vcpu_enabled(v) )
+        return;
+
+    if ( !vlapic_x2apic_mode(vlapic) )
+        return;
+
+    avic_domain_disable(v->domain);
+
+    gprintk(XENLOG_WARNING,
+            "%pv: AVIC: x2APIC mode is not supported; AVIC disabled\n",
+            v);
+}
+
 static void avic_domain_dump_stats(struct domain *d)
 {
     struct vcpu *v;
diff --git a/xen/arch/x86/hvm/svm/avic.h b/xen/arch/x86/hvm/svm/avic.h
index e40aa77264ad..61bfca7a9239 100644
--- a/xen/arch/x86/hvm/svm/avic.h
+++ b/xen/arch/x86/hvm/svm/avic.h
@@ -36,5 +36,6 @@ void cf_check svm_avic_hvm_deliver_posted_intr(struct vcpu *v, uint8_t vector);
 void cf_check svm_avic_hvm_dump_posted_intr(const struct vcpu *v);
 void cf_check svm_avic_hvm_debug_posted_intr(struct domain *d,
                                              unsigned int arg);
+void cf_check svm_avic_hvm_update_vlapic_mode(struct vcpu *v);
 
 #endif /* __SVM_AVIC_H__ */
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 667e8b05eecc..77f8278c598b 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2525,6 +2525,7 @@ static void __init svm_avic_start(void)
     svm_function_table.deliver_posted_intr = svm_avic_hvm_deliver_posted_intr;
     svm_function_table.dump_posted_intr = svm_avic_hvm_dump_posted_intr;
     svm_function_table.debug_posted_intr = svm_avic_hvm_debug_posted_intr;
+    svm_function_table.update_vlapic_mode = svm_avic_hvm_update_vlapic_mode;
 
     svm_function_table.caps.virtual_intr_delivery = true;
 
-- 
2.55.0

[PATCH v1 24/28] x86/hvm: avic: ensure SVM AVIC is disabled with nested guest

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

AVIC in nested virtualization environment is not supported as of yet.

Ensure there will be no problems in any production environments if AVIC
got enabled in nested guest by short-circuiting logic in
svm_avic_create_vmcb().

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/hvm/svm/avic.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/xen/arch/x86/hvm/svm/avic.c b/xen/arch/x86/hvm/svm/avic.c
index 37a857f28d14..e19489bd485c 100644
--- a/xen/arch/x86/hvm/svm/avic.c
+++ b/xen/arch/x86/hvm/svm/avic.c
@@ -4,6 +4,7 @@
  */
 
 #include <asm/hvm/emulate.h>
+#include <asm/hvm/nestedhvm.h>
 #include <asm/msr.h>
 #include <asm/p2m.h>
 #include <xen/param.h>
@@ -170,6 +171,14 @@ static bool avic_vcpu_supported(struct vcpu *v)
         return false;
     }
 
+    if ( nestedhvm_enabled(d) && nestedhvm_vcpu_in_guestmode(v) )
+    {
+        gprintk(XENLOG_WARNING,
+                "%pv: AVIC: nested guest is not supported\n",
+                v);
+        return false;
+    }
+
     id = GET_xAPIC_ID(vlapic_reg_read(v, APIC_ID));
     if ( id > AVIC_MAX_PHYSICAL_ID )
     {
-- 
2.55.0

[PATCH v1 25/28] x86/hvm: avic: account for AMD erratum 1235

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

Disable IPI virtualization for AMD Family 17h (Zen1 and Zen2) and
Hygon Family 18h (derived from AMD Zen1) CPUs due to erratum 1235,
which results in missed VM-Exits on the sender and thus missed wake
events for blocking vCPUs due to the CPU failing to see a software
update to clear 'IsRunning' bit.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/arch/x86/hvm/svm/svm.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 77f8278c598b..fd06ccfaab3f 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2536,6 +2536,21 @@ static void __init svm_avic_start(void)
      * x2APIC to guests when SVM AVIC is enabled.
      */
     svm_function_table.caps.x2apic = false;
+
+    /*
+     * Disable IPI virtualization for AMD Family 17h (Zen1 and Zen2) and
+     * Hygon Family 18h (derived from AMD Zen1) CPUs due to erratum 1235,
+     * which results in missed VM-Exits on the sender and thus missed wake
+     * events for blocking vCPUs due to the CPU failing to see a software
+     * update to clear 'IsRunning' bit.
+     */
+    if ( boot_cpu_data.x86 == 0x17 || boot_cpu_data.x86 == 0x18 )
+    {
+        svm_avic_features &= ~AVIC_HW_ISRUNNING;
+
+        printk(XENLOG_WARNING
+               "SVM: AVIC IPI acceleration disabled (erratum 1235)\n");
+    }
 }
 
 const struct hvm_function_table * __init start_svm(void)
-- 
2.55.0

[PATCH v1 26/28] docs: add coverage for svm_avic=

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

Document new command line switch to enable/disable SVM AVIC driver
in the hypervisor.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 docs/misc/xen-command-line.pandoc | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index a6f08463d177..e810f8836fe7 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -1623,7 +1623,7 @@ boolean (e.g. `iommu=no`) can override this and leave the IOMMUs disabled.
     This option depends on `intremap`, and is disabled by default due to some
     corner cases in the implementation which have yet to be resolved.
 
-    This option is only valid on x86, and only builds of Xen with HVM support.
+    This option is only valid on Intel x86, and only builds of Xen with HVM support.
 
 *   The `crash-disable` boolean controls disabling IOMMU functionality (DMAR/IR/QI)
     before switching to a crash kernel. This option is inactive by default and
@@ -2673,6 +2673,27 @@ On Sappire and Emerald Rapids CPUs with May 2025 microcode or later, the
 `ibpb-alt=` option can be used to switch to the alternative mitigation for
 Intel SA-00982.  Intel suggest that some workloads will benefit from this.
 
+### svm_avic (x86)
+> `= true | false | nodoorbell`
+
+This option configures Advanced Virtual Interrupt Controller (AVIC), which
+is an extension of AMD Secure Virtual Machine (SVM) to virtualize local APIC
+for guest VM.
+
+This parameter is experimental and for expert configuration only.
+
+Xen has mitigatations for the following AVIC hardware flaws:
+
+ * AMD Erratum 1235. A hardware flaw affecting certain AMD Family 17h and
+   Hygon Family 18h processors where the CPU hardware reads a stale
+   `IsRunning=1` bit during Interrupt Control Register (ICR) write emulation.
+
+Limitations:
+
+ * AVIC in x2APIC mode is not supported.
+
+ * AVIC in nested virtualization setup is not supported.
+
 ### sync_console
 > `= <boolean>`
 
-- 
2.55.0

[PATCH v1 27/28] CHANGELOG.md: add note on initial SVM AVIC support

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com>

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 CHANGELOG.md | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 225397f12876..f657efba6389 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
      permissions for the port range in question.
      XEN_DOMCTL_ioport_permission now needs invoking up front /
      afterwards.
+   - Initial SVM AVIC support to accelerate IPIs and posted interrupts on AMD
+     systems. AVIC with vLAPIC in x2APIC mode is not supported, GALog is not
+     supported.
 
 ### Added
 
-- 
2.55.0

Re: [PATCH v1 26/28] docs: add coverage for svm_avic=

[Jan Beulich]

On 05.11.2026 02:33, dmukhin@ford.com wrote:
> @@ -2673,6 +2673,27 @@ On Sappire and Emerald Rapids CPUs with May 2025 microcode or later, the
>  `ibpb-alt=` option can be used to switch to the alternative mitigation for
>  Intel SA-00982.  Intel suggest that some workloads will benefit from this.
>  
> +### svm_avic (x86)
> +> `= true | false | nodoorbell`
> +
> +This option configures Advanced Virtual Interrupt Controller (AVIC), which
> +is an extension of AMD Secure Virtual Machine (SVM) to virtualize local APIC
> +for guest VM.
> +
> +This parameter is experimental and for expert configuration only.
> +
> +Xen has mitigatations for the following AVIC hardware flaws:

Nit: mitigations

> + * AMD Erratum 1235. A hardware flaw affecting certain AMD Family 17h and
> +   Hygon Family 18h processors where the CPU hardware reads a stale
> +   `IsRunning=1` bit during Interrupt Control Register (ICR) write emulation.
> +
> +Limitations:
> +
> + * AVIC in x2APIC mode is not supported.
> +
> + * AVIC in nested virtualization setup is not supported.
> +
>  ### sync_console
>  > `= <boolean>`

This wants to be part of the patch where the option is introduced. New options
also want to use dashes in favor of underscores. Plus the default setting of
this option wants making explicit.

Jan

Re: [PATCH v1 12/28] x86: apic: use APIC_EIO_ACK instead of open-coded 0

[Jan Beulich]

On 05.11.2026 02:33, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com>
> 
> Use the symbolic value in ack_APIC_irq() instead of open-coded 0.
> 
> Also, correct the EIO -> EOI typo in the constant name.
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Looks like this is independent of the earlier 11 patches, and hence could go
in right away.

Jan

Re: [PATCH v1 12/28] x86: apic: use APIC_EIO_ACK instead of open-coded 0

[Jan Beulich]

On 05.11.2026 09:27, Jan Beulich wrote:
> On 05.11.2026 02:33, dmukhin@ford.com wrote:
>> From: Denis Mukhin <dmukhin@ford.com>
>>
>> Use the symbolic value in ack_APIC_irq() instead of open-coded 0.
>>
>> Also, correct the EIO -> EOI typo in the constant name.
>>
>> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>

Oh, actually: Only with the typo that you correct in the header also
corrected in the patch subject.

Jan

Re: [PATCH v1 13/28] x86/time: fix typo s/interruopt/interrupt/

[Jan Beulich]

On 05.11.2026 02:33, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com>
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH v1 09/28] x86/hvm: fix hvm_emulate_one_vm_event() definition

[Jan Beulich]

On 05.11.2026 02:33, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com>
> 
> Add empty definition for !CONFIG_VM_EVENT case.

Which is needed because of?

Jan

Re: [PATCH v1 14/28] x86/hvm: vlapic: update signatures of both vlapic_{clear,test}_*

[Jan Beulich]

On 05.11.2026 02:33, dmukhin@ford.com wrote:
> --- a/xen/arch/x86/hvm/vlapic.c
> +++ b/xen/arch/x86/hvm/vlapic.c
> @@ -85,14 +85,14 @@ static int vlapic_find_highest_vector(const void *bitmap)
>   * IRR-specific bitmap update & search routines.
>   */
>  
> -static int vlapic_test_and_set_irr(int vector, struct vlapic *vlapic)
> +static int vlapic_test_and_set_irr(struct vlapic *vlapic, uint8_t vec)
>  {
> -    return vlapic_test_and_set_vector(vector, &vlapic->regs->data[APIC_IRR]);
> +    return vlapic_test_and_set_vector(vec, &vlapic->regs->data[APIC_IRR]);
>  }
>  
> -static void vlapic_clear_irr(int vector, struct vlapic *vlapic)
> +static void vlapic_clear_irr(struct vlapic *vlapic, uint8_t vec)
>  {
> -    vlapic_clear_vector(vector, &vlapic->regs->data[APIC_IRR]);
> +    vlapic_clear_vector(vec, &vlapic->regs->data[APIC_IRR]);
>  }
>  
>  static int vlapic_find_highest_irr(struct vlapic *vlapic)

Each of the functions only having ...

> @@ -166,7 +166,7 @@ void vlapic_set_irq(struct vlapic *vlapic, uint8_t vec, uint8_t trig)
>  
>      if ( hvm_funcs.deliver_posted_intr )
>          alternative_vcall(hvm_funcs.deliver_posted_intr, target, vec);
> -    else if ( !vlapic_test_and_set_irr(vec, vlapic) )
> +    else if ( !vlapic_test_and_set_irr(vlapic, vec) )
>          vcpu_kick(target);
>  }
>  
> @@ -1414,7 +1414,7 @@ int vlapic_ack_pending_irq(struct vcpu *v, int vector, bool force_ack)
>           !viridian_synic_is_auto_eoi_sint(v, vector) )
>          vlapic_set_vector(vector, &vlapic->regs->data[APIC_ISR]);
>  
> -    vlapic_clear_irr(vector, vlapic);
> +    vlapic_clear_irr(vlapic, vector);
>  
>      return 1;
>  }

... a single caller: Do we actually need those helpers?

Jan

Re: [PATCH v1 12/28] x86: apic: use APIC_EIO_ACK instead of open-coded 0

[Andrew Cooper]

On 05/11/2026 1:33 am, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com>
>
> Use the symbolic value in ack_APIC_irq() instead of open-coded 0.
>
> Also, correct the EIO -> EOI typo in the constant name.
>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
>  xen/arch/x86/include/asm/apic.h    | 3 +--
>  xen/arch/x86/include/asm/apicdef.h | 2 +-
>  2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/x86/include/asm/apic.h b/xen/arch/x86/include/asm/apic.h
> index f30d57ad22c5..a4f6d2c26ce1 100644
> --- a/xen/arch/x86/include/asm/apic.h
> +++ b/xen/arch/x86/include/asm/apic.h
> @@ -155,8 +155,7 @@ void apic_wait_icr_idle(void);
>  
>  static inline void ack_APIC_irq(void)
>  {
> -	/* Docs say use 0 for future compatibility */
> -	apic_write(APIC_EOI, 0);
> +	apic_write(APIC_EOI, APIC_EOI_ACK);
>  }
>  
>  extern int get_maxlvt(void);
> diff --git a/xen/arch/x86/include/asm/apicdef.h b/xen/arch/x86/include/asm/apicdef.h
> index 112c1dc613a1..d3dc7b1d7643 100644
> --- a/xen/arch/x86/include/asm/apicdef.h
> +++ b/xen/arch/x86/include/asm/apicdef.h
> @@ -25,7 +25,7 @@
>  #define			APIC_ARBPRI_MASK	0xFF
>  #define		APIC_PROCPRI	0xA0
>  #define		APIC_EOI	0xB0
> -#define			APIC_EIO_ACK		0x0		/* Write this to the EOI register */
> +#define			APIC_EOI_ACK		0x0		/* Docs say use 0 for future compatibility */
>  #define		APIC_RRR	0xC0
>  #define		APIC_LDR	0xD0
>  #define			APIC_LDR_MASK		(0xFFu<<24)

Honestly, I'd prefer to delete this constant rather than to make use of
it.  The typo in the name shows it's not being used at all.

AMD have a feature called Selective EOI where you can write a vector in
and have it EOI something which isn't the currently in-service one.

~Andrew

Re: [PATCH v1 12/28] x86: apic: use APIC_EIO_ACK instead of open-coded 0

[Jan Beulich]

On 05.11.2026 11:20, Andrew Cooper wrote:
> On 05/11/2026 1:33 am, dmukhin@ford.com wrote:
>> --- a/xen/arch/x86/include/asm/apicdef.h
>> +++ b/xen/arch/x86/include/asm/apicdef.h
>> @@ -25,7 +25,7 @@
>>  #define			APIC_ARBPRI_MASK	0xFF
>>  #define		APIC_PROCPRI	0xA0
>>  #define		APIC_EOI	0xB0
>> -#define			APIC_EIO_ACK		0x0		/* Write this to the EOI register */
>> +#define			APIC_EOI_ACK		0x0		/* Docs say use 0 for future compatibility */
>>  #define		APIC_RRR	0xC0
>>  #define		APIC_LDR	0xD0
>>  #define			APIC_LDR_MASK		(0xFFu<<24)
> 
> Honestly, I'd prefer to delete this constant rather than to make use of
> it.  The typo in the name shows it's not being used at all.
> 
> AMD have a feature called Selective EOI where you can write a vector in
> and have it EOI something which isn't the currently in-service one.

Makes it even more not to use plain 0 for the present write, I'd say.

Jan

Re: [PATCH v1 24/28] x86/hvm: avic: ensure SVM AVIC is disabled with nested guest

[Ross Lagerwall]

> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> on behalf of dmukhin@ford.com <dmukhin@ford.com>
> Sent: Thursday, November 05, 2026 01:33
> To: xen-devel@lists.xenproject.org <xen-devel@lists.xenproject.org>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>; jason.andryuk@amd.com <jason.andryuk@amd.com>; jbeulich@suse.com <jbeulich@suse.com>; roger@xenproject.org <roger@xenproject.org>; teddy.astie@vates.tech <teddy.astie@vates.tech>; dmukhin@xen.org <dmukhin@xen.org>; Denis Mukhin <dmukhin@ford.com>
> Subject: [PATCH v1 24/28] x86/hvm: avic: ensure SVM AVIC is disabled with nested guest
>
>
> From: Denis Mukhin <dmukhin@ford.com>
>
> AVIC in nested virtualization environment is not supported as of yet.
>
> Ensure there will be no problems in any production environments if AVIC
> got enabled in nested guest by short-circuiting logic in
> svm_avic_create_vmcb().
>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
>  xen/arch/x86/hvm/svm/avic.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/xen/arch/x86/hvm/svm/avic.c b/xen/arch/x86/hvm/svm/avic.c
> index 37a857f28d14..e19489bd485c 100644
> --- a/xen/arch/x86/hvm/svm/avic.c
> +++ b/xen/arch/x86/hvm/svm/avic.c
> @@ -4,6 +4,7 @@
>   */
>  
>  #include <asm/hvm/emulate.h>
> +#include <asm/hvm/nestedhvm.h>
>  #include <asm/msr.h>
>  #include <asm/p2m.h>
>  #include <xen/param.h>
> @@ -170,6 +171,14 @@ static bool avic_vcpu_supported(struct vcpu *v)
>          return false;
>      }
>  
> +    if ( nestedhvm_enabled(d) && nestedhvm_vcpu_in_guestmode(v) )
> +    {
> +        gprintk(XENLOG_WARNING,
> +                "%pv: AVIC: nested guest is not supported\n",
> +                v);
> +        return false;
> +    }
> +
>      id = GET_xAPIC_ID(vlapic_reg_read(v, APIC_ID));
>      if ( id > AVIC_MAX_PHYSICAL_ID )
>      {

Does this actually work? This function is mostly called from
svm_vcpu_initialise() and the vCPU is never in guest mode there (i.e. L2).

You could probably just drop the second part of the if condition.

Ross

Re: [PATCH v1 12/28] x86: apic: use APIC_EIO_ACK instead of open-coded 0

[Andrew Cooper]

On 05/11/2026 10:21 am, Jan Beulich wrote:
> On 05.11.2026 11:20, Andrew Cooper wrote:
>> On 05/11/2026 1:33 am, dmukhin@ford.com wrote:
>>> --- a/xen/arch/x86/include/asm/apicdef.h
>>> +++ b/xen/arch/x86/include/asm/apicdef.h
>>> @@ -25,7 +25,7 @@
>>>  #define			APIC_ARBPRI_MASK	0xFF
>>>  #define		APIC_PROCPRI	0xA0
>>>  #define		APIC_EOI	0xB0
>>> -#define			APIC_EIO_ACK		0x0		/* Write this to the EOI register */
>>> +#define			APIC_EOI_ACK		0x0		/* Docs say use 0 for future compatibility */
>>>  #define		APIC_RRR	0xC0
>>>  #define		APIC_LDR	0xD0
>>>  #define			APIC_LDR_MASK		(0xFFu<<24)
>> Honestly, I'd prefer to delete this constant rather than to make use of
>> it.  The typo in the name shows it's not being used at all.
>>
>> AMD have a feature called Selective EOI where you can write a vector in
>> and have it EOI something which isn't the currently in-service one.
> Makes it even more not to use plain 0 for the present write, I'd say.

Nonsense.

This is introducing a layer of obfuscation which is unhelpful and, from
my point of view, firmly unwanted.

Everyone knows you write 0 to the EOI register.  It's been that way for
three decades, and it's not plausibly going to change.

The code used to be obviously correct at a glance, and now it requires
looking in a different header file.

~Andrew

Re: [PATCH v1 09/28] x86/hvm: fix hvm_emulate_one_vm_event() definition

[dmukhin]

On Thu, Nov 05, 2026 at 09:30:02AM +0100, Jan Beulich wrote:
> On 05.11.2026 02:33, dmukhin@ford.com wrote:
> > From: Denis Mukhin <dmukhin@ford.com>
> > 
> > Add empty definition for !CONFIG_VM_EVENT case.
> 
> Which is needed because of?

That is a fix for *-randconfig runs with AVIC series:
VMEXIT_AVIC_NOACCEL handlers can call hvm_emulate_one_vm_event() 

> 
> Jan
> 

Re: [PATCH v1 14/28] x86/hvm: vlapic: update signatures of both vlapic_{clear,test}_*

[dmukhin]

On Thu, Nov 05, 2026 at 09:33:17AM +0100, Jan Beulich wrote:
> On 05.11.2026 02:33, dmukhin@ford.com wrote:
> > --- a/xen/arch/x86/hvm/vlapic.c
> > +++ b/xen/arch/x86/hvm/vlapic.c
> > @@ -85,14 +85,14 @@ static int vlapic_find_highest_vector(const void *bitmap)
> >   * IRR-specific bitmap update & search routines.
> >   */
> >  
> > -static int vlapic_test_and_set_irr(int vector, struct vlapic *vlapic)
> > +static int vlapic_test_and_set_irr(struct vlapic *vlapic, uint8_t vec)
> >  {
> > -    return vlapic_test_and_set_vector(vector, &vlapic->regs->data[APIC_IRR]);
> > +    return vlapic_test_and_set_vector(vec, &vlapic->regs->data[APIC_IRR]);
> >  }
> >  
> > -static void vlapic_clear_irr(int vector, struct vlapic *vlapic)
> > +static void vlapic_clear_irr(struct vlapic *vlapic, uint8_t vec)
> >  {
> > -    vlapic_clear_vector(vector, &vlapic->regs->data[APIC_IRR]);
> > +    vlapic_clear_vector(vec, &vlapic->regs->data[APIC_IRR]);
> >  }
> >  
> >  static int vlapic_find_highest_irr(struct vlapic *vlapic)
> 
> Each of the functions only having ...
> 
> > @@ -166,7 +166,7 @@ void vlapic_set_irq(struct vlapic *vlapic, uint8_t vec, uint8_t trig)
> >  
> >      if ( hvm_funcs.deliver_posted_intr )
> >          alternative_vcall(hvm_funcs.deliver_posted_intr, target, vec);
> > -    else if ( !vlapic_test_and_set_irr(vec, vlapic) )
> > +    else if ( !vlapic_test_and_set_irr(vlapic, vec) )
> >          vcpu_kick(target);
> >  }
> >  
> > @@ -1414,7 +1414,7 @@ int vlapic_ack_pending_irq(struct vcpu *v, int vector, bool force_ack)
> >           !viridian_synic_is_auto_eoi_sint(v, vector) )
> >          vlapic_set_vector(vector, &vlapic->regs->data[APIC_ISR]);
> >  
> > -    vlapic_clear_irr(vector, vlapic);
> > +    vlapic_clear_irr(vlapic, vector);
> >  
> >      return 1;
> >  }
> 
> ... a single caller: Do we actually need those helpers?

Yeah, I would rather drop these...
Thanks for suggestion!

> 
> Jan
> 

Re: [PATCH v1 26/28] docs: add coverage for svm_avic=

[dmukhin]

On Thu, Nov 05, 2026 at 09:24:09AM +0100, Jan Beulich wrote:
> On 05.11.2026 02:33, dmukhin@ford.com wrote:
> > @@ -2673,6 +2673,27 @@ On Sappire and Emerald Rapids CPUs with May 2025 microcode or later, the
> >  `ibpb-alt=` option can be used to switch to the alternative mitigation for
> >  Intel SA-00982.  Intel suggest that some workloads will benefit from this.
> >  
> > +### svm_avic (x86)
> > +> `= true | false | nodoorbell`
> > +
> > +This option configures Advanced Virtual Interrupt Controller (AVIC), which
> > +is an extension of AMD Secure Virtual Machine (SVM) to virtualize local APIC
> > +for guest VM.
> > +
> > +This parameter is experimental and for expert configuration only.
> > +
> > +Xen has mitigatations for the following AVIC hardware flaws:
> 
> Nit: mitigations
> 
> > + * AMD Erratum 1235. A hardware flaw affecting certain AMD Family 17h and
> > +   Hygon Family 18h processors where the CPU hardware reads a stale
> > +   `IsRunning=1` bit during Interrupt Control Register (ICR) write emulation.
> > +
> > +Limitations:
> > +
> > + * AVIC in x2APIC mode is not supported.
> > +
> > + * AVIC in nested virtualization setup is not supported.
> > +
> >  ### sync_console
> >  > `= <boolean>`
> 
> This wants to be part of the patch where the option is introduced. New options

I introduced 'svm_avic=nodoorbell' later in the series, after 
svm_avic= appears for the first time. So my thinking was to document
all new options in one patch...

> also want to use dashes in favor of underscores. Plus the default setting of
> this option wants making explicit.

Will switch to dashes.

> 
> Jan
> 

Re: [PATCH v1 01/28] x86/hvm: move SVM feature printout to a separate function

[Jason Andryuk]

On 2026-11-04 20:33, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com>
> 
> Isolate long SVM feature list printout in a separate function to
> improve readability.
> 
> Not a functional change.
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

Re: [PATCH v1 02/28] x86/hvm: report SVM AVIC support

[Jason Andryuk]

On 2026-11-04 20:33, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com>
> 
> Report SVM AVIC support from CPUID Fn8000000A_EDX[AVIC](bit 13).
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

Re: [PATCH v1 03/28] x86/hvm: report SVM x2AVIC support

[Jason Andryuk]

On 2026-11-04 20:33, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com>
> 
> Report SVM AVIC support from CPUID Fn8000000A_EDX[X2AVIC](bit 18).
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
>   xen/arch/x86/hvm/svm/svm.c         | 1 +
>   xen/arch/x86/include/asm/hvm/svm.h | 2 ++
>   2 files changed, 3 insertions(+)
> 
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index cf008a8acdf2..db81dad4b681 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -2457,6 +2457,7 @@ static void __init print_svm_features(void)
>       P(cpu_has_svm_vnmi, "Virtual NMI");
>       P(cpu_has_svm_bus_lock, "Bus Lock Filter");
>       P(cpu_has_svm_avic, "Advanced Virtual Interrupt Controller");
> +    P(cpu_has_svm_x2avic, "x2 Advanced Virtual Interrupt Controller");

Sorry, this ...

>   #undef P
>   
>       if ( !printed )
> diff --git a/xen/arch/x86/include/asm/hvm/svm.h b/xen/arch/x86/include/asm/hvm/svm.h
> index 0fc33f41b09d..292a9ee19b11 100644
> --- a/xen/arch/x86/include/asm/hvm/svm.h
> +++ b/xen/arch/x86/include/asm/hvm/svm.h
> @@ -36,6 +36,7 @@ extern u32 svm_feature_flags;
>   #define SVM_FEATURE_AVIC          13 /* Advanced Virtual Interrupt Controller */
>   #define SVM_FEATURE_VLOADSAVE     15 /* virtual vmload/vmsave */
>   #define SVM_FEATURE_VGIF          16 /* Virtual GIF */
> +#define SVM_FEATURE_X2AVIC        18 /* x2 Advanced Virtual Interrupt Controller */
>   #define SVM_FEATURE_SSS           19 /* NPT Supervisor Shadow Stacks */
>   #define SVM_FEATURE_SPEC_CTRL     20 /* MSR_SPEC_CTRL virtualisation */
>   #define SVM_FEATURE_VNMI          25 /* Virtual NMI */
> @@ -57,6 +58,7 @@ static inline bool cpu_has_svm_feature(unsigned int feat)
>   #define cpu_has_pause_thresh  cpu_has_svm_feature(SVM_FEATURE_PAUSETHRESH)
>   #define cpu_has_tsc_ratio     cpu_has_svm_feature(SVM_FEATURE_TSCRATEMSR)
>   #define cpu_has_svm_avic      cpu_has_svm_feature(SVM_FEATURE_AVIC)
> +#define cpu_has_svm_x2avic    cpu_has_svm_feature(SVM_FEATURE_X2AVIC)

... and this is mostly ordered by feature number.  Maybe add an pre-req 
patch to sort them, and then re-do patch 2 and this patch to insert in 
feature order?

Re-ordered (for both patches):

Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

Thanks,
Jason

Re: [PATCH v1 04/28] x86/hvm: avic: add domain state initialization

[Jason Andryuk]

On 2026-11-04 20:33, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com>
> 
> Establish per-domain SVM AVIC backing page mapping and SPA mappings
> for APIC physical/logical ID tables. Use MEMF_no_refcount mappings
> similarly to Intel's VMX APICv setup.
> 
> Gate per-domain AVIC state initialization behind 'svm_avic=' command
> line option.

AIUI, "-" is preferred, so 'svm-avic='.

> +
> +/*
> + * Enable guest accesses to APIC_DEFAULT_PHYS_BASE.
> + *
> + * AVIC walks the nested page table to check access permissions, but
> + * ignores the SPA from the leaf entry and instead uses the address in
> + * VMCB.avic_backing_page.
> + *
> + * Install a mapping so APIC accesses don't trap to Xen while xAPIC is
> + * accelerated.

Maybe:
"""
Install a mapping to the shared avic_acpi_access_page.  This populates 
the nested page tables, so APIC accesses don't trap into Xen.
"""

With those two changes:

Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

Re: [PATCH v1 04/28] x86/hvm: avic: add domain state initialization

[Jason Andryuk]

On 2026-11-05 18:10, Jason Andryuk wrote:
> On 2026-11-04 20:33, dmukhin@ford.com wrote:
>> From: Denis Mukhin <dmukhin@ford.com>
>>
>> Establish per-domain SVM AVIC backing page mapping and SPA mappings
>> for APIC physical/logical ID tables. Use MEMF_no_refcount mappings
>> similarly to Intel's VMX APICv setup.
>>
>> Gate per-domain AVIC state initialization behind 'svm_avic=' command
>> line option.
> 
> AIUI, "-" is preferred, so 'svm-avic='.
> 
>> +
>> +/*
>> + * Enable guest accesses to APIC_DEFAULT_PHYS_BASE.
>> + *
>> + * AVIC walks the nested page table to check access permissions, but
>> + * ignores the SPA from the leaf entry and instead uses the address in
>> + * VMCB.avic_backing_page.
>> + *
>> + * Install a mapping so APIC accesses don't trap to Xen while xAPIC is
>> + * accelerated.

Now I have a question.  Why don't we eventually just use 
VMCB.avic_backing_page itself?

Regards,
Jason

Re: [PATCH v1 03/28] x86/hvm: report SVM x2AVIC support

[dmukhin]

On Thu, Nov 05, 2026 at 05:13:50PM -0500, Jason Andryuk wrote:
> On 2026-11-04 20:33, dmukhin@ford.com wrote:
> > From: Denis Mukhin <dmukhin@ford.com>
> > 
> > Report SVM AVIC support from CPUID Fn8000000A_EDX[X2AVIC](bit 18).
> > 
> > Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> > ---
> >   xen/arch/x86/hvm/svm/svm.c         | 1 +
> >   xen/arch/x86/include/asm/hvm/svm.h | 2 ++
> >   2 files changed, 3 insertions(+)
> > 
> > diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> > index cf008a8acdf2..db81dad4b681 100644
> > --- a/xen/arch/x86/hvm/svm/svm.c
> > +++ b/xen/arch/x86/hvm/svm/svm.c
> > @@ -2457,6 +2457,7 @@ static void __init print_svm_features(void)
> >       P(cpu_has_svm_vnmi, "Virtual NMI");
> >       P(cpu_has_svm_bus_lock, "Bus Lock Filter");
> >       P(cpu_has_svm_avic, "Advanced Virtual Interrupt Controller");
> > +    P(cpu_has_svm_x2avic, "x2 Advanced Virtual Interrupt Controller");
> 
> Sorry, this ...
> 
> >   #undef P
> >       if ( !printed )
> > diff --git a/xen/arch/x86/include/asm/hvm/svm.h b/xen/arch/x86/include/asm/hvm/svm.h
> > index 0fc33f41b09d..292a9ee19b11 100644
> > --- a/xen/arch/x86/include/asm/hvm/svm.h
> > +++ b/xen/arch/x86/include/asm/hvm/svm.h
> > @@ -36,6 +36,7 @@ extern u32 svm_feature_flags;
> >   #define SVM_FEATURE_AVIC          13 /* Advanced Virtual Interrupt Controller */
> >   #define SVM_FEATURE_VLOADSAVE     15 /* virtual vmload/vmsave */
> >   #define SVM_FEATURE_VGIF          16 /* Virtual GIF */
> > +#define SVM_FEATURE_X2AVIC        18 /* x2 Advanced Virtual Interrupt Controller */
> >   #define SVM_FEATURE_SSS           19 /* NPT Supervisor Shadow Stacks */
> >   #define SVM_FEATURE_SPEC_CTRL     20 /* MSR_SPEC_CTRL virtualisation */
> >   #define SVM_FEATURE_VNMI          25 /* Virtual NMI */
> > @@ -57,6 +58,7 @@ static inline bool cpu_has_svm_feature(unsigned int feat)
> >   #define cpu_has_pause_thresh  cpu_has_svm_feature(SVM_FEATURE_PAUSETHRESH)
> >   #define cpu_has_tsc_ratio     cpu_has_svm_feature(SVM_FEATURE_TSCRATEMSR)
> >   #define cpu_has_svm_avic      cpu_has_svm_feature(SVM_FEATURE_AVIC)
> > +#define cpu_has_svm_x2avic    cpu_has_svm_feature(SVM_FEATURE_X2AVIC)
> 
> ... and this is mostly ordered by feature number.  Maybe add an pre-req
> patch to sort them, and then re-do patch 2 and this patch to insert in
> feature order?

Will do.

> 
> Re-ordered (for both patches):
> 
> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
> 
> Thanks,
> Jason
> 

Re: [PATCH v1 26/28] docs: add coverage for svm_avic=

[Jan Beulich]

On 05.11.2026 21:22, dmukhin@ford.com wrote:
> On Thu, Nov 05, 2026 at 09:24:09AM +0100, Jan Beulich wrote:
>> On 05.11.2026 02:33, dmukhin@ford.com wrote:
>>> @@ -2673,6 +2673,27 @@ On Sappire and Emerald Rapids CPUs with May 2025 microcode or later, the
>>>  `ibpb-alt=` option can be used to switch to the alternative mitigation for
>>>  Intel SA-00982.  Intel suggest that some workloads will benefit from this.
>>>  
>>> +### svm_avic (x86)
>>> +> `= true | false | nodoorbell`
>>> +
>>> +This option configures Advanced Virtual Interrupt Controller (AVIC), which
>>> +is an extension of AMD Secure Virtual Machine (SVM) to virtualize local APIC
>>> +for guest VM.
>>> +
>>> +This parameter is experimental and for expert configuration only.
>>> +
>>> +Xen has mitigatations for the following AVIC hardware flaws:
>>
>> Nit: mitigations
>>
>>> + * AMD Erratum 1235. A hardware flaw affecting certain AMD Family 17h and
>>> +   Hygon Family 18h processors where the CPU hardware reads a stale
>>> +   `IsRunning=1` bit during Interrupt Control Register (ICR) write emulation.
>>> +
>>> +Limitations:
>>> +
>>> + * AVIC in x2APIC mode is not supported.
>>> +
>>> + * AVIC in nested virtualization setup is not supported.
>>> +
>>>  ### sync_console
>>>  > `= <boolean>`
>>
>> This wants to be part of the patch where the option is introduced. New options
> 
> I introduced 'svm_avic=nodoorbell' later in the series, after 
> svm_avic= appears for the first time. So my thinking was to document
> all new options in one patch...

Please do this as (sub)options are being added.

Jan

Re: [PATCH v1 09/28] x86/hvm: fix hvm_emulate_one_vm_event() definition

[Jan Beulich]

On 05.11.2026 21:10, dmukhin@ford.com wrote:
> On Thu, Nov 05, 2026 at 09:30:02AM +0100, Jan Beulich wrote:
>> On 05.11.2026 02:33, dmukhin@ford.com wrote:
>>> From: Denis Mukhin <dmukhin@ford.com>
>>>
>>> Add empty definition for !CONFIG_VM_EVENT case.
>>
>> Which is needed because of?
> 
> That is a fix for *-randconfig runs with AVIC series:
> VMEXIT_AVIC_NOACCEL handlers can call hvm_emulate_one_vm_event() 

Then I think the stub wants introducing in the patch where it's needed.
That'll allow better judgement as to possible alternatives.

Jan

Re: [PATCH v1 03/28] x86/hvm: report SVM x2AVIC support

[Teddy Astie]

[-- Attachment #1.1: Type: text/plain, Size: 2549 bytes --]

Le 05/11/2026 à 02:37, dmukhin@ford.com a écrit :
> From: Denis Mukhin <dmukhin@ford.com>
> 
> Report SVM AVIC support from CPUID Fn8000000A_EDX[X2AVIC](bit 18).
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
>   xen/arch/x86/hvm/svm/svm.c         | 1 +
>   xen/arch/x86/include/asm/hvm/svm.h | 2 ++
>   2 files changed, 3 insertions(+)
> 
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index cf008a8acdf2..db81dad4b681 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -2457,6 +2457,7 @@ static void __init print_svm_features(void)
>       P(cpu_has_svm_vnmi, "Virtual NMI");
>       P(cpu_has_svm_bus_lock, "Bus Lock Filter");
>       P(cpu_has_svm_avic, "Advanced Virtual Interrupt Controller");
> +    P(cpu_has_svm_x2avic, "x2 Advanced Virtual Interrupt Controller");
>   #undef P
>   

"x2 Advanced Virtual Interrupt Controller" looks a bit weird to me.

What about "x2APIC AVIC" ?
(and at the same time, adding "(AVIC)" to the general AVIC one, like 
what is done for NPT being named "Nested Page Tables (NPT)")

>       if ( !printed )
> diff --git a/xen/arch/x86/include/asm/hvm/svm.h b/xen/arch/x86/include/asm/hvm/svm.h
> index 0fc33f41b09d..292a9ee19b11 100644
> --- a/xen/arch/x86/include/asm/hvm/svm.h
> +++ b/xen/arch/x86/include/asm/hvm/svm.h
> @@ -36,6 +36,7 @@ extern u32 svm_feature_flags;
>   #define SVM_FEATURE_AVIC          13 /* Advanced Virtual Interrupt Controller */
>   #define SVM_FEATURE_VLOADSAVE     15 /* virtual vmload/vmsave */
>   #define SVM_FEATURE_VGIF          16 /* Virtual GIF */
> +#define SVM_FEATURE_X2AVIC        18 /* x2 Advanced Virtual Interrupt Controller */
>   #define SVM_FEATURE_SSS           19 /* NPT Supervisor Shadow Stacks */
>   #define SVM_FEATURE_SPEC_CTRL     20 /* MSR_SPEC_CTRL virtualisation */
>   #define SVM_FEATURE_VNMI          25 /* Virtual NMI */
> @@ -57,6 +58,7 @@ static inline bool cpu_has_svm_feature(unsigned int feat)
>   #define cpu_has_pause_thresh  cpu_has_svm_feature(SVM_FEATURE_PAUSETHRESH)
>   #define cpu_has_tsc_ratio     cpu_has_svm_feature(SVM_FEATURE_TSCRATEMSR)
>   #define cpu_has_svm_avic      cpu_has_svm_feature(SVM_FEATURE_AVIC)
> +#define cpu_has_svm_x2avic    cpu_has_svm_feature(SVM_FEATURE_X2AVIC)
>   #define cpu_has_svm_vloadsave cpu_has_svm_feature(SVM_FEATURE_VLOADSAVE)
>   #define cpu_has_svm_sss       cpu_has_svm_feature(SVM_FEATURE_SSS)
>   #define cpu_has_svm_spec_ctrl cpu_has_svm_feature(SVM_FEATURE_SPEC_CTRL)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]

Re: [PATCH v1 11/28] x86/hvm: introduce vcpu_is_vlapic_tpr_emulated()

[Andrew Cooper]

On 05/11/2026 1:33 am, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com>
>
> Introduce vcpu_is_vlapic_tpr_emulated() in genetic SVM code to account
> for the case when platform provides accelerated access to vLAPIC's TPR.
>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
>  xen/arch/x86/hvm/svm/svm.c | 38 +++++++++++++++++++++++++++-----------
>  1 file changed, 27 insertions(+), 11 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index 673c818a0039..14a86ca4ff8f 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -925,6 +925,19 @@ static void cf_check svm_ctxt_switch_to(struct vcpu *v)
>      }
>  }
>  
> +static bool vcpu_is_vlapic_tpr_emulated(const struct vcpu *v, bool force_noaccel)
> +{
> +    const struct vlapic *vlapic = vcpu_vlapic(v);
> +
> +    if ( force_noaccel )
> +        return false;
> +
> +    if ( vlapic_hw_disabled(vlapic) )
> +        return false;
> +
> +    return true;
> +}
> +
>  static void noreturn cf_check svm_do_resume(void)
>  {
>      struct vcpu *v = current;
> @@ -933,7 +946,6 @@ static void noreturn cf_check svm_do_resume(void)
>                          v->domain->arch.monitor.software_breakpoint_enabled ||
>                          v->domain->arch.monitor.debug_exception_enabled);
>      bool vcpu_guestmode = false;
> -    struct vlapic *vlapic = vcpu_vlapic(v);
>  
>      if ( nestedhvm_enabled(v->domain) && nestedhvm_vcpu_in_guestmode(v) )
>          vcpu_guestmode = 1;
> @@ -959,8 +971,9 @@ static void noreturn cf_check svm_do_resume(void)
>          vmcb->cleanbits.raw = 0;
>      }
>  
> -    if ( !vcpu_guestmode && !vlapic_hw_disabled(vlapic) )
> +    if ( vcpu_is_vlapic_tpr_emulated(v, vcpu_guestmode) )
>      {
> +        struct vlapic *vlapic = vcpu_vlapic(v);
>          vintr_t intr;
>  
>          /* Reflect the vlapic's TPR in the hardware vtpr */
> @@ -2538,7 +2551,6 @@ void asmlinkage svm_vmexit_handler(void)
>      int insn_len;
>      vintr_t intr;
>      bool vcpu_guestmode = false;
> -    struct vlapic *vlapic = vcpu_vlapic(v);
>  
>      regs->rax = vmcb->rax;
>      regs->rip = vmcb->rip;
> @@ -2565,8 +2577,10 @@ void asmlinkage svm_vmexit_handler(void)
>       * NB. We need to preserve the low bits of the TPR to make checked builds
>       * of Windows work, even though they don't actually do anything.
>       */
> -    if ( !vcpu_guestmode && !vlapic_hw_disabled(vlapic) )
> +    if ( vcpu_is_vlapic_tpr_emulated(v, vcpu_guestmode) )
>      {
> +        struct vlapic *vlapic = vcpu_vlapic(v);
> +
>          intr = vmcb_get_vintr(vmcb);
>          vlapic_set_reg(vlapic, APIC_TASKPRI,
>                     ((intr.fields.tpr & 0x0F) << 4) |
> @@ -3097,14 +3111,16 @@ void asmlinkage svm_vmexit_handler(void)
>      }
>  
>    out:
> -    if ( vcpu_guestmode || vlapic_hw_disabled(vlapic) )
> -        return;
> +    if ( vcpu_is_vlapic_tpr_emulated(v, vcpu_guestmode ) )
> +    {
> +        struct vlapic *vlapic = vcpu_vlapic(v);
>  
> -    /* The exit may have updated the TPR: reflect this in the hardware vtpr */
> -    intr = vmcb_get_vintr(vmcb);
> -    intr.fields.tpr =
> -        (vlapic_get_reg(vlapic, APIC_TASKPRI) & 0xFF) >> 4;
> -    vmcb_set_vintr(vmcb, intr);
> +        /* The exit may have updated the TPR: reflect this in the hardware vtpr */
> +        intr = vmcb_get_vintr(vmcb);
> +        intr.fields.tpr =
> +            (vlapic_get_reg(vlapic, APIC_TASKPRI) & 0xFF) >> 4;
> +        vmcb_set_vintr(vmcb, intr);
> +    }

There are a large number of bugs here; they're prexisting, but this
isn't helping.

First, these final two hunks are wonky.  They're necessary, but only
because of bugs elsewhere in Xen.

In order to explain, I need to cover one of the bigger sins of the AMD64
architecture, CR8.  See APM Vol2 8.5.2 External Interrupt Priorities.

Windows (at least used to) write APIC_TPR very frequently.  Microsoft
made demands of the hardware vendors, and AMD made CR8.  AMD invented a
register called TPR which is an architectural abstraction of APIC_TPR,
and defined it's access to be via CR8.  They even made LOCK MOV CR8 (an
otherwise illegal encoding) be an alias of MOV CR8, for 32bit software
to use.

AMD state that it is undefined how TPR and APIC_TPR interact.  Intel
stated that CR8 maps directly on to APIC_TPR, and that the APIC must be
enabled for this mechanism to work.  AMD do have storage for TPR which
is separate to APIC_TPR (this has been confirmed), and as a result it
must be possible to desynchronise TPR and APIC_TPR when the APIC is
disabled.  Intel's wording suggests that they do not have storage, and
"SDM Vol2 13.8.6.1 Interaction of Task Priorities between CR8 and APIC"
hints specifically that only writes are reflected meaning that reads may
not be.


Since the outset, SVM had accelerated CR8.  Furthermore, because the
V_INTR_MASKING controls both the host/guest EFLAGS.IF split *and* CR8
virtualisation, we can't run VMs without accelerated CR8.  V_TPR is the
storage.

This final hunk exists because we have no path from the vLAPIC code
wiring writes of APIC_TPR back into the V_TPR field.  But, it has the
side effect of always clearing the TPR cleanbit, and exists only to
cover an operation where both manuals say "don't mix and match using CR8
or APIC_TPR;  use one or the other".

When AVIC is active, the V_TPR field ceases to exist and is now mapped
directly on to the APIC backing page, without intermediate storage.


From the Intel side, original VT-x had no acceleration here, but one of
the very early features they added was virtualisation of TPR accesses. 
This covers both CR8 and APIC_TPR, and they both map onto the APIC
backing page without intermediate storage either.


So, the next steps (probably as a prereq to this series) are:

1) Determine the actual behaviour of CR8 vs APIC_TPR on real hardware,
including with the APIC being disabled.  It may well be different
between Intel and AMD
2) Use 1 to fix the synchronisation of V_TPR and APIC_TPR so we're not
writing V_TPR on every VMExit
3) Testing in place to confirm we don't regress.  I should dust off my
ESR test as a starting point.

In particular I note that disabling the APIC needs to disable all parts
of APIC acceleration, and that the Intel vmx_vlapic_msr_changed() hook
doesn't do this correctly for TPR.  I also observe there's a major
difference between the Intel hook and what you add in this series,
insofar as the x2APIC MSR intercepts don't get altered, which is suspicious.

Thanks,

~Andrew

Re: [PATCH v1 04/28] x86/hvm: avic: add domain state initialization

[Teddy Astie]

[-- Attachment #1.1: Type: text/plain, Size: 10036 bytes --]

Le 05/11/2026 à 02:37, dmukhin@ford.com a écrit :
> From: Denis Mukhin <dmukhin@ford.com>
> 
> Establish per-domain SVM AVIC backing page mapping and SPA mappings
> for APIC physical/logical ID tables. Use MEMF_no_refcount mappings
> similarly to Intel's VMX APICv setup.
> 
> Gate per-domain AVIC state initialization behind 'svm_avic=' command
> line option.
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
>   xen/arch/x86/hvm/svm/Makefile            |   1 +
>   xen/arch/x86/hvm/svm/avic.c              | 136 +++++++++++++++++++++++
>   xen/arch/x86/hvm/svm/avic.h              |  14 +++
>   xen/arch/x86/hvm/svm/svm.c               |  20 ++++
>   xen/arch/x86/include/asm/hvm/svm-types.h |   8 ++
>   5 files changed, 179 insertions(+)
>   create mode 100644 xen/arch/x86/hvm/svm/avic.c
>   create mode 100644 xen/arch/x86/hvm/svm/avic.h
> 
> diff --git a/xen/arch/x86/hvm/svm/Makefile b/xen/arch/x86/hvm/svm/Makefile
> index 8a072cafd572..7dd16e32a77d 100644
> --- a/xen/arch/x86/hvm/svm/Makefile
> +++ b/xen/arch/x86/hvm/svm/Makefile
> @@ -1,4 +1,5 @@
>   obj-y += asid.o
> +obj-y += avic.o
>   obj-y += emulate.o
>   obj-bin-y += entry.o
>   obj-y += intr.o
> diff --git a/xen/arch/x86/hvm/svm/avic.c b/xen/arch/x86/hvm/svm/avic.c
> new file mode 100644
> index 000000000000..2c0e113ae34e
> --- /dev/null
> +++ b/xen/arch/x86/hvm/svm/avic.c
> @@ -0,0 +1,136 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * AMD Advanced Virtual Interrupt Controller (AVIC)
> + */
> +
> +#include <asm/hvm/emulate.h>
> +#include <asm/msr.h>
> +#include <asm/p2m.h>
> +#include <xen/param.h>
> +
> +#include "avic.h"
> +
> +bool __read_mostly svm_avic_enable;
> +

We probably want it to be __ro_after_init, I don't think anything good 
can happen if it's toggled dynamically.

> +static const char __section(".bss.page_aligned.const") __aligned(PAGE_SIZE)
> +    avic_apic_access_page[PAGE_SIZE];
> +

I'm a bit dubious in making this const.

I also think guests can find a way to read/write to it (as it's mapped 
as MMIO direct in p2m), which could be a problem regardless of const-ness.

I guess we're supposed to map the vAPIC backing page into the guest 
(instead of a stub page), even if it's not the page used by AVIC hardware.

 > System software is responsible for setting up a translation in the 
nested page table granting guest read and write permissions for accesses 
to the vAPIC Backing Page in SPA space. AVIC hardware walks the nested 
page table to check permissions, but does not use the SPA address 
specified in the leaf page table entry. Instead, AVIC hardware finds 
this address in the AVIC_BACKING_PAGE pointer field of the VMCB.

> +static int cf_check avic_parse_cmdline(const char *s)
> +{
> +    int rc;
> +
> +    rc = parse_bool(s, NULL);
> +    if ( rc != -1 )
> +    {
> +        svm_avic_enable = !!rc;
> +    }
> +    else if ( *s == '\0' )
> +    {
> +        svm_avic_enable = true;
> +    }
> +    else
> +    {
> +        return -EINVAL;
> +    }
> +
> +    return 0;
> +}
> +custom_param("svm_avic", avic_parse_cmdline);
> +
> +/*
> + * Enable guest accesses to APIC_DEFAULT_PHYS_BASE.
> + *
> + * AVIC walks the nested page table to check access permissions, but
> + * ignores the SPA from the leaf entry and instead uses the address in
> + * VMCB.avic_backing_page.
> + *
> + * Install a mapping so APIC accesses don't trap to Xen while xAPIC is
> + * accelerated.
> + */
> +static int avic_set_mmio_p2m_entry(struct domain *d)
> +{
> +    gfn_t gfn = _gfn(paddr_to_pfn(APIC_DEFAULT_PHYS_BASE));

_gfn(paddr_to_pfn(x)) can be simplified as gaddr_to_gfn(x)

> +    mfn_t mfn = _mfn(virt_to_mfn(avic_apic_access_page));
> +
> +    return map_mmio_regions(d, gfn, 1, mfn);
> +}
> +
> +static int avic_clear_mmio_p2m_entry(struct domain *d)
> +{
> +    gfn_t gfn = _gfn(paddr_to_pfn(APIC_DEFAULT_PHYS_BASE));
> +    mfn_t mfn = _mfn(virt_to_mfn(avic_apic_access_page));
> +
> +    return unmap_mmio_regions(d, gfn, 1, mfn);
> +}
> +
> +int svm_avic_domain_initialise(struct domain *d)
> +{
> +    struct svm_domain *sd = &d->arch.hvm.svm;
> +    struct page_info *pg;
> +    mfn_t mfn;
> +
> +    if ( !svm_avic_enable || !has_vlapic(d) )
> +        return 0;
> +
> +    /* AVIC logical APIC ID table */
> +    pg = alloc_domheap_page(d, MEMF_no_refcount);
> +    if ( !pg )
> +        return -ENOMEM;
> +
> +    mfn = page_to_mfn(pg);
> +    clear_domain_page(mfn);
> +
> +    sd->avic_logical_id_table_mfn = mfn;
> +    sd->avic_logical_id_table = map_domain_page_global(mfn);
> +
> +    /* AVIC physical APIC ID table */
> +    pg = alloc_domheap_page(d, MEMF_no_refcount);
> +    if ( !pg )
> +        return -ENOMEM;
> +
> +    mfn = page_to_mfn(pg);
> +    clear_domain_page(mfn);
> +
> +    sd->avic_physical_id_table_mfn = mfn;
> +    sd->avic_physical_id_table = map_domain_page_global(mfn);
> +
> +    return avic_set_mmio_p2m_entry(d);
> +}
> +
> +void svm_avic_domain_relinquish_resources(struct domain *d)
> +{
> +    struct svm_domain *sd = &d->arch.hvm.svm;
> +    mfn_t mfn;
> +
> +    (void)avic_clear_mmio_p2m_entry(d);
> +
> +    mfn = sd->avic_physical_id_table_mfn;
> +    if ( !mfn_eq(mfn, _mfn(0)) )
> +    {
> +        unmap_domain_page_global(sd->avic_physical_id_table);
> +        sd->avic_physical_id_table = NULL;
> +
> +        free_domheap_page(mfn_to_page(mfn));
> +        sd->avic_physical_id_table_mfn = _mfn(0);
> +    }
> +
> +    mfn = sd->avic_logical_id_table_mfn;
> +    if ( !mfn_eq(mfn, _mfn(0)) )
> +    {
> +        unmap_domain_page_global(sd->avic_logical_id_table);
> +        sd->avic_logical_id_table = NULL;
> +
> +        free_domheap_page(mfn_to_page(mfn));
> +        sd->avic_logical_id_table_mfn = _mfn(0);
> +    }

I think using INVALID_MFN would be better than _mfn(0). But given that 
Intel VMX does also use _mfn(0), I would like other's opinion on that 
matter.
(though, if we switch to page_info *, that may not matter anymore)

> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/x86/hvm/svm/avic.h b/xen/arch/x86/hvm/svm/avic.h
> new file mode 100644
> index 000000000000..3da62f59b0cf
> --- /dev/null
> +++ b/xen/arch/x86/hvm/svm/avic.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * AMD Advanced Virtual Interrupt Controller (AVIC)
> + */
> +
> +#ifndef __SVM_AVIC_H__
> +#define __SVM_AVIC_H__
> +
> +extern bool svm_avic_enable;
> +
> +int svm_avic_domain_initialise(struct domain *d);
> +void svm_avic_domain_relinquish_resources(struct domain *d);
> +
> +#endif /* __SVM_AVIC_H__ */
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index db81dad4b681..8b7bfed0049f 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -40,6 +40,7 @@
>   
>   #include <public/sched.h>
>   
> +#include "avic.h"
>   #include "nestedhvm.h"
>   #include "svm.h"
>   #include "vmcb.h"
> @@ -1084,6 +1085,11 @@ static int cf_check svm_domain_initialise(struct domain *d)
>           .to   = svm_ctxt_switch_to,
>           .tail = svm_do_resume,
>       };
> +    int rc;
> +
> +    rc = svm_avic_domain_initialise(d);
> +    if ( rc )
> +        return rc;
>   
>       d->arch.ctxt_switch = &csw;
>   
> @@ -1095,6 +1101,11 @@ static int cf_check svm_domain_initialise(struct domain *d)
>       return 0;
>   }
>   
> +static void cf_check svm_domain_relinquish_resources(struct domain *d)
> +{
> +    svm_avic_domain_relinquish_resources(d);
> +}
> +
>   static int cf_check svm_vcpu_initialise(struct vcpu *v)
>   {
>       int rc;
> @@ -2384,6 +2395,7 @@ static struct hvm_function_table __initdata_cf_clobber svm_function_table = {
>       .cpu_up               = svm_cpu_up,
>       .cpu_down             = svm_cpu_down,
>       .domain_initialise    = svm_domain_initialise,
> +    .domain_relinquish_resources = svm_domain_relinquish_resources,
>       .vcpu_initialise      = svm_vcpu_initialise,
>       .vcpu_destroy         = svm_vcpu_destroy,
>       .save_cpu_ctxt        = svm_save_vmcb_ctxt,
> @@ -2464,6 +2476,12 @@ static void __init print_svm_features(void)
>           printk(" - none\n");
>   }
>   
> +static void __init svm_avic_start(void)
> +{
> +    if ( !cpu_has_svm_avic || pv_shim )
> +        svm_avic_enable = false;

What's special about pv_shim ?

> +}
> +
>   const struct hvm_function_table * __init start_svm(void)
>   {
>       svm_host_osvw_reset();
> @@ -2487,6 +2505,8 @@ const struct hvm_function_table * __init start_svm(void)
>       if ( cpu_has_tsc_ratio )
>           svm_function_table.tsc_scaling.ratio_frac_bits = 32;
>   
> +    svm_avic_start();
> +
>       svm_function_table.caps.hap = cpu_has_svm_npt;
>       svm_function_table.caps.hap_superpage_2mb = true;
>       svm_function_table.caps.hap_superpage_1gb = cpu_has_page1gb;
> diff --git a/xen/arch/x86/include/asm/hvm/svm-types.h b/xen/arch/x86/include/asm/hvm/svm-types.h
> index 71427b3c11d1..30bdaf27a856 100644
> --- a/xen/arch/x86/include/asm/hvm/svm-types.h
> +++ b/xen/arch/x86/include/asm/hvm/svm-types.h
> @@ -15,6 +15,14 @@ struct svm_domain {
>               uint64_t status;
>           };
>       } osvw;
> +
> +    /*
> +     * Domain AVIC state.
> +     */
> +    mfn_t avic_logical_id_table_mfn;
> +    uint32_t *avic_logical_id_table;
> +    mfn_t avic_physical_id_table_mfn;
> +    uint64_t *avic_physical_id_table;

I suggest moving avic-specific fields to a dedicated structure e.g 
struct svm_avic (that would allow you to drop 'avic_' prefix for all 
fields).

Is there a reason to use mfn_t instead of page_info * ?

>   };
>   
>   struct svm_vcpu {

Teddy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]

Re: [PATCH v1 07/28] x86/hvm: avic: add VMCB state initialization

[Ross Lagerwall]

On 11/5/26 1:33 AM, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com>
> 
> Update SVM VMCB with AVIC-related data types as per [1].
> 
> Install AVIC backing page and logical/physical APIC ID tables mappings
> into VMCB.
> 
> Set VMCB 'avic_enable' bit to 1 in case of successful initialization.
> 
> [1] AMD APM Volumes 1-5 "15.29 Advanced Virtual Interrupt Controller".
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
>   xen/arch/x86/hvm/svm/avic.c | 110 ++++++++++++++++++++++++++++++++++++
>   xen/arch/x86/hvm/svm/avic.h |   3 +
>   xen/arch/x86/hvm/svm/vmcb.c |   7 +++
>   xen/arch/x86/hvm/svm/vmcb.h |  19 +++++--
>   4 files changed, 135 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/svm/avic.c b/xen/arch/x86/hvm/svm/avic.c
> index 2c0e113ae34e..bc98661a7057 100644
> --- a/xen/arch/x86/hvm/svm/avic.c
> +++ b/xen/arch/x86/hvm/svm/avic.c
> @@ -9,6 +9,16 @@
>   #include <xen/param.h>
>   
>   #include "avic.h"
> +#include "vmcb.h"
> +
> +#define AVIC_PHYSICAL_ID_ENTRY_BACKING_PAGE_MASK        GENMASK_ULL(51, 12U)
> +#define AVIC_PHYSICAL_ID_TABLE_SIZE_MASK                (0xFFULL)
> +
> +/*
> + * For AVIC, the max index allowed for physical APIC ID table is 0xfe (254), as
> + * 0xff is a broadcast to all CPUs, i.e. can't be targeted individually.
> + */
> +#define AVIC_MAX_PHYSICAL_ID                            (0xFEU)
>   
>   bool __read_mostly svm_avic_enable;
>   
> @@ -125,6 +135,106 @@ void svm_avic_domain_relinquish_resources(struct domain *d)
>       }
>   }
>   
> +static bool avic_vcpu_supported(struct vcpu *v)
> +{
> +    struct domain *d = v->domain;
> +    unsigned int id;
> +
> +    if ( !svm_avic_enable || !has_vlapic(d) )
> +        return false;
> +
> +    if ( d->max_vcpus * 2 >= AVIC_PHYSICAL_ID_TABLE_SIZE_MASK )
> +    {
> +        gprintk(XENLOG_WARNING,
> +                "%pv: AVIC: unsupported domain max_vcpus %u\n",
> +                v, d->max_vcpus);
> +        return false;
> +    }
> +
> +    id = GET_xAPIC_ID(vlapic_reg_read(v, APIC_ID));
> +    if ( id > AVIC_MAX_PHYSICAL_ID )
> +    {
> +        gprintk(XENLOG_WARNING,
> +                "%pv: AVIC: unsupported APIC ID %u\n",
> +                v, id);
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
> +static void avic_vcpu_enable(const struct vcpu *v, bool enable)
> +{
> +    const struct svm_vcpu *sv = &v->arch.hvm.svm;
> +    struct vmcb_struct *vmcb = sv->vmcb;
> +
> +    vmcb->_vintr.fields.x2avic_enable = 0;
> +    vmcb->_vintr.fields.avic_enable = enable;
> +    vmcb->cleanbits.avic = 0;
> +
> +    printk(XENLOG_INFO
> +           "%pv: AVIC: %s: apic_id=%#x avic_physical_id=%#"PRIx64" max_vcpus=%u\n",
> +           v,
> +           enable ? "enable" : "disable",
> +           GET_xAPIC_ID(vlapic_reg_read(v, APIC_ID)),
> +           sv->avic_physical_id ? ACCESS_ONCE(*sv->avic_physical_id) :
> +                                  0xDEADDEADUL,
> +           v->domain->max_vcpus);
> +}
> +
> +int svm_avic_vmcb_create(struct vcpu *v)
> +{
> +    struct domain *d = v->domain;
> +    struct svm_domain *sd = &d->arch.hvm.svm;
> +    struct svm_vcpu *sv = &v->arch.hvm.svm;
> +    struct vmcb_struct *vmcb = sv->vmcb;
> +    const struct vlapic *vlapic = vcpu_vlapic(v);
> +    mfn_t mfn;
> +    uint64_t entry;
> +    unsigned int id;
> +
> +    if ( !avic_vcpu_supported(v) )
> +        return 0;
> +
> +    vmcb->avic_vapic_bar = APIC_DEFAULT_PHYS_BASE;
> +    vmcb->avic_backing_page = page_to_maddr(vlapic->regs_page);
> +
> +    mfn = domain_page_map_to_mfn(sd->avic_logical_id_table);
> +    vmcb->avic_logical_id_table = mfn_to_maddr(mfn);
> +
> +    mfn = domain_page_map_to_mfn(sd->avic_physical_id_table);
> +    vmcb->avic_physical_id_table = mfn_to_maddr(mfn);
> +    vmcb->avic_physical_id_table |= (d->max_vcpus * 2) &
> +                                    AVIC_PHYSICAL_ID_TABLE_SIZE_MASK;
> +
> +    id = GET_xAPIC_ID(vlapic_reg_read(v, APIC_ID));
> +
> +    sv->avic_physical_id = &sd->avic_physical_id_table[id];
> +
> +    entry = vmcb->avic_backing_page & AVIC_PHYSICAL_ID_ENTRY_BACKING_PAGE_MASK;
> +    entry |= AVIC_PHYSICAL_ID_ENTRY_VALID_MASK;
> +
> +    ACCESS_ONCE(*sv->avic_physical_id) = entry;
> +
> +    avic_vcpu_enable(v, true);
> +
> +    return 0;
> +}
> +
> +void svm_avic_vmcb_destroy(struct vcpu *v)
> +{
> +    struct svm_vcpu *sv = &v->arch.hvm.svm;
> +    struct vmcb_struct *vmcb = sv->vmcb;
> +
> +    if ( svm_avic_vcpu_enabled(v) )
> +        avic_vcpu_enable(v, false);
> +
> +    vmcb->avic_physical_id_table = 0;
> +    vmcb->avic_logical_id_table = 0;
> +    vmcb->avic_backing_page = 0;
> +    vmcb->avic_vapic_bar = 0;
> +}
> +
>   /*
>    * Local variables:
>    * mode: C
> diff --git a/xen/arch/x86/hvm/svm/avic.h b/xen/arch/x86/hvm/svm/avic.h
> index 3da62f59b0cf..1084ae683944 100644
> --- a/xen/arch/x86/hvm/svm/avic.h
> +++ b/xen/arch/x86/hvm/svm/avic.h
> @@ -11,4 +11,7 @@ extern bool svm_avic_enable;
>   int svm_avic_domain_initialise(struct domain *d);
>   void svm_avic_domain_relinquish_resources(struct domain *d);
>   
> +int svm_avic_vmcb_create(struct vcpu *v);
> +void svm_avic_vmcb_destroy(struct vcpu *v);
> +
>   #endif /* __SVM_AVIC_H__ */
> diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
> index 3ec5bdddf588..c4282bfd6c6e 100644
> --- a/xen/arch/x86/hvm/svm/vmcb.c
> +++ b/xen/arch/x86/hvm/svm/vmcb.c
> @@ -20,6 +20,7 @@
>   #include <asm/p2m.h>
>   #include <asm/spec_ctrl.h>
>   
> +#include "avic.h"
>   #include "svm.h"
>   #include "vmcb.h"
>   
> @@ -215,6 +216,10 @@ int svm_create_vmcb(struct vcpu *v)
>       if ( rc )
>           return rc;
>   
> +    rc = svm_avic_vmcb_create(v);
> +    if ( rc )
> +        return rc;
> +
>       paddr = virt_to_maddr(nv->nv_n1vmcx);
>       nv->nv_n1vmcx_pa = paddr;
>       svm->vmcb_pa = paddr;
> @@ -227,6 +232,8 @@ void svm_destroy_vmcb(struct vcpu *v)
>       struct nestedvcpu *nv = &vcpu_nestedhvm(v);
>       struct svm_vcpu *svm = &v->arch.hvm.svm;
>   
> +    svm_avic_vmcb_destroy(v);
> +
>       if ( nv->nv_n1vmcx != NULL )
>           free_vmcb(nv->nv_n1vmcx);
>   
> diff --git a/xen/arch/x86/hvm/svm/vmcb.h b/xen/arch/x86/hvm/svm/vmcb.h
> index 8b351679fac5..1ee5da053a9a 100644
> --- a/xen/arch/x86/hvm/svm/vmcb.h
> +++ b/xen/arch/x86/hvm/svm/vmcb.h
> @@ -346,7 +346,9 @@ typedef union
>           u64 intr_masking: 1;
>           u64 vgif_enable:  1;
>           u64 vnmi_enable:  1;
> -        u64 :             5;
> +        u64 rsvd2:        3;
> +        u64 x2avic_enable:1;
> +        u64 avic_enable:  1;  /* bit 31 */
>           u64 vector:       8;
>           u64 rsvd3:       24;
>       } fields;
> @@ -362,6 +364,10 @@ typedef union
>       uint64_t raw;
>   } virt_ext_t;
>   
> +/*
> + * AMD64 Achitecture's Programmer's Manual, Volume 2
> + * 15.15.3 VMCB Clean Field
> + */
>   typedef union
>   {
>       struct {
> @@ -377,7 +383,7 @@ typedef union
>           bool seg:1;        /* 8:  cs, ds, es, ss, cpl */
>           bool cr2:1;        /* 9:  cr2 */
>           bool lbr:1;        /* 10: debugctlmsr, last{branch,int}{to,from}ip */
> -        bool :1;
> +        bool avic:1;       /* 11: AVIC */
>           bool cet:1;        /* 12: msr_s_set, ssp, msr_isst */
>       };
>       uint32_t raw;
> @@ -469,7 +475,8 @@ struct vmcb_struct {
>           };
>           uint64_t _np_ctrl;
>       };
> -    uint64_t res08[2];
> +    uint64_t avic_vapic_bar;    /* offset 0x98 */
> +    uint64_t res08;             /* offset 0xA0 */
>       intinfo_t event_inj;        /* offset 0xA8 */
>       uint64_t _h_cr3;            /* offset 0xB0 - cleanbit 4 */
>       virt_ext_t virt_ext;        /* offset 0xB8 */
> @@ -478,7 +485,11 @@ struct vmcb_struct {
>       uint64_t nextrip;           /* offset 0xC8 */
>       uint8_t  guest_ins_len;     /* offset 0xD0 */
>       uint8_t  guest_ins[15];     /* offset 0xD1 */
> -    uint64_t res10a[8];         /* offset 0xE0 */
> +    uint64_t avic_backing_page;      /* offset 0xE0 */
> +    uint64_t res09a;                 /* offset 0xE8 */
> +    uint64_t avic_logical_id_table;  /* offset 0xF0 */
> +    uint64_t avic_physical_id_table; /* offset 0xF8 */
> +    uint64_t res10a[4];         /* offset 0x100 pad to save area */
>       uint16_t bus_lock_count;    /* offset 0x120 */
>       uint16_t res10b[3];         /* offset 0x122 */
>       uint64_t res10c[91];        /* offset 0x128 pad to save area */

The avic cleanbit is defined above but then you don't actually make use
of it.  The fields to which the cleanbit applies should be marked and
have accessors defined.

In principle, it shouldn't make a difference since AFAICT these fields
are only written during VMCB creation when all the cleanbits are cleared
anyway but it makes sense for consistency.

Ross

Re: [PATCH v1 01/28] x86/hvm: move SVM feature printout to a separate function

[Andrew Cooper]

On 05/11/2026 1:33 am, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com>
>
> Isolate long SVM feature list printout in a separate function to
> improve readability.
>
> Not a functional change.
>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Re: [PATCH v1 05/28] x86/hvm: cleanup svm_create_vmcb()

[Andrew Cooper]

On 05/11/2026 1:33 am, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com>
>
> svm_create_vmcb() does not need to be idempotent because in case of
> failure caller must call svm_destroy_vmcb().
>
> Simplify code in svm_create_vmcb() to account for that.

This is more complicated than it seems.

There are two paths which need to clean up a struct vcpu:
1) The domain_kill hypercall (i.e. clean destruction)
2) A failure within vcpu_create()

Nowadays, both vcpu and domain create() end up calling back into the
relevant destroy paths.  This is some work I started a long time ago and
never managed to finish.

We had a number of memory leaks because each init/create function was
required to clean up after itself, and several did this incorrectly.  It
was also wasteful because we had duplicated cleanup code.

At the moment, both vcpu_create() and hvm_vcpu_initialise() will call
the relevant destruction path.

In the future, we want to remove all the create() error handling except
from the top-level, because that removes the duplication.  However this
is a large task and it's the common/arch boundary where it gets
particularly hard to organise safely.


Anyway.  We can take this patch with an adjusted commit message.  In
particular, saying that the "caller must call ..." is a hope, rather
than a clear statement that this is how the code behaves.  I suggest
something like:

---8<---
svm_create_vmcb() does not need to do error handling itself because
vcpu_create() handles this at the top level by calling vcpu_destroy().
---8<---

But, if someone has some copious free time, it would be nicer if we
could do this a little bit wider and make better progress towards
removing the create() error handling.

~Andrew

Re: [PATCH v1 22/28] x86/hvm: avic: ensure SVM AVIC supports xAPIC operation only

[Andrew Cooper]

On 05/11/2026 1:33 am, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com>
>
> xAVIC does not support x2APIC operation (yet).
>
> Until x2APIC mode is supported, don't advertise x2APIC to HVM
> guests when SVM AVIC is enabled.
>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
>  xen/arch/x86/cpu-policy.c          |  3 ++-
>  xen/arch/x86/hvm/svm/svm.c         | 12 ++++++++++++
>  xen/arch/x86/hvm/vmx/vmx.c         |  4 ++++
>  xen/arch/x86/include/asm/hvm/hvm.h | 15 ++++++++++++++-
>  4 files changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
> index 48a3185eed8d..ca9b1a523570 100644
> --- a/xen/arch/x86/cpu-policy.c
> +++ b/xen/arch/x86/cpu-policy.c
> @@ -764,7 +764,8 @@ static void __init calculate_hvm_max_policy(void)
>       * (x2)APIC isn't enabled.
>       */
>      __set_bit(X86_FEATURE_APIC, fs);
> -    __set_bit(X86_FEATURE_X2APIC, fs);
> +    if ( hvm_x2apic_supported() )
> +        __set_bit(X86_FEATURE_X2APIC, fs);

The reasoning is backwards, and breaks things.

HVM guests do support x2APIC, and have seen the feature bit for a decade
or more.

Clobbering this bit will cause the Xapi toolstack to block migration. 
The xl toolstack doesn't do the relevant safety checks (yet) and the VM
will explode as a result if it was actually using x2APIC.

x2AVIC is an acceleration.  It's fine for the acceleration not to be
turned on, but it is not fine to hide x2APIC as a consequence.

Both the Viridian spec and Xen itself have hints in the virtualised
leaves to tell a guest whether it ought to prefer using xAPIC or x2APIC
mode.  Under plain virt, x2APIC is vastly preferable because the
RD/WRMSR exit gives you all the information you require, whereas xAPIC
requires instruction emulation to figure out the register was being
accessed.

~Andrew

Re: [PATCH v1 22/28] x86/hvm: avic: ensure SVM AVIC supports xAPIC operation only

[Ross Lagerwall]

On 11/5/26 1:33 AM, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com>
> 
> xAVIC does not support x2APIC operation (yet).
> 
> Until x2APIC mode is supported, don't advertise x2APIC to HVM
> guests when SVM AVIC is enabled.
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
>   xen/arch/x86/cpu-policy.c          |  3 ++-
>   xen/arch/x86/hvm/svm/svm.c         | 12 ++++++++++++
>   xen/arch/x86/hvm/vmx/vmx.c         |  4 ++++
>   xen/arch/x86/include/asm/hvm/hvm.h | 15 ++++++++++++++-
>   4 files changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
> index 48a3185eed8d..ca9b1a523570 100644
> --- a/xen/arch/x86/cpu-policy.c
> +++ b/xen/arch/x86/cpu-policy.c
> @@ -764,7 +764,8 @@ static void __init calculate_hvm_max_policy(void)
>        * (x2)APIC isn't enabled.
>        */
>       __set_bit(X86_FEATURE_APIC, fs);
> -    __set_bit(X86_FEATURE_X2APIC, fs);
> +    if ( hvm_x2apic_supported() )
> +        __set_bit(X86_FEATURE_X2APIC, fs);

Perhaps this is moot given Andrew's comment, but on my host the X2APIC feature
bit is already enabled by inheriting from the host policy so this doesn't have
the desired effect and X2APIC is still exposed to the guest.

Ross

Re: [PATCH v1 07/28] x86/hvm: avic: add VMCB state initialization

[Ross Lagerwall]

On 11/5/26 1:33 AM, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com>
> 
> Update SVM VMCB with AVIC-related data types as per [1].
> 
> Install AVIC backing page and logical/physical APIC ID tables mappings
> into VMCB.
> 
> Set VMCB 'avic_enable' bit to 1 in case of successful initialization.
> 
> [1] AMD APM Volumes 1-5 "15.29 Advanced Virtual Interrupt Controller".
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
>   xen/arch/x86/hvm/svm/avic.c | 110 ++++++++++++++++++++++++++++++++++++
>   xen/arch/x86/hvm/svm/avic.h |   3 +
>   xen/arch/x86/hvm/svm/vmcb.c |   7 +++
>   xen/arch/x86/hvm/svm/vmcb.h |  19 +++++--
>   4 files changed, 135 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/svm/avic.c b/xen/arch/x86/hvm/svm/avic.c
> index 2c0e113ae34e..bc98661a7057 100644
> --- a/xen/arch/x86/hvm/svm/avic.c
> +++ b/xen/arch/x86/hvm/svm/avic.c
> @@ -9,6 +9,16 @@
>   #include <xen/param.h>
>   
>   #include "avic.h"
> +#include "vmcb.h"
> +
> +#define AVIC_PHYSICAL_ID_ENTRY_BACKING_PAGE_MASK        GENMASK_ULL(51, 12U)
> +#define AVIC_PHYSICAL_ID_TABLE_SIZE_MASK                (0xFFULL)
> +
> +/*
> + * For AVIC, the max index allowed for physical APIC ID table is 0xfe (254), as
> + * 0xff is a broadcast to all CPUs, i.e. can't be targeted individually.
> + */
> +#define AVIC_MAX_PHYSICAL_ID                            (0xFEU)
>   
>   bool __read_mostly svm_avic_enable;
>   
> @@ -125,6 +135,106 @@ void svm_avic_domain_relinquish_resources(struct domain *d)
>       }
>   }
>   
> +static bool avic_vcpu_supported(struct vcpu *v)
> +{
> +    struct domain *d = v->domain;
> +    unsigned int id;
> +
> +    if ( !svm_avic_enable || !has_vlapic(d) )
> +        return false;
> +
> +    if ( d->max_vcpus * 2 >= AVIC_PHYSICAL_ID_TABLE_SIZE_MASK )
> +    {
> +        gprintk(XENLOG_WARNING,
> +                "%pv: AVIC: unsupported domain max_vcpus %u\n",
> +                v, d->max_vcpus);
> +        return false;
> +    }
> +
> +    id = GET_xAPIC_ID(vlapic_reg_read(v, APIC_ID));
> +    if ( id > AVIC_MAX_PHYSICAL_ID )
> +    {
> +        gprintk(XENLOG_WARNING,
> +                "%pv: AVIC: unsupported APIC ID %u\n",
> +                v, id);
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
> +static void avic_vcpu_enable(const struct vcpu *v, bool enable)
> +{
> +    const struct svm_vcpu *sv = &v->arch.hvm.svm;
> +    struct vmcb_struct *vmcb = sv->vmcb;
> +
> +    vmcb->_vintr.fields.x2avic_enable = 0;
> +    vmcb->_vintr.fields.avic_enable = enable;
> +    vmcb->cleanbits.avic = 0;
> +
> +    printk(XENLOG_INFO
> +           "%pv: AVIC: %s: apic_id=%#x avic_physical_id=%#"PRIx64" max_vcpus=%u\n",
> +           v,
> +           enable ? "enable" : "disable",
> +           GET_xAPIC_ID(vlapic_reg_read(v, APIC_ID)),
> +           sv->avic_physical_id ? ACCESS_ONCE(*sv->avic_physical_id) :
> +                                  0xDEADDEADUL,
> +           v->domain->max_vcpus);
> +}

This is called from svm_avic_vmcb_destroy() which may be called after
domain_relinquish_resources(). Dereferencing avic_physical_id hits a page
fault since the page is already unmapped by then.

Ross

Re: [PATCH v1 19/28] x86/hvm: avic: enable SVM AVIC statistics reporting

[Andrew Cooper]

On 05/11/2026 1:33 am, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com>
>
> Introduce performance counters for AVIC.
>
> Enable statistics reporing via new 'dump_posted_intr' mechanism.
>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>

Xen has CONFIG_PERF_COUNTERS already.

Please see about using those, rather than inventing an ad-hoc mechanism.

~Andrew

Re: [PATCH v1 17/28] x86/hvm: avic: route interrupt delivery to vCPU

[Ross Lagerwall]

On 11/5/26 1:33 AM, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com>
> 
> Enabling AVIC implicitly disables the V_IRQ, V_INTR_PRIO, V_IGN_TPR,
> and V_INTR_VECTOR fields in the VMCB Control Word.
> 
> Introduce new interrupt injection code via AVIC backing page.
> 
> Essentially, that is handling of 'IsRunning' bit in physical APIC ID
> table entry: it needs to be in sync with the scheduled vCPU. Shadow
> the state to work around hardware not perceiving 'IsRunning' bit.
> 
> Add new doorbell MSR to accelerate IPI emulation. The MSR can be used
> for posting interrupts from hypervisor to the guest (e.g. event
> channels).
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
>   xen/arch/x86/hvm/svm/avic.c              | 156 +++++++++++++++++++++++
>   xen/arch/x86/hvm/svm/avic.h              |  17 +++
>   xen/arch/x86/hvm/svm/intr.c              |   9 ++
>   xen/arch/x86/hvm/svm/svm.c               |  32 +++++
>   xen/arch/x86/include/asm/hvm/svm-types.h |   2 +
>   xen/arch/x86/include/asm/msr-index.h     |   1 +
>   6 files changed, 217 insertions(+)
> 
> diff --git a/xen/arch/x86/hvm/svm/avic.c b/xen/arch/x86/hvm/svm/avic.c
> index 18a814871133..0bcab0465227 100644
> --- a/xen/arch/x86/hvm/svm/avic.c
> +++ b/xen/arch/x86/hvm/svm/avic.c
> @@ -33,6 +33,8 @@
>   #define AVIC_MAX_PHYSICAL_ID                            (0xFEU)
>   
>   bool __read_mostly svm_avic_enable;
> +unsigned int __read_mostly svm_avic_features = AVIC_HW_ISRUNNING |
> +                                               AVIC_DOORBELL_MSR;
>   
>   static const char __section(".bss.page_aligned.const") __aligned(PAGE_SIZE)
>       avic_apic_access_page[PAGE_SIZE];
> @@ -50,6 +52,11 @@ static int cf_check avic_parse_cmdline(const char *s)
>       {
>           svm_avic_enable = true;
>       }
> +    else if ( !strcmp(s, "nodoorbell") )
> +    {
> +        svm_avic_enable = true;
> +        svm_avic_features &= ~AVIC_DOORBELL_MSR;
> +    }
>       else
>       {
>           return -EINVAL;
> @@ -228,6 +235,7 @@ int svm_avic_vmcb_create(struct vcpu *v)
>       entry |= AVIC_PHYSICAL_ID_ENTRY_VALID_MASK;
>   
>       ACCESS_ONCE(*sv->avic_physical_id) = entry;
> +    ACCESS_ONCE(sv->avic_shadow_id) = entry;
>   
>       avic_vcpu_enable(v, true);
>   
> @@ -248,6 +256,102 @@ void svm_avic_vmcb_destroy(struct vcpu *v)
>       vmcb->avic_vapic_bar = 0;
>   }
>   
> +static void avic_vcpu_set_cpu(struct vcpu *v)
> +{
> +    struct svm_vcpu *sv = &v->arch.hvm.svm;
> +    unsigned apic_id = cpu_data[v->processor].apicid;
> +    uint64_t entry;
> +
> +    BUG_ON(sv->avic_physical_id == NULL);
> +    ASSERT(!(apic_id & ~AVIC_PHYSICAL_ID_ENTRY_HOST_PHYSICAL_ID_MASK));
> +
> +    entry = ACCESS_ONCE(sv->avic_shadow_id);
> +    entry &= ~AVIC_PHYSICAL_ID_ENTRY_HOST_PHYSICAL_ID_MASK;
> +    entry |= apic_id;
> +
> +    ACCESS_ONCE(*sv->avic_physical_id) = entry;
> +
> +    ACCESS_ONCE(sv->avic_shadow_id) = entry;
> +}
> +
> +static void avic_vcpu_set_running(struct vcpu *v)
> +{
> +    struct svm_vcpu *sv = &v->arch.hvm.svm;
> +    uint64_t entry;
> +
> +    BUG_ON(sv->avic_physical_id == NULL);
> +
> +    entry = ACCESS_ONCE(sv->avic_shadow_id);
> +    entry |= AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK;
> +
> +    /* Shadow IsRunning state update. */
> +    ACCESS_ONCE(sv->avic_shadow_id) = entry;
> +
> +    /*
> +     * If hardware has problems detecting IsRunning (erratum 1235)
> +     * keep IsRunning in the shadow entry only.
> +     */
> +    if ( svm_avic_features & AVIC_HW_ISRUNNING )
> +        ACCESS_ONCE(*sv->avic_physical_id) = entry;
> +}
> +
> +static void avic_vcpu_clear_running(struct vcpu *v)
> +{
> +    struct svm_vcpu *sv = &v->arch.hvm.svm;
> +    unsigned apic_id = cpu_data[v->processor].apicid;
> +    uint64_t entry;
> +
> +    BUG_ON(sv->avic_physical_id == NULL);
> +    ASSERT(!(apic_id & ~AVIC_PHYSICAL_ID_ENTRY_HOST_PHYSICAL_ID_MASK));
> +
> +    entry = ACCESS_ONCE(sv->avic_shadow_id);
> +    entry &= ~AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK;
> +
> +    /*
> +     * If hardware has problems detecting IsRunning (erratum 1235)
> +     * keep IsRunning in the shadow entry only.
> +     */
> +    if ( svm_avic_features & AVIC_HW_ISRUNNING )
> +        ACCESS_ONCE(*sv->avic_physical_id) = entry;
> +
> +    /* Shadow IsRunning state update. */
> +    ACCESS_ONCE(sv->avic_shadow_id) = entry;
> +}
> +
> +/* vCPU enters blocking state. */
> +void svm_avic_vcpu_block(struct vcpu *v)
> +{
> +    avic_vcpu_clear_running(v);
> +}
> +
> +/* vCPU is preparing to resume guest execution. */
> +void svm_avic_vcpu_resume(struct vcpu *v)
> +{
> +    avic_vcpu_set_running(v);
> +}
> +
> +/* vCPU is being assigned a new pCPU. */
> +void svm_avic_vcpu_switch_to(struct vcpu *v)
> +{
> +    avic_vcpu_set_cpu(v);
> +}
> +
> +/* vCPU is being removed from a pCPU. */
> +void svm_avic_vcpu_switch_from(struct vcpu *v)
> +{
> +    avic_vcpu_clear_running(v);
> +}
> +
> +bool svm_avic_vcpu_enabled(const struct vcpu *v)
> +{
> +    const struct svm_vcpu *sv = &v->arch.hvm.svm;
> +    const struct vmcb_struct *vmcb = sv->vmcb;
> +
> +    ASSERT(!(vmcb->_vintr.fields.x2avic_enable));
> +
> +    return vmcb->_vintr.fields.avic_enable;
> +}
> +
>   /*
>    * Handle the AVIC_INCOMP_IPI #VMEXIT when AVIC is enabled.
>    *
> @@ -546,6 +650,58 @@ void svm_avic_vmexit_noaccel(struct cpu_user_regs *regs)
>           hvm_emulate_one_vm_event(EMUL_KIND_NORMAL, X86_EXC_UD, X86_EVENT_NO_EC);
>   }
>   
> +static void avic_deliver_posted_intr(struct vcpu *v)
> +{
> +    struct svm_vcpu *sv = &v->arch.hvm.svm;
> +    uint64_t entry;
> +    unsigned int apicid;
> +
> +    smp_mb();
> +
> +    entry = ACCESS_ONCE(sv->avic_shadow_id);
> +
> +    if ( !(entry & AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK) )
> +        return;
> +
> +    /*
> +     * Some AMD systems (e.g. Zen5 Family 26 (0x1a), Model 36 (0x24)) have
> +     * broken remote notification via doorbell MSR.
> +     * Fallback to brute force, if requested.
> +     */
> +    if ( svm_avic_features & AVIC_DOORBELL_MSR )
> +    {
> +        apicid = entry & AVIC_PHYSICAL_ID_ENTRY_HOST_PHYSICAL_ID_MASK;
> +
> +        /* Ensure the pending interrupt wakes vCPU. */
> +        vcpu_unblock(v);
> +
> +        /*
> +         * AMD APM, 5.29.8.2 Doorbell Mechanism
> +         * The serializing semantics of WRMSR are relaxed when writing to
> +         * the Doorbell Register.
> +         */
> +        wrmsrl(MSR_AMD_AVIC_DOORBELL, apicid);
> +    }
> +    else
> +    {
> +        vcpu_kick(v);
> +    }
> +}
> +
> +void cf_check svm_avic_hvm_deliver_posted_intr(struct vcpu *v, uint8_t vector)
> +{
> +    ASSERT(v != current);
> +
> +    if ( svm_avic_vcpu_enabled(v) )
> +    {
> +        avic_deliver_posted_intr(v);
> +    }
> +    else
> +    {
> +        vcpu_kick(v);
> +    }
> +}
> +
>   /*
>    * Local variables:
>    * mode: C
> diff --git a/xen/arch/x86/hvm/svm/avic.h b/xen/arch/x86/hvm/svm/avic.h
> index f9ff3ffc9346..098924925e89 100644
> --- a/xen/arch/x86/hvm/svm/avic.h
> +++ b/xen/arch/x86/hvm/svm/avic.h
> @@ -6,6 +6,15 @@
>   #ifndef __SVM_AVIC_H__
>   #define __SVM_AVIC_H__
>   
> +#include <xen/const.h>
> +
> +/* Known AVIC hardware quirks. */
> +enum {
> +    AVIC_HW_ISRUNNING       = BIT(0, U),    /* See erratum 1235 */
> +    AVIC_DOORBELL_MSR       = BIT(1, U),    /* Remote vCPU notification OK */
> +};
> +
> +extern unsigned int svm_avic_features;
>   extern bool svm_avic_enable;
>   
>   int svm_avic_domain_initialise(struct domain *d);
> @@ -17,4 +26,12 @@ void svm_avic_vmcb_destroy(struct vcpu *v);
>   void svm_avic_vmexit_incomp_ipi(struct cpu_user_regs *regs);
>   void svm_avic_vmexit_noaccel(struct cpu_user_regs *regs);
>   
> +bool svm_avic_vcpu_enabled(const struct vcpu *v);
> +void svm_avic_vcpu_block(struct vcpu *v);
> +void svm_avic_vcpu_resume(struct vcpu *v);
> +void svm_avic_vcpu_switch_to(struct vcpu *v);
> +void svm_avic_vcpu_switch_from(struct vcpu *v);
> +
> +void cf_check svm_avic_hvm_deliver_posted_intr(struct vcpu *v, uint8_t vector);
> +
>   #endif /* __SVM_AVIC_H__ */
> diff --git a/xen/arch/x86/hvm/svm/intr.c b/xen/arch/x86/hvm/svm/intr.c
> index 1d39f25e5209..9cceca704a91 100644
> --- a/xen/arch/x86/hvm/svm/intr.c
> +++ b/xen/arch/x86/hvm/svm/intr.c
> @@ -24,6 +24,7 @@
>   #include <public/hvm/ioreq.h>
>   #include <xen/domain_page.h>
>   
> +#include "avic.h"
>   #include "nestedhvm.h"
>   #include "vmcb.h"
>   
> @@ -80,6 +81,14 @@ static void svm_enable_intr_window(struct vcpu *v, struct hvm_intack intack)
>   
>       ASSERT(intack.source != hvm_intsrc_none);
>   
> +    if ( svm_avic_vcpu_enabled(v) )
> +        /*
> +         * AVIC delivers pending LAPIC interrupts from the backing page
> +         * when the guest becomes interruptible, no need to arm the legacy
> +         * V_IRQ/VINTR interrupt-window mechanism.
> +         */
> +        return;
> +

In this case, don't you miss out the call to pt_intr_post() that would normally
happen?

With this patch series applied, OVMF reliably hangs for me. Adding in a call to
pt_intr_post() lets it get as far as Linux booting (but not fully booting up
successfully).

Ross

Re: [PATCH v1 11/28] x86/hvm: introduce vcpu_is_vlapic_tpr_emulated()

[Ross Lagerwall]

On 11/6/26 1:36 PM, Andrew Cooper wrote:
> On 05/11/2026 1:33 am, dmukhin@ford.com wrote:
>> From: Denis Mukhin <dmukhin@ford.com>
>>
>> Introduce vcpu_is_vlapic_tpr_emulated() in genetic SVM code to account
>> for the case when platform provides accelerated access to vLAPIC's TPR.
>>
>> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
>> ---
>>   xen/arch/x86/hvm/svm/svm.c | 38 +++++++++++++++++++++++++++-----------
>>   1 file changed, 27 insertions(+), 11 deletions(-)
>>
>> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
>> index 673c818a0039..14a86ca4ff8f 100644
>> --- a/xen/arch/x86/hvm/svm/svm.c
>> +++ b/xen/arch/x86/hvm/svm/svm.c
>> @@ -925,6 +925,19 @@ static void cf_check svm_ctxt_switch_to(struct vcpu *v)
>>       }
>>   }
>>   
>> +static bool vcpu_is_vlapic_tpr_emulated(const struct vcpu *v, bool force_noaccel)
>> +{
>> +    const struct vlapic *vlapic = vcpu_vlapic(v);
>> +
>> +    if ( force_noaccel )
>> +        return false;
>> +
>> +    if ( vlapic_hw_disabled(vlapic) )
>> +        return false;
>> +
>> +    return true;
>> +}
>> +
>>   static void noreturn cf_check svm_do_resume(void)
>>   {
>>       struct vcpu *v = current;
>> @@ -933,7 +946,6 @@ static void noreturn cf_check svm_do_resume(void)
>>                           v->domain->arch.monitor.software_breakpoint_enabled ||
>>                           v->domain->arch.monitor.debug_exception_enabled);
>>       bool vcpu_guestmode = false;
>> -    struct vlapic *vlapic = vcpu_vlapic(v);
>>   
>>       if ( nestedhvm_enabled(v->domain) && nestedhvm_vcpu_in_guestmode(v) )
>>           vcpu_guestmode = 1;
>> @@ -959,8 +971,9 @@ static void noreturn cf_check svm_do_resume(void)
>>           vmcb->cleanbits.raw = 0;
>>       }
>>   
>> -    if ( !vcpu_guestmode && !vlapic_hw_disabled(vlapic) )
>> +    if ( vcpu_is_vlapic_tpr_emulated(v, vcpu_guestmode) )
>>       {
>> +        struct vlapic *vlapic = vcpu_vlapic(v);
>>           vintr_t intr;
>>   
>>           /* Reflect the vlapic's TPR in the hardware vtpr */
>> @@ -2538,7 +2551,6 @@ void asmlinkage svm_vmexit_handler(void)
>>       int insn_len;
>>       vintr_t intr;
>>       bool vcpu_guestmode = false;
>> -    struct vlapic *vlapic = vcpu_vlapic(v);
>>   
>>       regs->rax = vmcb->rax;
>>       regs->rip = vmcb->rip;
>> @@ -2565,8 +2577,10 @@ void asmlinkage svm_vmexit_handler(void)
>>        * NB. We need to preserve the low bits of the TPR to make checked builds
>>        * of Windows work, even though they don't actually do anything.
>>        */
>> -    if ( !vcpu_guestmode && !vlapic_hw_disabled(vlapic) )
>> +    if ( vcpu_is_vlapic_tpr_emulated(v, vcpu_guestmode) )
>>       {
>> +        struct vlapic *vlapic = vcpu_vlapic(v);
>> +
>>           intr = vmcb_get_vintr(vmcb);
>>           vlapic_set_reg(vlapic, APIC_TASKPRI,
>>                      ((intr.fields.tpr & 0x0F) << 4) |
>> @@ -3097,14 +3111,16 @@ void asmlinkage svm_vmexit_handler(void)
>>       }
>>   
>>     out:
>> -    if ( vcpu_guestmode || vlapic_hw_disabled(vlapic) )
>> -        return;
>> +    if ( vcpu_is_vlapic_tpr_emulated(v, vcpu_guestmode ) )
>> +    {
>> +        struct vlapic *vlapic = vcpu_vlapic(v);
>>   
>> -    /* The exit may have updated the TPR: reflect this in the hardware vtpr */
>> -    intr = vmcb_get_vintr(vmcb);
>> -    intr.fields.tpr =
>> -        (vlapic_get_reg(vlapic, APIC_TASKPRI) & 0xFF) >> 4;
>> -    vmcb_set_vintr(vmcb, intr);
>> +        /* The exit may have updated the TPR: reflect this in the hardware vtpr */
>> +        intr = vmcb_get_vintr(vmcb);
>> +        intr.fields.tpr =
>> +            (vlapic_get_reg(vlapic, APIC_TASKPRI) & 0xFF) >> 4;
>> +        vmcb_set_vintr(vmcb, intr);
>> +    }
> 
> There are a large number of bugs here; they're prexisting, but this
> isn't helping.
> 
> First, these final two hunks are wonky.  They're necessary, but only
> because of bugs elsewhere in Xen.
> 
> In order to explain, I need to cover one of the bigger sins of the AMD64
> architecture, CR8.  See APM Vol2 8.5.2 External Interrupt Priorities.
> 
> Windows (at least used to) write APIC_TPR very frequently.  Microsoft
> made demands of the hardware vendors, and AMD made CR8.  AMD invented a
> register called TPR which is an architectural abstraction of APIC_TPR,
> and defined it's access to be via CR8.  They even made LOCK MOV CR8 (an
> otherwise illegal encoding) be an alias of MOV CR8, for 32bit software
> to use.
> 
> AMD state that it is undefined how TPR and APIC_TPR interact.  Intel
> stated that CR8 maps directly on to APIC_TPR, and that the APIC must be
> enabled for this mechanism to work.  AMD do have storage for TPR which
> is separate to APIC_TPR (this has been confirmed), and as a result it
> must be possible to desynchronise TPR and APIC_TPR when the APIC is
> disabled.  Intel's wording suggests that they do not have storage, and
> "SDM Vol2 13.8.6.1 Interaction of Task Priorities between CR8 and APIC"
> hints specifically that only writes are reflected meaning that reads may
> not be.
> 
> 
> Since the outset, SVM had accelerated CR8.  Furthermore, because the
> V_INTR_MASKING controls both the host/guest EFLAGS.IF split *and* CR8
> virtualisation, we can't run VMs without accelerated CR8.  V_TPR is the
> storage.
> 
> This final hunk exists because we have no path from the vLAPIC code
> wiring writes of APIC_TPR back into the V_TPR field.  But, it has the
> side effect of always clearing the TPR cleanbit, and exists only to
> cover an operation where both manuals say "don't mix and match using CR8
> or APIC_TPR;  use one or the other".
> 
> When AVIC is active, the V_TPR field ceases to exist and is now mapped
> directly on to the APIC backing page, without intermediate storage.

Not according to the manual... It makes it clear that when AVIC is
enabled V_TPR is still used with bi-directional synchronization
between the CR8/V_TPR and the AVIC backing page:

"When the guest operating system writes to the TPR, the value is updated
in the backing page and the upper 4 bits of the value are automatically
copied by the hardware to the V_TPR value in the VMCB. All reads from
the TPR location return the value from the vAPIC backing page. Also, any
TPR accesses using the MOV CR8 semantics update the backing page and
V_TPR values.
...
The synchronization between the Task Priority field of the TPR and the
Task Priority field of CR8 is normal local APIC behavior which is
emulated by AVIC.
...
Enabling AVIC also affects CR8 behavior independent of V_INTR_MASKING
enable (bit 24): writes to CR8 affect the V_TPR and update the backing
page and reads from CR8 return V_TPR."

I can't find anything about what happens if the value in V_TPR and the
backing page differ during a VMRUN. Are they synchronized again or not?

Ross

Re: [PATCH v1 15/28] x86/hvm: vlapic: add checks before posted interrupt delivery

[Teddy Astie]

[-- Attachment #1.1: Type: text/plain, Size: 1840 bytes --]

Le 05/11/2026 à 02:37, dmukhin@ford.com a écrit :
> From: Denis Mukhin <dmukhin@ford.com>
> 
> Hoist IRR check for pending vector in question to avoid calling into
> posted interrupt delivery callback.
> 
> Also, avoid calling posted interrupts when current vCPU is target vCPU.
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
>   xen/arch/x86/hvm/vlapic.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 

Hmm, that patch likely introduces issues with Intel APICv.

> diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
> index 22c472d569b5..222a78b02b77 100644
> --- a/xen/arch/x86/hvm/vlapic.c
> +++ b/xen/arch/x86/hvm/vlapic.c
> @@ -164,9 +164,17 @@ void vlapic_set_irq(struct vlapic *vlapic, uint8_t vec, uint8_t trig)
>       if ( hvm_funcs.update_eoi_exit_bitmap )
>           alternative_vcall(hvm_funcs.update_eoi_exit_bitmap, target, vec, trig);
>   
> +    if ( vlapic_test_and_set_irr(vlapic, vec) )
> +        /* Interrupt vector is already pending */
> +        return;

On Intel APICv, we usually don't directly set vLAPIC IRR, but use the 
dedicated PIR to inject a interrupt (which will cause a update to vLAPIC 
IRR, details in "Posted-Interrupt Processing" section of SDM). With this 
change, there are cases where we set both vLAPIC IRR and PIR which will 
cause issues.

> +
> +    if ( target == current )
> +        /* Kick remote vCPUs only */
> +        return;
> +

There may be cases where current is blocked, so I don't think we can 
safely skip vcpu_kick() in this case.

>       if ( hvm_funcs.deliver_posted_intr )
>           alternative_vcall(hvm_funcs.deliver_posted_intr, target, vec);
> -    else if ( !vlapic_test_and_set_irr(vlapic, vec) )
> +    else
>           vcpu_kick(target);
>   }
>   

Teddy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]

Re: [PATCH v1 04/28] x86/hvm: avic: add domain state initialization

[Andrew Cooper]

On 05/11/2026 1:33 am, dmukhin@ford.com wrote:
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index db81dad4b681..8b7bfed0049f 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -2464,6 +2476,12 @@ static void __init print_svm_features(void)
>          printk(" - none\n");
>  }
>  
> +static void __init svm_avic_start(void)
> +{
> +    if ( !cpu_has_svm_avic || pv_shim )
> +        svm_avic_enable = false;
> +}
> +

arch/x86/hvm/svm/svm.c: In function ‘svm_avic_start’:
arch/x86/hvm/svm/svm.c:2480:31: error: ‘pv_shim’ undeclared (first use in this function)
 2480 |     if ( !cpu_has_svm_avic || pv_shim )
      |                               ^~~~~~~
arch/x86/hvm/svm/svm.c:2480:31: note: each undeclared identifier is reported only once for each function it appears in



But, you shouldn't care about pv_shim here at all.  By and large, shim
won't have CONFIG_HVM, but if someone has decided to make such a shim
and AVIC is in advertised in CPUID, then Xen should see about using it
just as on native.

~Andrew