Re: [PATCH v3 08/10] x86/mtrr: let cache_aps_delayed_init replace mtrr_aps_delayed_init

[Juergen Gross <jgross@xxxxxxxx>]

On 30.09.22 13:55, Borislav Petkov wrote:
> On Thu, Sep 29, 2022 at 10:26:59AM +0200, Juergen Gross wrote:
> > So right now I'm inclined to be better on the safe side by not adding any
> > cpu hotplug hook, but to use just the same "delayed AP init" flag as today,
> > just renaming it. This would leave the delayed MTRR/PAT init in place for
> > resume and kexec cases, but deferring the MTRR/PAT cleanup due to this
> > potential issue seems not appropriate, as the cleanup isn't changing the
> > behavior here.
> Ok, what's wrong with adding a special hotplug level just for that thing
> and running it very early? Practically pretty much where it was in time,
> in identify_secondary_cpu()?
Yes, this can be done. It would practically have to be the first one just
after CPUHP_BRINGUP_CPU.

The question is whether we really want to call the MTRR/PAT initialization
on hotplugged cpus only after enabling interrupts. Note that the callbacks
are activated only at the end of start_secondary(), while today MTRR/PAT
initialization is called some time earlier by:

  start_secondary()
    smp_callin()
      smp_store_cpu_info()
        identify_secondary_cpu()
          mtrr_ap_init()

I don't think this is a real problem, but I wanted to mention it.

The next question would be, why MTRR/PAT init should be special (meaning:
why are all the other functions called that early not realized via
callbacks)? Is it just because of the special handling during boot/resume?

It might be worth a discussion whether there shouldn't be a special group
of callbacks activated BEFORE interrupts are being enabled.

> Having a special one is warranted, as you explain, I'd say.
Thanks. I'll write a patch for that.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature