[v5,09/15] x86/mtrr: allocate mtrr_value array dynamically
Commit Message
The mtrr_value[] array is a static variable, which is used only in a
few configurations. Consuming 6kB is ridiculous for this case,
especially as the array doesn't need to be that large and it can easily
be allocated dynamically.
The "few configurations" are all 32-bit ones, so put the code inside a
CONFIG_X86_32 #ifdef.
Signed-off-by: Juergen Gross <jgross@suse.com>
Tested-by: Michael Kelley <mikelley@microsoft.com>
---
V5:
- check for allocation failure (Kai Huang, Boris Petkov)
- add #ifdef
---
arch/x86/kernel/cpu/mtrr/mtrr.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
Comments
On Sat, Apr 01, 2023 at 08:36:46AM +0200, Juergen Gross wrote:
> +#ifdef CONFIG_X86_32
TBH, I'm not really crazy about adding more ifdeffery.
Wondering if adding a 32-bit only build object:
obj-$(CONFIG_X86_32) += amd.o cyrix.o centaur.o legacy.o
to arch/x86/kernel/cpu/mtrr/Makefile and moving all that gunk over
there, out of the way, would be even cleaner...
> + mtrr_value = kcalloc(num_var_ranges, sizeof(*mtrr_value), GFP_KERNEL);
> +
> /*
> * The CPU has no MTRR and seems to not support SMP. They have
> * specific drivers, we use a tricky method to support
> - * suspend/resume for them.
> + * suspend/resume for them. In case above allocation failed we can't
> + * support suspend/resume (handled in mtrr_save()).
Oh well.
On 12.04.23 23:11, Borislav Petkov wrote:
> On Sat, Apr 01, 2023 at 08:36:46AM +0200, Juergen Gross wrote:
>> +#ifdef CONFIG_X86_32
>
> TBH, I'm not really crazy about adding more ifdeffery.
>
> Wondering if adding a 32-bit only build object:
>
> obj-$(CONFIG_X86_32) += amd.o cyrix.o centaur.o legacy.o
>
> to arch/x86/kernel/cpu/mtrr/Makefile and moving all that gunk over
> there, out of the way, would be even cleaner...
Yes, that should be rather simple.
Will add a patch to do that.
Juergen
@@ -560,8 +560,10 @@ int arch_phys_wc_index(int handle)
}
EXPORT_SYMBOL_GPL(arch_phys_wc_index);
-/* The suspend/resume methods are only for CPU without MTRR. CPU using generic
- * MTRR driver doesn't require this
+#ifdef CONFIG_X86_32
+/*
+ * The suspend/resume methods are only for CPUs without MTRR. CPUs using generic
+ * MTRR driver don't require this.
*/
struct mtrr_value {
mtrr_type ltype;
@@ -569,12 +571,15 @@ struct mtrr_value {
unsigned long lsize;
};
-static struct mtrr_value mtrr_value[MTRR_MAX_VAR_RANGES];
+static struct mtrr_value *mtrr_value;
static int mtrr_save(void)
{
int i;
+ if (!mtrr_value)
+ return -ENOMEM;
+
for (i = 0; i < num_var_ranges; i++) {
mtrr_if->get(i, &mtrr_value[i].lbase,
&mtrr_value[i].lsize,
@@ -596,12 +601,11 @@ static void mtrr_restore(void)
}
}
-
-
static struct syscore_ops mtrr_syscore_ops = {
.suspend = mtrr_save,
.resume = mtrr_restore,
};
+#endif /* CONFIG_X86_32 */
int __initdata changed_by_mtrr_cleanup;
@@ -730,15 +734,20 @@ static int __init mtrr_init_finialize(void)
return 0;
}
+#ifdef CONFIG_X86_32
+ mtrr_value = kcalloc(num_var_ranges, sizeof(*mtrr_value), GFP_KERNEL);
+
/*
* The CPU has no MTRR and seems to not support SMP. They have
* specific drivers, we use a tricky method to support
- * suspend/resume for them.
+ * suspend/resume for them. In case above allocation failed we can't
+ * support suspend/resume (handled in mtrr_save()).
*
* TBD: is there any system with such CPU which supports
* suspend/resume? If no, we should remove the code.
*/
register_syscore_ops(&mtrr_syscore_ops);
+#endif
return 0;
}