lkml.org 
[lkml]   [2023]   [Apr]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v5 08/15] x86/mtrr: have only one set_mtrr() variant
On 12.04.23 14:30, Borislav Petkov wrote:
> On Sat, Apr 01, 2023 at 08:36:45AM +0200, Juergen Gross wrote:
>> Today there are two variants of set_mtrr(): one calling stop_machine()
>
> "... two variants which set MTRRs: set_mtrr() and set_mtrr_cpuslocked(),
> first calling ..."
>
>> and one calling stop_machine_cpuslocked().
>>
>> The first one (set_mtrr()) has only one caller, and this caller is
>> always running with only one CPU online and interrupts being off.
>
> Wait, whaaat?
>
> It's only caller is mtrr_restore() and that is part of syscorse ops
> which is registered for
>
> "The CPU has no MTRR and seems to not support SMP."
>
> Do you mean that, per chance?

I'm not sure the comment is accurate ("has no MTRR" is a bad way
to say that X86_FEATURE_MTRR is 0, BTW).

The main point is that mtrr_restore() is called with interrupts off
and before any other potential CPUs are started again.

> If you do, please explain that properly in the commit message - this is
> not a guessing game.

Okay.

> By the looks of that syscore thing, it is needed for the very old MTRR
> implementations which weren't SMP (K6, Centaur, Cyrix etc).
>
> Please explain that in the commit message too. It needs to say *why* the
> transformation you're doing is ok.

I've outlined the "why" above. I'll replace the paragraph you were referring
to.

>
> "this caller is always running with only one CPU online" is not nearly
> beginning to explain what the situation is.
>
>> Remove the first variant completely and replace the call of it with
>> a call of mtrr_if->set().
>>
>> Rename the second variant set_mtrr_cpuslocked() to set_mtrr() now that
>> there is only one variant left.
>
> Those are visible from the diff itself - you don't need to explain what
> the patch does but why.

Okay, I'll drop that paragraph.


Juergen
[unhandled content-type:application/pgp-keys][unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2023-04-12 14:57    [W:0.644 / U:0.156 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site