There's no point in forcing a system wide update of the MTRRs on all processors
when there are no changes to be propagated.  On AP startup it's only the AP
that needs to write the system wide MTRR values in order to match the rest of
the already online CPUs.

We have occasionally seen the watchdog trigger during `xen-hptool cpu-online`
in one Intel Cascade Lake box with 448 CPUs due to the re-setting of the MTRRs
on all the CPUs in the system.

While there adjust the comment to clarify why the system-wide resetting of the
MTRR registers is not needed for the purposes of mtrr_ap_init().

Signed-off-by: Roger Pau Monné <roger....@citrix.com>
---
For consideration for 4.19: it's a bugfix of a rare instance of the watchdog
triggering, but it's also a good performance improvement when performing
cpu-online.

Hopefully runtime changes to MTRR will affect a single MSR at a time, lowering
the chance of the watchdog triggering due to the system-wide resetting of the
range.
---
 xen/arch/x86/cpu/mtrr/main.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/cpu/mtrr/main.c b/xen/arch/x86/cpu/mtrr/main.c
index 90b235f57e68..0a44ebbcb04f 100644
--- a/xen/arch/x86/cpu/mtrr/main.c
+++ b/xen/arch/x86/cpu/mtrr/main.c
@@ -573,14 +573,15 @@ void mtrr_ap_init(void)
        if (!mtrr_if || hold_mtrr_updates_on_aps)
                return;
        /*
-        * Ideally we should hold mtrr_mutex here to avoid mtrr entries changed,
-        * but this routine will be called in cpu boot time, holding the lock
-        * breaks it. This routine is called in two cases: 1.very earily time
-        * of software resume, when there absolutely isn't mtrr entry changes;
-        * 2.cpu hotadd time. We let mtrr_add/del_page hold cpuhotplug lock to
-        * prevent mtrr entry changes
+        * hold_mtrr_updates_on_aps takes care of preventing unnecessary MTRR
+        * updates when batch starting the CPUs (see
+        * mtrr_aps_sync_{begin,end}()).
+        *
+        * Otherwise just apply the current system wide MTRR values to this AP.
+        * Note this doesn't require synchronization with the other CPUs, as
+        * there are strictly no modifications of the current MTRR values.
         */
-       set_mtrr(~0U, 0, 0, 0);
+       mtrr_set_all();
 }
 
 /**
-- 
2.44.0


Reply via email to