Re: [Xen-devel] Core parking feature enable

["Liu, Jinsong" <jinsong.liu@xxxxxxxxx>]

Jan Beulich wrote:
>>>> On 04.03.12 at 16:48, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> wrote:
>> Yes, it does need to intercept ACPI PAD notify then handle it
>> similar way as native kernel did. The logic is at dom0 '[PATCH 3/3]
>> Xen pad logic and notification' as attached.
> 
> And I know now why I didn't look at this in any detail - it modifies a
> file that doesn't even exist in 3.3-rc5 (drivers/xen/xen_acpi_pad.c),
> nor does it mention what tree/branch this would be against. Plus,
> ACPI_PROCESSOR_AGGREGATOR is a tristate symbol, yet your code
> assumes it to be boolean.

Yes, tristate/bool is an issue of core parking dom0-side patches which has 
involved some discussion how to solve it in a reasonable way (as attached). I 
will update it later.

> 
> But yes, given the way this is implemented I withdraw my reservations
> against the hypervisor side changes, as doing this completely in the
> Dom0 kernel would be cumbersome (due to the need to call sysctl
> functions), and doing it entirely in user space would indeed only be
> justified if the native kernel did it this way too.
> 
> Jan

Thanks Jan, Keir, and Haitao!

--- Begin Message ---

• To: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
• From: "Liu, Jinsong" <jinsong.liu@xxxxxxxxx>
• Date: Wed, 29 Feb 2012 08:01:54 +0000
• Accept-language: en-US
• Cc: "Brown, Len" <len.brown@xxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, "linux-kernel@xxxxxxxxxxxxxxx" <linux-kernel@xxxxxxxxxxxxxxx>, "linux-acpi@xxxxxxxxxxxxxxx" <linux-acpi@xxxxxxxxxxxxxxx>, "keir.xen@xxxxxxxxx" <keir.xen@xxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, "Li, Shaohua" <shaohua.li@xxxxxxxxx>, "lenb@xxxxxxxxxx" <lenb@xxxxxxxxxx>
• List-id: Xen developer discussion <xen-devel.lists.xen.org>
• Thread-index: AQHM9Kz1m4MGy1d3TzK01xgwJYdnxZZSeQoggAD7/5A=
• Thread-topic: [Xen-devel] [PATCH 1/2] RFC: Prepare PAD for native and xen platform

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" <jinsong.liu@xxxxxxxxx> 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
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel

--- End Message ---

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel