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

From: Juergen Gross <jgross@suse.com>
To: Borislav Petkov <bp@alien8.de>
Cc: xen-devel@lists.xenproject.org, x86@kernel.org,
	linux-kernel@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	"H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [PATCH v3 08/10] x86/mtrr: let cache_aps_delayed_init replace mtrr_aps_delayed_init
Date: Fri, 30 Sep 2022 15:11:07 +0200	[thread overview]
Message-ID: <2e843e28-2836-910e-bcd8-f35872adf21a@suse.com> (raw)
In-Reply-To: <YzbZJEeVHkTnWIfc@zn.tnic>

[-- Attachment #1.1.1: Type: text/plain, Size: 1765 bytes --]

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 #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]

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