[PATCH v2 19/25] x86: mtrr: Update MTRRs on all CPUs

[Simon Glass]

Hi Bin,

On Sun, 28 Jun 2020 at 02:07, Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Simon,
>
> On Mon, Jun 15, 2020 at 1:00 AM Simon Glass <sjg at chromium.org> wrote:
> >
> > When the boot CPU MTRRs are updated, perform the same update on all
other
> > CPUs so they are kept in sync.
> >
> > This avoids kernel warnings about mismatched MTRRs.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > Reviewed-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>
> > ---
> >
> > Changes in v2:
> > - Rename function to mtrr_write_all()
> >
> >  arch/x86/cpu/mtrr.c | 57 +++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 57 insertions(+)
> >
> > diff --git a/arch/x86/cpu/mtrr.c b/arch/x86/cpu/mtrr.c
> > index c9b4e7d06e..5c567551e5 100644
> > --- a/arch/x86/cpu/mtrr.c
> > +++ b/arch/x86/cpu/mtrr.c
> > @@ -74,10 +74,61 @@ void mtrr_read_all(struct mtrr_info *info)
> >         }
> >  }
> >
> > +void mtrr_write_all(struct mtrr_info *info)
> > +{
> > +       struct mtrr_state state;
> > +       int i;
> > +
> > +       for (i = 0; i < MTRR_COUNT; i++) {
> > +               mtrr_open(&state, true);
> > +               wrmsrl(MTRR_PHYS_BASE_MSR(i), info->mtrr[i].base);
> > +               wrmsrl(MTRR_PHYS_MASK_MSR(i), info->mtrr[i].mask);
> > +               mtrr_close(&state, true);
> > +       }
> > +}
> > +
> > +static void write_mtrrs(void *arg)
> > +{
> > +       struct mtrr_info *info = arg;
> > +
> > +       mtrr_write_all(info);
> > +}
> > +
> > +static void read_mtrrs(void *arg)
> > +{
> > +       struct mtrr_info *info = arg;
> > +
> > +       mtrr_read_all(info);
> > +}
> > +
> > +/**
> > + * mtrr_copy_to_aps() - Copy the MTRRs from the boot CPU to other CPUs
> > + *
> > + * @return 0 on success, -ve on failure
> > + */
> > +static int mtrr_copy_to_aps(void)
> > +{
> > +       struct mtrr_info info;
> > +       int ret;
> > +
> > +       ret = mp_run_on_cpus(MP_SELECT_BSP, read_mtrrs, &info);
> > +       if (ret == -ENXIO)
> > +               return 0;
> > +       else if (ret)
> > +               return log_msg_ret("bsp", ret);
> > +
> > +       ret = mp_run_on_cpus(MP_SELECT_APS, write_mtrrs, &info);
> > +       if (ret)
> > +               return log_msg_ret("bsp", ret);
> > +
> > +       return 0;
> > +}
> > +
> >  int mtrr_commit(bool do_caches)
> >  {
> >         struct mtrr_request *req = gd->arch.mtrr_req;
> >         struct mtrr_state state;
> > +       int ret;
> >         int i;
> >
> >         debug("%s: enabled=%d, count=%d\n", __func__, gd->arch.has_mtrr,
> > @@ -99,6 +150,12 @@ int mtrr_commit(bool do_caches)
> >         mtrr_close(&state, do_caches);
> >         debug("mtrr done\n");
> >
> > +       if (gd->flags & GD_FLG_RELOC) {
> > +               ret = mtrr_copy_to_aps();
> > +               if (ret)
> > +                       return log_msg_ret("copy", ret);
> > +       }
> > +
> >         return 0;
> >  }
> >
> > --
>
> We need to find a place where mtrr in BSP is duplicated in AP.
>
> Currently this is done in init_cache_f_r() but at that time SMP is not
> ready yet.

I don't think we can. I thought about this quite a bit.

We need mtrr to be set up in init_cache_f_r(), otherwise U-Boot becomes
very slow after relocation.

We want to set up mp_init() while setting up the CPUs.

The video driver decides the address of the MTRR for the display, but it
will not be probed until used.

For example, for a board that uses serial for stdout but has video and
wants to use it later, 'setenv stdout vidconsole' on the command line will
probe the video and add the MTRR.

So I think we have to sync over the MTRRs whenever there is a change.

Regards,
Simon