>From 9a12d7c610bffb900015e8947a57e4e428058abf Mon Sep 17 00:00:00 2001
From: Liu, Jinsong <[email protected]>
Date: Fri, 24 Feb 2012 01:51:18 +0800
Subject: [PATCH 1/2] Prepare PAD for native and xen platform
This patch is to prepare PAD (Processor Aggregator Device) for native and xen platform.
When working under baremetal it initializes native acpi_pad, while working under
xen platform it would hook to xen acpi_pad (would implement at later patch).
Signed-off-by: Liu, Jinsong <[email protected]>
---
drivers/acpi/Kconfig | 3 ++-
drivers/acpi/acpi_pad.c | 21 +++++++++------------
include/acpi/acpi_drivers.h | 15 +++++++++++++++
3 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 7556913..2653a98 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -213,10 +213,11 @@ config ACPI_HOTPLUG_CPU
default y
config ACPI_PROCESSOR_AGGREGATOR
- tristate "Processor Aggregator"
+ bool "Processor Aggregator"
depends on ACPI_PROCESSOR
depends on EXPERIMENTAL
depends on X86
+ default n
help
ACPI 4.0 defines processor Aggregator, which enables OS to perform
specific processor configuration and control that applies to all
diff --git a/drivers/acpi/acpi_pad.c b/drivers/acpi/acpi_pad.c
index a43fa1a..8aebbe5 100644
--- a/drivers/acpi/acpi_pad.c
+++ b/drivers/acpi/acpi_pad.c
@@ -2,6 +2,7 @@
* acpi_pad.c ACPI Processor Aggregator Driver
*
* Copyright (c) 2009, Intel Corporation.
+ * Author: Li, Shaohua <[email protected]>
*
* This program is free software; you can redistribute it and/or modify it
* under the terms and conditions of the GNU General Public License,
@@ -32,9 +33,6 @@
#include <acpi/acpi_drivers.h>
#include <asm/mwait.h>
-#define ACPI_PROCESSOR_AGGREGATOR_CLASS "acpi_pad"
-#define ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME "Processor Aggregator"
-#define ACPI_PROCESSOR_AGGREGATOR_NOTIFY 0x80
static DEFINE_MUTEX(isolated_cpus_lock);
static unsigned long power_saving_mwait_eax;
@@ -510,7 +508,7 @@ static struct acpi_driver acpi_pad_driver = {
},
};
-static int __init acpi_pad_init(void)
+static int __init native_acpi_pad_init(void)
{
power_saving_mwait_init();
if (power_saving_mwait_eax == 0)
@@ -519,13 +517,12 @@ static int __init acpi_pad_init(void)
return acpi_bus_register_driver(&acpi_pad_driver);
}
-static void __exit acpi_pad_exit(void)
+struct acpi_pad_ops acpi_pad_ops = {
+ .init = native_acpi_pad_init,
+};
+
+static int __init acpi_pad_init(void)
{
- acpi_bus_unregister_driver(&acpi_pad_driver);
+ return acpi_pad_ops.init();
}
-
-module_init(acpi_pad_init);
-module_exit(acpi_pad_exit);
-MODULE_AUTHOR("Shaohua Li<[email protected]>");
-MODULE_DESCRIPTION("ACPI Processor Aggregator Driver");
-MODULE_LICENSE("GPL");
+device_initcall(acpi_pad_init);
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index bb145e4..d40abbc 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -115,6 +115,21 @@ void pci_acpi_crs_quirks(void);
#define ACPI_PROCESSOR_LIMIT_INCREMENT 0x01
#define ACPI_PROCESSOR_LIMIT_DECREMENT 0x02
+/* --------------------------------------------------------------------------
+ PAD (Processor Aggregator Device)
+ -------------------------------------------------------------------------- */
+
+#define ACPI_PROCESSOR_AGGREGATOR_CLASS "acpi_pad"
+#define ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME "Processor Aggregator"
+#define ACPI_PROCESSOR_AGGREGATOR_NOTIFY 0x80
+
+struct acpi_pad_ops {
+ int (*init)(void);
+};
+#ifdef CONFIG_ACPI_PROCESSOR_AGGREGATOR
+extern struct acpi_pad_ops acpi_pad_ops;
+#endif
+
/*--------------------------------------------------------------------------
Dock Station
-------------------------------------------------------------------------- */
--
1.7.1
>>> "Liu, Jinsong" <[email protected]> 02/23/12 2:29 PM >>>
>--- a/drivers/acpi/Kconfig
>+++ b/drivers/acpi/Kconfig
>@@ -213,10 +213,11 @@ config ACPI_HOTPLUG_CPU
> default y
>
>config ACPI_PROCESSOR_AGGREGATOR
>- tristate "Processor Aggregator"
>+ bool "Processor Aggregator"
There must be ways to address whatever strange problem you see without
making this piece of code non-modular.
> depends on ACPI_PROCESSOR
> depends on EXPERIMENTAL
> depends on X86
>+ default n
This is pointless.
> help
> ACPI 4.0 defines processor Aggregator, which enables OS to perform
> specific processor configuration and control that applies to all
Jan
Jan Beulich wrote:
>>>> "Liu, Jinsong" <[email protected]> 02/23/12 2:29 PM >>>
>> --- a/drivers/acpi/Kconfig
>> +++ b/drivers/acpi/Kconfig
>> @@ -213,10 +213,11 @@ config ACPI_HOTPLUG_CPU
>> default y
> >
>> config ACPI_PROCESSOR_AGGREGATOR
>> - tristate "Processor Aggregator"
>> + bool "Processor Aggregator"
>
> There must be ways to address whatever strange problem you see without
> making this piece of code non-modular.
>
Yes, another approach is x86_init approach, defining acpi_pad_ops at x86_init.c and overwritten when xen_start_kernel.
This patch is just a RFC patch, to evaluate which approch is more reasonable :-)
>> depends on ACPI_PROCESSOR
>> depends on EXPERIMENTAL
>> depends on X86
>> + default n
>
> This is pointless.
Elaborate more? just a little curious why Kconfig has so many default n.
>
>> help
>> ACPI 4.0 defines processor Aggregator, which enables OS to
>> perform specific processor configuration and control that
>> applies to all
>
> Jan
>>> On 23.02.12 at 17:58, "Liu, Jinsong" <[email protected]> wrote:
> Jan Beulich wrote:
>>>>> "Liu, Jinsong" <[email protected]> 02/23/12 2:29 PM >>>
>>> depends on ACPI_PROCESSOR
>>> depends on EXPERIMENTAL
>>> depends on X86
>>> + default n
>>
>> This is pointless.
>
> Elaborate more? just a little curious why Kconfig has so many default n.
Which are all pointless afaict (and I did already get a couple of
patches merged to remove some of them) - the tool defaults
them to 'n' anyway.
Jan
Liu, Jinsong wrote:
> Jan Beulich wrote:
>>>>> "Liu, Jinsong" <[email protected]> 02/23/12 2:29 PM >>>
>>> --- a/drivers/acpi/Kconfig
>>> +++ b/drivers/acpi/Kconfig
>>> @@ -213,10 +213,11 @@ config ACPI_HOTPLUG_CPU
>>> default y
>> >
>>> config ACPI_PROCESSOR_AGGREGATOR
>>> - tristate "Processor Aggregator"
>>> + bool "Processor Aggregator"
>>
>> There must be ways to address whatever strange problem you see
>> without making this piece of code non-modular.
>>
>
> Yes, another approach is x86_init approach, defining acpi_pad_ops at
> x86_init.c and overwritten when xen_start_kernel. This patch is just
> a RFC patch, to evaluate which approch is more reasonable :-)
>
Have a more think about it, x86_init approach still need to disable acpi_pad module.
Seems we have to set acpi_pad as bool, as long as it need to hook to native acpi_pad fucs/variables.
Thanks,
Jinsong-
On Sun, Feb 26, 2012 at 08:25:41AM +0000, Liu, Jinsong wrote:
> Liu, Jinsong wrote:
> > Jan Beulich wrote:
> >>>>> "Liu, Jinsong" <[email protected]> 02/23/12 2:29 PM >>>
> >>> --- a/drivers/acpi/Kconfig
> >>> +++ b/drivers/acpi/Kconfig
> >>> @@ -213,10 +213,11 @@ config ACPI_HOTPLUG_CPU
> >>> default y
> >> >
> >>> config ACPI_PROCESSOR_AGGREGATOR
> >>> - tristate "Processor Aggregator"
> >>> + bool "Processor Aggregator"
> >>
> >> There must be ways to address whatever strange problem you see
> >> without making this piece of code non-modular.
> >>
> >
> > Yes, another approach is x86_init approach, defining acpi_pad_ops at
> > x86_init.c and overwritten when xen_start_kernel. This patch is just
> > a RFC patch, to evaluate which approch is more reasonable :-)
> >
>
> Have a more think about it, x86_init approach still need to disable acpi_pad module.
> Seems we have to set acpi_pad as bool, as long as it need to hook to native acpi_pad fucs/variables.
What about the other approach I suggested where there are some function
overrides in osl.c? Something similar to https://lkml.org/lkml/2012/1/17/401,
specifically https://lkml.org/lkml/2012/1/17/403 - that way you are not turning
the modules into being built in, but intead have the function table already in
the kernel (as some form of EXPORT_SYMBOL_GPL or a registration function).
Instead of just one function being over-ridden it could have some more. However
I am not sure if the osl.c is the place for this either. Perhaps Len might
have some better ideas?
Konrad Rzeszutek Wilk wrote:
> On Sun, Feb 26, 2012 at 08:25:41AM +0000, Liu, Jinsong wrote:
>> Liu, Jinsong wrote:
>>> Jan Beulich wrote:
>>>>>>> "Liu, Jinsong" <[email protected]> 02/23/12 2:29 PM >>>
>>>>> --- a/drivers/acpi/Kconfig
>>>>> +++ b/drivers/acpi/Kconfig
>>>>> @@ -213,10 +213,11 @@ config ACPI_HOTPLUG_CPU
>>>>> default y
>>>> >
>>>>> config ACPI_PROCESSOR_AGGREGATOR
>>>>> - tristate "Processor Aggregator"
>>>>> + bool "Processor Aggregator"
>>>>
>>>> There must be ways to address whatever strange problem you see
>>>> without making this piece of code non-modular.
>>>>
>>>
>>> Yes, another approach is x86_init approach, defining acpi_pad_ops at
>>> x86_init.c and overwritten when xen_start_kernel. This patch is just
>>> a RFC patch, to evaluate which approch is more reasonable :-)
>>>
>>
>> Have a more think about it, x86_init approach still need to disable
>> acpi_pad module.
>> Seems we have to set acpi_pad as bool, as long as it need to hook to
>> native acpi_pad fucs/variables.
>
> What about the other approach I suggested where there are some
> function overrides in osl.c? Something similar to
> https://lkml.org/lkml/2012/1/17/401, specifically
> https://lkml.org/lkml/2012/1/17/403 - that way you are not turning
> the modules into being built in, but intead have the function table
> already in the kernel (as some form of EXPORT_SYMBOL_GPL or a
> registration function).
>
Thanks for the example (it's good itself :-), but per my understanding they are different cases.
In the osl example case, tboot_late_init call acpi_os_set_prepare_sleep to register func, so it works no matter tboot.c built as y/n/m (through currently it's bool).
However, in our case, we just cannot do so. We need
1. predefine a hook to native acpi pad funcs, take effect when static compile stage;
2. when xen init, redirect the hook to xen acpi pad funcs, take effect at running time;
(The point is, we cannot do init twice for native and xen acpi pad, so we have to statically predefine a hook to native acpi_pad)
For the reason above, I think we have to remove acpi_pad module, as long as we need to hook to native acpi_pad funcs. Thoughts?
Regards,
Jinsong
Liu, Jinsong wrote:
> Konrad Rzeszutek Wilk wrote:
>> On Sun, Feb 26, 2012 at 08:25:41AM +0000, Liu, Jinsong wrote:
>>> Liu, Jinsong wrote:
>>>> Jan Beulich wrote:
>>>>>>>> "Liu, Jinsong" <[email protected]> 02/23/12 2:29 PM >>>
>>>>>> --- a/drivers/acpi/Kconfig
>>>>>> +++ b/drivers/acpi/Kconfig
>>>>>> @@ -213,10 +213,11 @@ config ACPI_HOTPLUG_CPU
>>>>>> default y
>>>>> >
>>>>>> config ACPI_PROCESSOR_AGGREGATOR
>>>>>> - tristate "Processor Aggregator"
>>>>>> + bool "Processor Aggregator"
>>>>>
>>>>> There must be ways to address whatever strange problem you see
>>>>> without making this piece of code non-modular.
>>>>>
>>>>
>>>> Yes, another approach is x86_init approach, defining acpi_pad_ops
>>>> at x86_init.c and overwritten when xen_start_kernel. This patch is
>>>> just a RFC patch, to evaluate which approch is more reasonable :-)
>>>>
>>>
>>> Have a more think about it, x86_init approach still need to disable
>>> acpi_pad module. Seems we have to set acpi_pad as bool, as long as
>>> it need to hook to native acpi_pad fucs/variables.
>>
>> What about the other approach I suggested where there are some
>> function overrides in osl.c? Something similar to
>> https://lkml.org/lkml/2012/1/17/401, specifically
>> https://lkml.org/lkml/2012/1/17/403 - that way you are not turning
>> the modules into being built in, but intead have the function table
>> already in the kernel (as some form of EXPORT_SYMBOL_GPL or a
>> registration function).
>>
>
> Thanks for the example (it's good itself :-), but per my
> understanding they are different cases.
>
> In the osl example case, tboot_late_init call
> acpi_os_set_prepare_sleep to register func, so it works no matter
> tboot.c built as y/n/m (through currently it's bool).
>
> However, in our case, we just cannot do so. We need
> 1. predefine a hook to native acpi pad funcs, take effect when static
> compile stage;
> 2. when xen init, redirect the hook to xen acpi pad funcs, take
> effect at running time; (The point is, we cannot do init twice for
> native and xen acpi pad, so we have to statically predefine a hook to
> native acpi_pad)
>
> For the reason above, I think we have to remove acpi_pad module, as
> long as we need to hook to native acpi_pad funcs. Thoughts?
>
Compare approaches:
1. xen overwritten approach (patches V2, x86_init, osl approach)
Pros:
a little simpler code
Cons:
1). specific to xen, cannot extend to other virt platform;
2). need to change natvie acpi_pad as modular;
2. paravirt interface approach (original patches V1)
Pros:
1). standard hypervisor-agnostic interface (USENIX conference 2006), can easily hook to Xen/lguest/... on demand;
2). arch independent;
3). no need to change native acpi_pad as non-modular;
Cons:
a little complicated code, and code patching is some overkilled for this case (but no harm);
(BTW, in the future we need add more and more pv ops, like pv_pm_ops, pv_cpu_hotplug_ops, pv_mem_hotplug_ops, etc. So how about add a pv_misc_ops template to handle them all? seems it's a common issue).
Your opinion?
Thanks,
Jinsong-