On Tue, May 14, 2024 at 02:50:18PM +0100, Andrew Cooper wrote:
> On 14/05/2024 12:09 pm, Andrew Cooper wrote:
> > On 13/05/2024 9:59 am, Roger Pau Monne wrote:
> >> 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.
> > "Runtime" changes will only be during dom0 boot, if at all, but yes - it
> > is restricted to a single MTRR at a time.
> >
> > It's XENPF_{add,del,read}_memtype, but it's only used by Classic Linux.
> > PVOps only issues read_memtype.
> >
> > Acked-by: Andrew Cooper <andrew.coop...@citrix.com>
>
> Actually no - this isn't safe in all cases.
>
> There are BIOSes which get MTRRs wrong, and with the APs having UC
> covering a wider region than the BSP.
>
> In this case, creating consistency will alter the MTRRs on all CPUs
> currently up, and we do need to perform the rendezvous in that case.
I'm confused, the state that gets applied in mtrr_set_all() is not
modified to match what's in the started AP registers.
An AP starting with a different set of MTRR registers than the saved
state will result in the MTRR state on the AP being changed, but not
the Xen state stored in mtrr_state, and hence there will be no changes
to synchronize.
> There are 3 cases:
>
> 1) Nothing to do. This is the overwhemlingly common case.
> 2) Local changes only. No broadcast, but we do need to enter CD mode.
> 3) Remote changes needed. Needs full broadcast.
Please bear with me, but I don't think 3) is possible during AP
bringup. It's possible I'm missing a path where the differences in
the started AP MTRR state are somehow reconciled with the cached MTRR
state?
Thanks, Roger.
