Thread (9 messages) 9 messages, 2 authors, 2012-02-23

RE: [Xen-devel] [PATCH 1/3] PAD helper for native and paravirt platform

From: Liu, Jinsong <hidden>
Date: 2012-02-23 13:28:18
Also in: lkml, xen-devel

Konrad Rzeszutek Wilk wrote:
On Wed, Feb 22, 2012 at 05:02:59PM +0000, Liu, Jinsong wrote:
quoted
Konrad Rzeszutek Wilk wrote:
quoted
On Tue, Feb 21, 2012 at 05:49:58AM +0000, Liu, Jinsong wrote:
quoted
Konrad Rzeszutek Wilk wrote:
quoted
quoted
quoted
quoted
quoted
+struct pv_pad_ops {
+	int (*acpi_pad_init)(void);
+	void (*acpi_pad_exit)(void);
+};
+
Looking at this a bit closer I am not sure why you choose the
paravirt interface for this? There is another one - the x86 that
could have been choosen. Or introduce a new one that is
specific to ACPI. 

I am curious - what was the reason for using the paravirt
interface? I understand it does get the job done, but it seems a
bit overkill when something simple could have been used?
It uses paravirt interface to avoid some code like 'xen_...' in
native code path (acpi_pad.c).
I'm not quite sure what does 'x86' here mean? Adding 2 fields
(acpi_pad_init/exit) in arch/x86/xen/enlighten.c --> xen_cpu_ops?
seems it's much simpler.
arch/x86/include/asm/x86_init.h

But before you go that way let me ask you another question - can
ACPI PAD be used on ARM or IA64? If so, wouldn't this fail
compilation as this pvops structure is not defined on IA64?
Ideally ACPI PAD is not bound to some arch, so IMO it could be used
at least on IA64 (through currently no real PAD on IA64 platform as
far as I know). However, in native acpi_pad implementation, it
indeed depends on X86 for reason like mwait.
So for xen acpi_pad, I think it's OK to choose x86, defining an
acpi_pad_ops at x86_init.c which would be overwritten when xen
init. 
OK, or in osl.c. We need Len to chime in here as I can see this
expanding in the future.
quoted
Another choice is to define config ACPI_PROCESSOR_AGGREGATOR as
'bool', which would disable native acpi_pad module.
Ewww. No.
I'm OK with x86_init approach, but advantage of 'config
ACPI_PROCESSOR_AGGREGATOR as bool' would get rid of X86/IA64/...
arch issue for xen (at least from coding view), through it need
disable native acpi_pad module (IMO acpi_pad module has not strong
reason to must be so). Have a re-consider of this approach? :-)   
But it is a compile option right? We wantone kernel that can do both
baremetal and Xen.
Sorry, I didn't speak clear my approach.
The approach can enable kernel run both baremetal and Xen platform, and not bind acpi pad logic to x86.
Send 2 patches as RFC.

Thanks
Jinsong
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help