* [PATCH] x86/cpu: Drop _init from *_cpu_cap functions
@ 2022-08-11 10:17 Ross Lagerwall
2022-08-11 10:21 ` Andrew Cooper
0 siblings, 1 reply; 5+ messages in thread
From: Ross Lagerwall @ 2022-08-11 10:17 UTC (permalink / raw)
To: xen-devel
Cc: Jan Beulich, Andrew Cooper, Roger Pau Monné, Wei Liu,
Ross Lagerwall
These functions may be called by init_amd() after the _init functions
have been purged during CPU hotplug or PV shim boot so drop the _init.
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
xen/arch/x86/cpu/common.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index 0412dbc915..20581bf3f8 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -57,7 +57,7 @@ static unsigned int forced_caps[NCAPINTS];
DEFINE_PER_CPU(bool, full_gdt_loaded);
-void __init setup_clear_cpu_cap(unsigned int cap)
+void setup_clear_cpu_cap(unsigned int cap)
{
const uint32_t *dfs;
unsigned int i;
@@ -86,7 +86,7 @@ void __init setup_clear_cpu_cap(unsigned int cap)
}
}
-void __init setup_force_cpu_cap(unsigned int cap)
+void setup_force_cpu_cap(unsigned int cap)
{
if (__test_and_set_bit(cap, forced_caps))
return;
@@ -100,7 +100,7 @@ void __init setup_force_cpu_cap(unsigned int cap)
__set_bit(cap, boot_cpu_data.x86_capability);
}
-bool __init is_forced_cpu_cap(unsigned int cap)
+bool is_forced_cpu_cap(unsigned int cap)
{
return test_bit(cap, forced_caps);
}
--
2.31.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] x86/cpu: Drop _init from *_cpu_cap functions
2022-08-11 10:17 [PATCH] x86/cpu: Drop _init from *_cpu_cap functions Ross Lagerwall
@ 2022-08-11 10:21 ` Andrew Cooper
2022-08-11 10:30 ` Ross Lagerwall
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Cooper @ 2022-08-11 10:21 UTC (permalink / raw)
To: Ross Lagerwall, xen-devel@lists.xenproject.org
Cc: Jan Beulich, Roger Pau Monne, Wei Liu
On 11/08/2022 11:17, Ross Lagerwall wrote:
> These functions may be called by init_amd() after the _init functions
> have been purged during CPU hotplug or PV shim boot so drop the _init.
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Hmm. That's a bug in init_amd() I'd say. These really shouldn't be
used after __init.
Which path exploded specifically?
~Andrew
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] x86/cpu: Drop _init from *_cpu_cap functions
2022-08-11 10:21 ` Andrew Cooper
@ 2022-08-11 10:30 ` Ross Lagerwall
2022-08-11 10:34 ` Andrew Cooper
0 siblings, 1 reply; 5+ messages in thread
From: Ross Lagerwall @ 2022-08-11 10:30 UTC (permalink / raw)
To: Andrew Cooper, xen-devel@lists.xenproject.org
Cc: Jan Beulich, Roger Pau Monne, Wei Liu
> From: Andrew Cooper <Andrew.Cooper3@citrix.com>
> Sent: Thursday, August 11, 2022 11:21 AM
> To: Ross Lagerwall <ross.lagerwall@citrix.com>; xen-devel@lists.xenproject.org <xen-devel@lists.xenproject.org>
> Cc: Jan Beulich <jbeulich@suse.com>; Roger Pau Monne <roger.pau@citrix.com>; Wei Liu <wl@xen.org>
> Subject: Re: [PATCH] x86/cpu: Drop _init from *_cpu_cap functions
>
> On 11/08/2022 11:17, Ross Lagerwall wrote:
> > These functions may be called by init_amd() after the _init functions
> > have been purged during CPU hotplug or PV shim boot so drop the _init.
> >
> > Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
>
> Hmm. That's a bug in init_amd() I'd say. These really shouldn't be
> used after __init.
>
> Which path exploded specifically?
The stack trace was:
setup_force_cpu_cap
init_amd
identify_cpu
start_secondary
In setup_force_cpu_cap() here:
/*
* On pre-CLFLUSHOPT AMD CPUs, CLFLUSH is weakly ordered with
* everything, including reads and writes to address, and
* LFENCE/SFENCE instructions.
*/
if (!cpu_has_clflushopt)
setup_force_cpu_cap(X86_BUG_CLFLUSH_MFENCE);
which was recently introduced by:
commit 062868a5a8b428b85db589fa9a6d6e43969ffeb9
Author: Andrew Cooper <andrew.cooper3@citrix.com>
Date: Thu Jun 9 14:23:07 2022 +0200
x86/amd: Work around CLFLUSH ordering on older parts
Should the fix rather be to guard that call with "if (c == &boot_cpu_data ..." ?
Ross
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] x86/cpu: Drop _init from *_cpu_cap functions
2022-08-11 10:30 ` Ross Lagerwall
@ 2022-08-11 10:34 ` Andrew Cooper
2022-08-11 10:59 ` Jan Beulich
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Cooper @ 2022-08-11 10:34 UTC (permalink / raw)
To: Ross Lagerwall, xen-devel@lists.xenproject.org
Cc: Jan Beulich, Roger Pau Monne, Wei Liu
On 11/08/2022 11:30, Ross Lagerwall wrote:
>> From: Andrew Cooper <Andrew.Cooper3@citrix.com>
>> Sent: Thursday, August 11, 2022 11:21 AM
>> To: Ross Lagerwall <ross.lagerwall@citrix.com>; xen-devel@lists.xenproject.org <xen-devel@lists.xenproject.org>
>> Cc: Jan Beulich <jbeulich@suse.com>; Roger Pau Monne <roger.pau@citrix.com>; Wei Liu <wl@xen.org>
>> Subject: Re: [PATCH] x86/cpu: Drop _init from *_cpu_cap functions
>>
>> On 11/08/2022 11:17, Ross Lagerwall wrote:
>>> These functions may be called by init_amd() after the _init functions
>>> have been purged during CPU hotplug or PV shim boot so drop the _init.
>>>
>>> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
>> Hmm. That's a bug in init_amd() I'd say. These really shouldn't be
>> used after __init.
>>
>> Which path exploded specifically?
> The stack trace was:
>
> setup_force_cpu_cap
> init_amd
> identify_cpu
> start_secondary
>
> In setup_force_cpu_cap() here:
>
> /*
> * On pre-CLFLUSHOPT AMD CPUs, CLFLUSH is weakly ordered with
> * everything, including reads and writes to address, and
> * LFENCE/SFENCE instructions.
> */
> if (!cpu_has_clflushopt)
> setup_force_cpu_cap(X86_BUG_CLFLUSH_MFENCE);
>
> which was recently introduced by:
>
> commit 062868a5a8b428b85db589fa9a6d6e43969ffeb9
> Author: Andrew Cooper <andrew.cooper3@citrix.com>
> Date: Thu Jun 9 14:23:07 2022 +0200
>
> x86/amd: Work around CLFLUSH ordering on older parts
Bah, and that was also backported in a security fix, to everything back
to 4.12 is broken.
> Should the fix rather be to guard that call with "if (c == &boot_cpu_data ..." ?
Yes please.
Sorry.
~Andrew
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] x86/cpu: Drop _init from *_cpu_cap functions
2022-08-11 10:34 ` Andrew Cooper
@ 2022-08-11 10:59 ` Jan Beulich
0 siblings, 0 replies; 5+ messages in thread
From: Jan Beulich @ 2022-08-11 10:59 UTC (permalink / raw)
To: Andrew Cooper, Ross Lagerwall
Cc: Roger Pau Monne, Wei Liu, xen-devel@lists.xenproject.org
On 11.08.2022 12:34, Andrew Cooper wrote:
> On 11/08/2022 11:30, Ross Lagerwall wrote:
>>> From: Andrew Cooper <Andrew.Cooper3@citrix.com>
>>> Sent: Thursday, August 11, 2022 11:21 AM
>>> To: Ross Lagerwall <ross.lagerwall@citrix.com>; xen-devel@lists.xenproject.org <xen-devel@lists.xenproject.org>
>>> Cc: Jan Beulich <jbeulich@suse.com>; Roger Pau Monne <roger.pau@citrix.com>; Wei Liu <wl@xen.org>
>>> Subject: Re: [PATCH] x86/cpu: Drop _init from *_cpu_cap functions
>>>
>>> On 11/08/2022 11:17, Ross Lagerwall wrote:
>>>> These functions may be called by init_amd() after the _init functions
>>>> have been purged during CPU hotplug or PV shim boot so drop the _init.
>>>>
>>>> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
>>> Hmm. That's a bug in init_amd() I'd say. These really shouldn't be
>>> used after __init.
>>>
>>> Which path exploded specifically?
>> The stack trace was:
>>
>> setup_force_cpu_cap
>> init_amd
>> identify_cpu
>> start_secondary
>>
>> In setup_force_cpu_cap() here:
>>
>> /*
>> * On pre-CLFLUSHOPT AMD CPUs, CLFLUSH is weakly ordered with
>> * everything, including reads and writes to address, and
>> * LFENCE/SFENCE instructions.
>> */
>> if (!cpu_has_clflushopt)
>> setup_force_cpu_cap(X86_BUG_CLFLUSH_MFENCE);
>>
>> which was recently introduced by:
>>
>> commit 062868a5a8b428b85db589fa9a6d6e43969ffeb9
>> Author: Andrew Cooper <andrew.cooper3@citrix.com>
>> Date: Thu Jun 9 14:23:07 2022 +0200
>>
>> x86/amd: Work around CLFLUSH ordering on older parts
>
> Bah, and that was also backported in a security fix, to everything back
> to 4.12 is broken.
4.13, but yes. Oh well.
It's actually odd that we use __set_bit() for X86_FEATURE_MFENCE_RDTSC (a
few lines up) but the more heavyweight function for X86_BUG_CLFLUSH_MFENCE.
Jan
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-08-11 10:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-11 10:17 [PATCH] x86/cpu: Drop _init from *_cpu_cap functions Ross Lagerwall
2022-08-11 10:21 ` Andrew Cooper
2022-08-11 10:30 ` Ross Lagerwall
2022-08-11 10:34 ` Andrew Cooper
2022-08-11 10:59 ` Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.